Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29737602: Issue 4580 - Make filter/request logging use plain tabIds, prepare for multi-tab requests (Closed)

Created:
March 31, 2018, 12:27 a.m. by Sebastian Noack
Modified:
April 5, 2018, 6:32 p.m.
Reviewers:
kzar
CC:
Manish Jethani
Visibility:
Public.

Description

Issue 4580 - Make filter/request logging use plain tabIds, prepare for multi-tab requests

Patch Set 1 : #

Total comments: 8

Patch Set 2 : Removed outdated comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -60 lines) Patch
M lib/csp.js View 1 chunk +3 lines, -3 lines 0 comments Download
M lib/cssInjection.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/devtools.js View 1 9 chunks +16 lines, -20 lines 0 comments Download
M lib/popupBlocker.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/requestBlocker.js View 4 chunks +12 lines, -14 lines 0 comments Download
M lib/stats.js View 1 chunk +18 lines, -14 lines 0 comments Download
M lib/whitelisting.js View 2 chunks +3 lines, -7 lines 0 comments Download

Messages

Total messages: 5
Sebastian Noack
https://codereview.adblockplus.org/29737602/diff/29737603/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29737602/diff/29737603/lib/requestBlocker.js#newcode80 lib/requestBlocker.js:80: let tabIds = tabId != -1 ? [tabId] : ...
March 31, 2018, 1:18 a.m. (2018-03-31 01:18:53 UTC) #1
kzar
https://codereview.adblockplus.org/29737602/diff/29739586/lib/devtools.js File lib/devtools.js (right): https://codereview.adblockplus.org/29737602/diff/29739586/lib/devtools.js#newcode34 lib/devtools.js:34: // and recorded items. We can't use a PageMap ...
April 3, 2018, 5:14 p.m. (2018-04-03 17:14:04 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29737602/diff/29739586/lib/devtools.js File lib/devtools.js (right): https://codereview.adblockplus.org/29737602/diff/29739586/lib/devtools.js#newcode34 lib/devtools.js:34: // and recorded items. We can't use a PageMap ...
April 4, 2018, 1:50 a.m. (2018-04-04 01:50:34 UTC) #3
Sebastian Noack
Also note that this patch is based on https://codereview.adblockplus.org/29737568/ (which you didn't comment on yet).
April 4, 2018, 2:01 a.m. (2018-04-04 02:01:34 UTC) #4
kzar
April 5, 2018, 10:13 a.m. (2018-04-05 10:13:00 UTC) #5
LGTM

Powered by Google App Engine
This is Rietveld