Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2697363005: Move DocumentSubresourceFilter to core/common. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 1 week ago by pkalinnikov
Modified:
4 months, 1 week ago
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, subresource-filter-reviews_chromium.org, droger+watchlist_chromium.org, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, jam, darin-cc_chromium.org, Charlie Harrison
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move DocumentSubresourceFilter to core/common. This removes the DocumentSubresourceFilter's dependency on Blink, and will allow DSF to be used both from the renderer and the browser side without violating DEPS rules. This CL also adds WebDocumentSubresourceFilterImpl, a render-side wrapper around DSF that implements blink's WebDocumentSubresourceFilter interface. Basically, this class is equivalent to the DocumentSubresourceFilter class prior to this CL. BUG=637415 TBR=dcheng@chromium.org Review-Url: https://codereview.chromium.org/2697363005 Cr-Commit-Position: refs/heads/master@{#451375} Committed: https://chromium.googlesource.com/chromium/src/+/666b9969d62eaae9e2f097ae92ccbaae82046b83

Patch Set 1 #

Patch Set 2 : Clean up. #

Total comments: 22

Patch Set 3 : Fix tests. #

Patch Set 4 : Address comments from engedy@. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+287 lines, -951 lines) Patch
M components/subresource_filter/content/browser/DEPS View 1 chunk +0 lines, -1 line 0 comments Download
M components/subresource_filter/content/browser/content_subresource_filter_driver_factory.h View 1 chunk +1 line, -1 line 0 comments Download
M components/subresource_filter/content/common/BUILD.gn View 2 chunks +1 line, -7 lines 0 comments Download
D components/subresource_filter/content/common/DEPS View 1 chunk +0 lines, -3 lines 0 comments Download
D components/subresource_filter/content/common/document_load_statistics.h View 1 chunk +0 lines, -35 lines 0 comments Download
D components/subresource_filter/content/common/document_subresource_filter.h View 1 chunk +0 lines, -105 lines 0 comments Download
D components/subresource_filter/content/common/document_subresource_filter.cc View 1 chunk +0 lines, -228 lines 0 comments Download
D components/subresource_filter/content/common/document_subresource_filter_unittest.cc View 1 chunk +0 lines, -338 lines 0 comments Download
M components/subresource_filter/content/common/subresource_filter_messages.h View 1 chunk +1 line, -1 line 0 comments Download
M components/subresource_filter/content/renderer/BUILD.gn View 2 chunks +3 lines, -0 lines 0 comments Download
M components/subresource_filter/content/renderer/subresource_filter_agent.h View 1 2 3 2 chunks +4 lines, -6 lines 0 comments Download
M components/subresource_filter/content/renderer/subresource_filter_agent.cc View 1 2 3 5 chunks +14 lines, -10 lines 0 comments Download
M components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
A components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h View 1 2 3 1 chunk +52 lines, -0 lines 0 comments Download
A components/subresource_filter/content/renderer/web_document_subresource_filter_impl.cc View 1 2 3 1 chunk +124 lines, -0 lines 0 comments Download
M components/subresource_filter/core/common/BUILD.gn View 1 3 chunks +5 lines, -0 lines 0 comments Download
A + components/subresource_filter/core/common/document_load_statistics.h View 1 3 chunks +5 lines, -5 lines 0 comments Download
A + components/subresource_filter/core/common/document_subresource_filter.h View 1 2 3 4 chunks +26 lines, -30 lines 0 comments Download
A + components/subresource_filter/core/common/document_subresource_filter.cc View 1 2 3 4 chunks +18 lines, -101 lines 0 comments Download
A + components/subresource_filter/core/common/document_subresource_filter_unittest.cc View 1 2 3 4 chunks +32 lines, -79 lines 0 comments Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 34 (25 generated)
pkalinnikov
Balazs, PTAL. Charlie, FIY. I will be able to land https://codereview.chromium.org/2683413003/ only after this CL.
4 months, 1 week ago (2017-02-17 15:35:07 UTC) #10
engedy
LGTM, huge kudos for refactoring this! https://codereview.chromium.org/2697363005/diff/20001/components/subresource_filter/content/renderer/subresource_filter_agent.cc File components/subresource_filter/content/renderer/subresource_filter_agent.cc (left): https://codereview.chromium.org/2697363005/diff/20001/components/subresource_filter/content/renderer/subresource_filter_agent.cc#oldcode8 components/subresource_filter/content/renderer/subresource_filter_agent.cc:8: #include "base/memory/ref_counted.h" nit: ...
4 months, 1 week ago (2017-02-17 16:44:05 UTC) #17
pkalinnikov
Addressed. https://codereview.chromium.org/2697363005/diff/20001/components/subresource_filter/content/renderer/subresource_filter_agent.cc File components/subresource_filter/content/renderer/subresource_filter_agent.cc (left): https://codereview.chromium.org/2697363005/diff/20001/components/subresource_filter/content/renderer/subresource_filter_agent.cc#oldcode8 components/subresource_filter/content/renderer/subresource_filter_agent.cc:8: #include "base/memory/ref_counted.h" On 2017/02/17 16:44:04, engedy wrote: > ...
4 months, 1 week ago (2017-02-17 17:16:48 UTC) #19
engedy
Awesome, thanks, LGTM.
4 months, 1 week ago (2017-02-17 17:17:40 UTC) #21
pkalinnikov
dcheng@: Daniel, I added you to TBR because the change in subresource_filter_messages.h is a trivial ...
4 months, 1 week ago (2017-02-17 17:40:59 UTC) #24
pkalinnikov
On 2017/02/17 17:40:59, pkalinnikov wrote: > dcheng@: Daniel, I added you to TBR because the ...
4 months, 1 week ago (2017-02-17 18:01:06 UTC) #26
dcheng (OOO Jun 25 - Jul 1)
rs lgtm for #include path change in ipc
4 months, 1 week ago (2017-02-17 18:03:03 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697363005/60001
4 months, 1 week ago (2017-02-17 20:03:11 UTC) #31
commit-bot: I haz the power
4 months, 1 week ago (2017-02-17 20:59:28 UTC) #34
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/666b9969d62eaae9e2f097ae92cc...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318