The AdTracker could use some cleanup. 1) We can remove the ExecutingScript struct. 2) We can remove SubresourceFilter::IsFilterAssociatedWithAdSubframe
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c926d3bb2f3b7d166c59aa2bb4fde82eb351a5b3 commit c926d3bb2f3b7d166c59aa2bb4fde82eb351a5b3 Author: Josh Karlin <jkarlin@chromium.org> Date: Wed Oct 17 20:51:40 2018 [AdTagging] Simplify the pseudo-stack down to a vector of bools This is a cleanup CL. All we're using in the stack is the bool value, so we can remove the struct. Bug: 894505 Change-Id: Id7511420e860c21a1dc84ce5329942eb3bfeb90f Reviewed-on: https://chromium-review.googlesource.com/c/1276686 Commit-Queue: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#600547} [modify] https://crrev.com/c926d3bb2f3b7d166c59aa2bb4fde82eb351a5b3/third_party/blink/renderer/core/frame/ad_tracker.cc [modify] https://crrev.com/c926d3bb2f3b7d166c59aa2bb4fde82eb351a5b3/third_party/blink/renderer/core/frame/ad_tracker.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3ca45225bd6858c8fb280db1ad3884c7b0033e1 commit d3ca45225bd6858c8fb280db1ad3884c7b0033e1 Author: Josh Karlin <jkarlin@chromium.org> Date: Thu Oct 18 19:08:33 2018 [AdTagging] Additional check for ad frame in WillSendRequest When checking if a resource request is an ad or not, also check if the frame requesting the resource is considered an ad frame. Note that this is redundant at the moment because the SubresourceFilter already does this on its inspection of the request but the plan is to remove the SubresourceFilter logic for that in a follow-up CL. Bug: 894505 Change-Id: Ib85fa44ed74a59e090e9ae893090d1c99d7c71e6 Reviewed-on: https://chromium-review.googlesource.com/c/1288850 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#600854} [modify] https://crrev.com/d3ca45225bd6858c8fb280db1ad3884c7b0033e1/third_party/blink/renderer/core/frame/ad_tracker.cc [modify] https://crrev.com/d3ca45225bd6858c8fb280db1ad3884c7b0033e1/third_party/blink/renderer/core/frame/ad_tracker_test.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c66a5db10063f1dcdc9f61ac3578682bc324398 commit 7c66a5db10063f1dcdc9f61ac3578682bc324398 Author: Josh Karlin <jkarlin@chromium.org> Date: Fri Oct 19 01:01:22 2018 [AdTagging] Optimize IsAdScriptInStack When scanning the stack we're really just checking if any element in the vector is true. We can replace this O(n) check with O(1) if we keep track of the number of true elements in the vector. Bug: 894505 Change-Id: I378cffe80212277a7fa8ad414e15a5cf3c9824c8 Reviewed-on: https://chromium-review.googlesource.com/c/1289329 Commit-Queue: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#601002} [modify] https://crrev.com/7c66a5db10063f1dcdc9f61ac3578682bc324398/third_party/blink/renderer/core/frame/ad_tracker.cc [modify] https://crrev.com/7c66a5db10063f1dcdc9f61ac3578682bc324398/third_party/blink/renderer/core/frame/ad_tracker.h [modify] https://crrev.com/7c66a5db10063f1dcdc9f61ac3578682bc324398/third_party/blink/renderer/core/frame/ad_tracker_test.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43dcdf8ccdded58773303240980daa5d79179dec commit 43dcdf8ccdded58773303240980daa5d79179dec Author: Josh Karlin <jkarlin@chromium.org> Date: Wed Oct 31 13:18:30 2018 [AdTagging] Remove SubresourceFilter::IsFilterAssociatedWithAdSubframe Since LocalFrame already has a IsAdSubframe() method we don't need to also have this information in the SubresourceFilter. This is a behavior change for workers, because we'll no longer tag ad resources in workers but we weren't actually doing anything with that information. If we want that information in the future, we can add a similar IsAdWorker method on the ExecutionContext itself. Bug: 894505 Change-Id: I17cdc258d42169679a205993123b5026e76b2b89 Reviewed-on: https://chromium-review.googlesource.com/c/1277546 Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#604228} [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/components/subresource_filter/content/renderer/subresource_filter_agent.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/components/subresource_filter/content/renderer/subresource_filter_agent_unittest.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/components/subresource_filter/content/renderer/web_document_subresource_filter_impl.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/components/subresource_filter/content/renderer/web_document_subresource_filter_impl.h [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/content/shell/test_runner/mock_web_document_subresource_filter.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/content/shell/test_runner/mock_web_document_subresource_filter.h [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/third_party/blink/public/platform/web_document_subresource_filter.h [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/third_party/blink/renderer/core/exported/web_document_subresource_filter_test.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/third_party/blink/renderer/core/loader/frame_fetch_context_test.cc [modify] https://crrev.com/43dcdf8ccdded58773303240980daa5d79179dec/third_party/blink/renderer/core/loader/subresource_filter.cc
Comment 1 by bugdroid1@chromium.org
, Oct 17