Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(272)

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

Created:
3 years, 10 months ago by pkalinnikov
Modified:
3 years, 10 months ago
Reviewers:
engedy, dcheng
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

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.
3 years, 10 months 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: ...
3 years, 10 months 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: > ...
3 years, 10 months ago (2017-02-17 17:16:48 UTC) #19
engedy
Awesome, thanks, LGTM.
3 years, 10 months 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 ...
3 years, 10 months 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 ...
3 years, 10 months ago (2017-02-17 18:01:06 UTC) #26
dcheng
rs lgtm for #include path change in ipc
3 years, 10 months 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
3 years, 10 months ago (2017-02-17 20:03:11 UTC) #31
commit-bot: I haz the power
3 years, 10 months 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...

Powered by Google App Engine
This is Rietveld 408576698