Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2175763002:
Integrate IndexedRuleset into DocumentSubresourceFilter. (Closed)
|
Created:
4 years, 5 months ago by pkalinnikov Modified:
4 years, 4 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, mlamouri+watch-content_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionIntegrate IndexedRuleset into DocumentSubresourceFilter.
BUG=609747
Committed: https://crrev.com/f7d7ff47d3a6f5250c3134e1b4fb45db76270de9
Cr-Commit-Position: refs/heads/master@{#408097}
Patch Set 1 #
Total comments: 35
Patch Set 2 : Address comments from engedy@ #Patch Set 3 : Add comment. #Patch Set 4 : Fix build. #Patch Set 5 : Add missing dependency. #
Total comments: 10
Patch Set 6 : Fix build and update element types mapping. #
Total comments: 2
Patch Set 7 : Allow all requests with ELEMENT_TYPE_UNSPECIFIED. #Patch Set 8 : Fix tests. #Dependent Patchsets: Messages
Total messages: 55 (41 generated)
pkalinnikov@chromium.org changed reviewers: + engedy@chromium.org
PTAL. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:20: proto::ElementType ToElementType( Please review this conversion carefully. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:74: // TODO(pkalinnikov): Check all |ancestor_document_urls| for activation bit. Or check |ancestor_document_rules| once during construction and remember the activation bit. It depends on whether this bit should be computed with the request in account.
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
Please see comments below. I'll need to take another look at element type conversions. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:24: case blink::WebURLRequest::RequestContextVideo: Also Track https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:31: case blink::WebURLRequest::RequestContextImage: Also ImageSet https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:34: case blink::WebURLRequest::RequestContextPlugin: On 2016/07/22 13:20:03, pkalinnikov wrote: > Please review this conversion carefully. According to web_url_request_util.cc:68, Embed and Object correspond to OBJECT, and Plugin to OBJECT_SUBREQUEST. This contradicts my understanding. We should verify this. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:36: case blink::WebURLRequest::RequestContextPing: Also Beacon. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:41: return proto::ELEMENT_TYPE_STYLESHEET; Let's classify XSLT as a stylesheet too. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:42: case blink::WebURLRequest::RequestContextXMLHttpRequest: EventSource and Fetch should probably be considered XHR too. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:49: } We'll have to carefully look into CSPReport, Favicon, Prefetch, Download, Manifest, Subresource. Potentially, we won't even consult the filter for these. Then Form, Hyperlink, Location and Internal correspond to various navigation types of main document loads into frames, so we will have to look at them too. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:80: const bool load_is_allowed = matcher_.IsAllowed( nit: How about just inlining this? https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:81: GURL(resourceUrl), initiator, ToElementType(request_context)); Either need to handle invalid URLs here, or verify that call sites provide a valid URL + add comment and a DCHECK. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/core/common/test_ruleset_creator.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:41: bool ok = indexer.AddUrlRule(CreateSuffixRule(suffix)); In tests, prefer failing with a fatal GTest failure, so let's make this ASSERT_TRUE(indexer....);, and enclose the call site in line 58 in ASSERT_NO_FATAL_FAILURE(...). https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:44: buffer->insert(buffer->end(), indexer.data(), Is it meaningful to append? If not, I'd suggest making this an assignment. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:60: const char* ruleset_data = reinterpret_cast<const char*>(ruleset.data()); nit: I'd suggest inlining this if it does not look too ugly. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:61: int ruleset_size = base::checked_cast<int>(ruleset.size()); nit: Please add somewhere to this function: static_assert(CHAR_BIT == 8, "Assumed char was 8 bits."); https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/core/common/test_ruleset_creator.h (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.h:27: // having the given |suffix|, and appends the ruleset data to the |buffer|. nit: Don't forget to update this. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.h:41: void CreateRulesetToDisallowURLsWithPathSuffix(base::StringPiece suffix, nit: Rename this to CreateRulesetFile for clarity.
https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:24: case blink::WebURLRequest::RequestContextVideo: On 2016/07/22 14:19:58, engedy wrote: > Also Track Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:31: case blink::WebURLRequest::RequestContextImage: On 2016/07/22 14:19:58, engedy wrote: > Also ImageSet Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:34: case blink::WebURLRequest::RequestContextPlugin: On 2016/07/22 14:19:58, engedy wrote: > On 2016/07/22 13:20:03, pkalinnikov wrote: > > Please review this conversion carefully. > > According to web_url_request_util.cc:68, Embed and Object correspond to OBJECT, > and Plugin to OBJECT_SUBREQUEST. This contradicts my understanding. We should > verify this. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:36: case blink::WebURLRequest::RequestContextPing: On 2016/07/22 14:19:58, engedy wrote: > Also Beacon. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:41: return proto::ELEMENT_TYPE_STYLESHEET; On 2016/07/22 14:19:58, engedy wrote: > Let's classify XSLT as a stylesheet too. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:42: case blink::WebURLRequest::RequestContextXMLHttpRequest: On 2016/07/22 14:19:58, engedy wrote: > EventSource and Fetch should probably be considered XHR too. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:80: const bool load_is_allowed = matcher_.IsAllowed( On 2016/07/22 14:19:58, engedy wrote: > nit: How about just inlining this? Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/core/common/test_ruleset_creator.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:41: bool ok = indexer.AddUrlRule(CreateSuffixRule(suffix)); On 2016/07/22 14:19:58, engedy wrote: > In tests, prefer failing with a fatal GTest failure, so let's make this > ASSERT_TRUE(indexer....);, and enclose the call site in line 58 in > ASSERT_NO_FATAL_FAILURE(...). Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:44: buffer->insert(buffer->end(), indexer.data(), On 2016/07/22 14:19:58, engedy wrote: > Is it meaningful to append? If not, I'd suggest making this an assignment. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:60: const char* ruleset_data = reinterpret_cast<const char*>(ruleset.data()); On 2016/07/22 14:19:58, engedy wrote: > nit: I'd suggest inlining this if it does not look too ugly. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.cc:61: int ruleset_size = base::checked_cast<int>(ruleset.size()); On 2016/07/22 14:19:58, engedy wrote: > nit: Please add somewhere to this function: > > static_assert(CHAR_BIT == 8, "Assumed char was 8 bits."); Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/core/common/test_ruleset_creator.h (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.h:27: // having the given |suffix|, and appends the ruleset data to the |buffer|. On 2016/07/22 14:19:58, engedy wrote: > nit: Don't forget to update this. Done. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/core/common/test_ruleset_creator.h:41: void CreateRulesetToDisallowURLsWithPathSuffix(base::StringPiece suffix, On 2016/07/22 14:19:58, engedy wrote: > nit: Rename this to CreateRulesetFile for clarity. Done.
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:74: // TODO(pkalinnikov): Check all |ancestor_document_urls| for activation bit. On 2016/07/22 13:20:03, pkalinnikov wrote: > Or check |ancestor_document_rules| once during construction and remember the > activation bit. It depends on whether this bit should be computed with the > request in account. Added comment. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:81: GURL(resourceUrl), initiator, ToElementType(request_context)); On 2016/07/22 14:19:58, engedy wrote: > Either need to handle invalid URLs here, or verify that call sites provide a > valid URL + add comment and a DCHECK. The |matcher_| already takes care of invalid GURLs.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
Patchset #5 (id:100001) has been deleted
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_gyp_rel on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gyp_...)
LGTM % comments. https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:81: GURL(resourceUrl), initiator, ToElementType(request_context)); On 2016/07/25 11:47:31, pkalinnikov wrote: > On 2016/07/22 14:19:58, engedy wrote: > > Either need to handle invalid URLs here, or verify that call sites provide a > > valid URL + add comment and a DCHECK. > > The |matcher_| already takes care of invalid GURLs. Yes, it has a DCHECK, but then we still need to verify that the DCHECK codifies a precondition that is actually met. As a third option, we can remove the DCHECK from there, and declare that the method is completely happy to filter invalid URLs. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:33: return proto::ELEMENT_TYPE_OBJECT; As discussed offline, we normally cannot distinguish between the main plugin resource, and any other loads it makes. Should we map, during indexing, ELEMENT_TYPE_OBJECT_SUBRESOURCE to ELEMENT_TYPE_OBJECT in the filtering rules so that either of those rules will apply to all plugin-related loads? https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:44: case blink::WebURLRequest::RequestContextFrame: Also add here: RequestContextForm RequestContextHyperlink RequestContextLocation RequestContextInternal https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:47: case blink::WebURLRequest::RequestContextImport: Move Import to "no filter" list. In fact, theoretically, the fitter will never be consulted for Import unless for Prefetches. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:59: return proto::ELEMENT_TYPE_OTHER; Let's return OTHER for: RequestContextPrefetch RequestContextSubresource https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:59: return proto::ELEMENT_TYPE_OTHER; Let's be conservative and return a value that will result in no filtering being performed later for the following: default RequestContextUnspecified RequestContextCSPReport RequestContextDownload RequestContextManifest
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:81: GURL(resourceUrl), initiator, ToElementType(request_context)); On 2016/07/25 14:06:38, engedy wrote: > On 2016/07/25 11:47:31, pkalinnikov wrote: > > On 2016/07/22 14:19:58, engedy wrote: > > > Either need to handle invalid URLs here, or verify that call sites provide a > > > valid URL + add comment and a DCHECK. > > > > The |matcher_| already takes care of invalid GURLs. > > Yes, it has a DCHECK, but then we still need to verify that the DCHECK codifies > a precondition that is actually met. As a third option, we can remove the DCHECK > from there, and declare that the method is completely happy to filter invalid > URLs. Wait, I'm talking about IndexedRulesetMatcher::IsAllowed. It has no a DCHECK, the first 2 lines are: if (!url.is_valid()) return true; https://codereview.chromium.org/2175763002/diff/120001/components/subresource... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:33: return proto::ELEMENT_TYPE_OBJECT; On 2016/07/25 14:06:38, engedy wrote: > As discussed offline, we normally cannot distinguish between the main plugin > resource, and any other loads it makes. > > Should we map, during indexing, ELEMENT_TYPE_OBJECT_SUBRESOURCE to > ELEMENT_TYPE_OBJECT in the filtering rules so that either of those rules will > apply to all plugin-related loads? Done. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:44: case blink::WebURLRequest::RequestContextFrame: On 2016/07/25 14:06:38, engedy wrote: > Also add here: > RequestContextForm > RequestContextHyperlink > RequestContextLocation > RequestContextInternal Done. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:47: case blink::WebURLRequest::RequestContextImport: On 2016/07/25 14:06:38, engedy wrote: > Move Import to "no filter" list. In fact, theoretically, the fitter will never > be consulted for Import unless for Prefetches. Done. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:59: return proto::ELEMENT_TYPE_OTHER; On 2016/07/25 14:06:38, engedy wrote: > Let's be conservative and return a value that will result in no filtering being > performed later for the following: > > default > RequestContextUnspecified > RequestContextCSPReport > RequestContextDownload > RequestContextManifest Done. https://codereview.chromium.org/2175763002/diff/120001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:59: return proto::ELEMENT_TYPE_OTHER; On 2016/07/25 14:06:38, engedy wrote: > Let's return OTHER for: > > RequestContextPrefetch > RequestContextSubresource Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM % one negation. Thanks! https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/1/components/subresource_filt... components/subresource_filter/content/renderer/document_subresource_filter.cc:81: GURL(resourceUrl), initiator, ToElementType(request_context)); On 2016/07/25 17:34:54, pkalinnikov wrote: > On 2016/07/25 14:06:38, engedy wrote: > > On 2016/07/25 11:47:31, pkalinnikov wrote: > > > On 2016/07/22 14:19:58, engedy wrote: > > > > Either need to handle invalid URLs here, or verify that call sites provide > a > > > > valid URL + add comment and a DCHECK. > > > > > > The |matcher_| already takes care of invalid GURLs. > > > > Yes, it has a DCHECK, but then we still need to verify that the DCHECK > codifies > > a precondition that is actually met. As a third option, we can remove the > DCHECK > > from there, and declare that the method is completely happy to filter invalid > > URLs. > > Wait, I'm talking about IndexedRulesetMatcher::IsAllowed. It has no a DCHECK, > the first 2 lines are: > > if (!url.is_valid()) > return true; Oh, right, I didn't noticed that. Ack. https://codereview.chromium.org/2175763002/diff/140001/components/subresource... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/140001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:104: ToElementType(request_context))) { Hang on, ELEMENT_TYPE_UNSPECIFIED matches all and any element type filters, we would want to opposite, right?
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
https://codereview.chromium.org/2175763002/diff/140001/components/subresource... File components/subresource_filter/content/renderer/document_subresource_filter.cc (right): https://codereview.chromium.org/2175763002/diff/140001/components/subresource... components/subresource_filter/content/renderer/document_subresource_filter.cc:104: ToElementType(request_context))) { On 2016/07/25 19:47:01, engedy wrote: > Hang on, ELEMENT_TYPE_UNSPECIFIED matches all and any element type filters, we > would want to opposite, right? Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM, thank you!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pkalinnikov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
pkalinnikov@chromium.org changed reviewers: + jochen@chromium.org
Hi Jochen, Can you please review this? Adding you because I introduced a dependency in chrome/browser/BUILD.gn and chrome/chrome_tests.gypi.
lgtm
The CQ bit was checked by pkalinnikov@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from engedy@chromium.org Link to the patchset: https://codereview.chromium.org/2175763002/#ps180001 (title: "Fix tests.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #8 (id:180001)
Message was sent while issue was closed.
Description was changed from ========== Integrate IndexedRuleset into DocumentSubresourceFilter. BUG=609747 ========== to ========== Integrate IndexedRuleset into DocumentSubresourceFilter. BUG=609747 Committed: https://crrev.com/f7d7ff47d3a6f5250c3134e1b4fb45db76270de9 Cr-Commit-Position: refs/heads/master@{#408097} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/f7d7ff47d3a6f5250c3134e1b4fb45db76270de9 Cr-Commit-Position: refs/heads/master@{#408097} |
