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

Issue 29329527: Issue 3208 - Consider private browsing in contentPolicy module rather than filterStorage (Closed)

Created:
Oct. 29, 2015, 8:32 p.m. by Wladimir Palant
Modified:
Nov. 11, 2015, 2:07 p.m.
Visibility:
Public.

Description

Issue 3208 - Consider private browsing in contentPolicy module rather than filterStorage

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fixed PrivateBrowsingUtils call #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -78 lines) Patch
M lib/contentPolicy.js View 1 5 chunks +11 lines, -4 lines 0 comments Download
M lib/filterStorage.js View 2 chunks +2 lines, -74 lines 0 comments Download

Messages

Total messages: 5
Wladimir Palant
Oct. 29, 2015, 8:32 p.m. (2015-10-29 20:32:36 UTC) #1
tschuster
LGTM
Oct. 31, 2015, 12:16 p.m. (2015-10-31 12:16:39 UTC) #2
Thomas Greiner
https://codereview.adblockplus.org/29329527/diff/29329528/lib/contentPolicy.js File lib/contentPolicy.js (right): https://codereview.adblockplus.org/29329527/diff/29329528/lib/contentPolicy.js#newcode148 lib/contentPolicy.js:148: if (!PrivateBrowsingUtils.isWindowPrivate(wnd)) I'm wondering whether `wnd` here could potentially ...
Nov. 2, 2015, 5:29 p.m. (2015-11-02 17:29:23 UTC) #3
Wladimir Palant
https://codereview.adblockplus.org/29329527/diff/29329528/lib/contentPolicy.js File lib/contentPolicy.js (right): https://codereview.adblockplus.org/29329527/diff/29329528/lib/contentPolicy.js#newcode148 lib/contentPolicy.js:148: if (!PrivateBrowsingUtils.isWindowPrivate(wnd)) On 2015/11/02 17:29:23, Thomas Greiner wrote: > ...
Nov. 2, 2015, 6:49 p.m. (2015-11-02 18:49:22 UTC) #4
Thomas Greiner
Nov. 2, 2015, 7:02 p.m. (2015-11-02 19:02:07 UTC) #5
LGTM

Powered by Google App Engine
This is Rietveld