Subresource filter may be disabled on the first page
Reported by
ivafa...@gmail.com,
Feb 28 2018
|
|||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 YaBrowser/18.3.1.349 Yowser/2.5 Safari/537.36 Steps to reproduce the problem: Subresource filter may skip the first page due to race condition. In order to reproduce the issue add a |sleep| function call inside |CreateOrOpenHelper::RunWork| for an indexed ruleset file. A sequence of events for ruleset file setup is: 1. |RulesetService::OpenAndPublishRuleset| posts a |CreateOrOpen| task from UI thread to BP on browser start. 2. |RulesetService| receives a result of a |CreateOrOpen| task in |RulesetService::OnOpenedRuleset| on UI thread and calls (with a long callstack) |VerifiedRulesetDealer::Handle::SetRulesetFile| 3. |VerifiedRulesetDealer::Handle::SetRulesetFile| is called on UI thread. It posts |VerifiedRulesetDealer::SetRulesetFile| task to BP. i.e. UI (start) -> BP (open file) -> UI (delegating) -> BP (setup dealer) A sequence of events for checking of ruleset file is: 1. |AsyncDocumentSubresourceFilter| is created on navigation response processing on UI thread. It posts |Core::Initialize| task to BP. 2. |AsyncDocumentSubresourceFilter::Core::Initialize| checks dealer the verified dealer data. i.e. UI (nav. response) -> BP (check dealer) Navigation response on UI is always happened after the start. If navigation response happens before the open file result delivered to UI thread the first page will not get subresource filter feature. Otherwise subresource filter works properly. What is the expected behavior? Subresource filter is enabled on the first page in the case of indexed ruleset file and related local state preference present. What went wrong? Subresource filter may be disabled on the first page. Did this work before? No Chrome version: 64.0.3282.140 Channel: n/a OS Version: 10.0 Flash Version: Shockwave Flash 28.0 r0 There is a fix proposal: https://chromium-review.googlesource.com/c/chromium/src/+/939161
,
Feb 28 2018
ivafanas: Can you check if your yandex account has edit-bugs access?
,
Mar 2 2018
Charlie, I unintentionally failed this bug from my google account. It has not edit access.
,
Mar 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/46ea76fff211538020a5a95dfcff9a8a24e51465 commit 46ea76fff211538020a5a95dfcff9a8a24e51465 Author: Ivan Afanasyev <ivafanas@yandex-team.ru> Date: Tue Mar 06 21:40:00 2018 fix race condition on ruleset file setup Subresource filter may skip the first page due to race condition. In order to reproduce the issue add a |sleep| function call inside |CreateOrOpenHelper::RunWork| for an indexed ruleset file. A sequence of events for ruleset file setup is: 1. |RulesetService::OpenAndPublishRuleset| posts a |CreateOrOpen| task from UI thread to task schedulers blocking pool (BP) on browser start. 2. |RulesetService| receives a result of a |CreateOrOpen| task in |RulesetService::OnOpenedRuleset| on UI thread and calls (with a long callstack) |VerifiedRulesetDealer::Handle::SetRulesetFile| 3. |VerifiedRulesetDealer::Handle::SetRulesetFile| is called on UI thread. It posts |VerifiedRulesetDealer::SetRulesetFile| task to BP. i.e. UI (start) -> BP (open file) -> UI (delegating) -> BP (setup dealer) A sequence of events for checking of ruleset file is: 1. |AsyncDocumentSubresourceFilter| is created on navigation response processing on UI thread. It posts |Core::Initialize| task to BP. 2. |AsyncDocumentSubresourceFilter::Core::Initialize| checks dealer the verified dealer data. i.e. UI (nav. response) -> BP (check dealer) Navigation response on UI is always happened after the start. If navigation response happens before the open file result delivered to UI thread the first page will not get subresource filter feature. Otherwise subresource filter works properly. The proposed fix is to eliminate "UI (delegating)" stage and perform file opening and dealer setup together using |VerifiedRulesetDealer::Handle::TryOpenAndSetRulesetFile|. The fix shouldn't affect the first page open time so as |ActivationStateComputingNavigationThrottle| already waits for ruleset file opening due to operations ordering on BP. Please let me know if I should fail a bug. I haven't found this issue in the tracker. Change-Id: If54b9cdfddd8adb9b17cb6da80d8199721ff669b Bug: 817308 Reviewed-on: https://chromium-review.googlesource.com/939161 Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#541200} [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/chrome/browser/browser_process_impl.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/chrome/browser/component_updater/subresource_filter_component_installer_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/chrome/browser/subresource_filter/subresource_filter_test_harness.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/BUILD.gn [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/activation_state_computing_navigation_throttle_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/async_document_subresource_filter_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/content_ruleset_service.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/content_ruleset_service.h [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/content_ruleset_service_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/content_subresource_filter_throttle_manager_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/subframe_navigation_filtering_throttle_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/subresource_filter_safe_browsing_activation_throttle_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/verified_ruleset_dealer.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/verified_ruleset_dealer.h [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/content/browser/verified_ruleset_dealer_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/BUILD.gn [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/ruleset_service.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/ruleset_service.h [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/ruleset_service_delegate.h [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/ruleset_service_unittest.cc [modify] https://crrev.com/46ea76fff211538020a5a95dfcff9a8a24e51465/components/subresource_filter/core/browser/subresource_filter_features.cc
,
Mar 6 2018
Good work. |
|||
►
Sign in to add a comment |
|||
Comment 1 by dtapu...@chromium.org
, Feb 28 2018Status: Assigned (was: Unconfirmed)