Subresource Filter: Presubmit checks for indexed ruleset version change are ineffictive. |
||
Issue descriptionSteps- 1) Change components/url_pattern_index/flat/url_pattern_index.fbs in local branch. 2) git cl upload. 3) Expect presubmit warning/error. But don't get it. The file components/subresource_filter/core/common/PRESUBMIT.py contains checks to ensure that the indexed ruleset format version (IndexedRuleset::kIndexedFormatVersion) is incremented whenever necessary. However these checks are ineffective. Some errors: - When only components/url_pattern_index/flat/url_pattern_index.fbs is modified, the PRESUBMIT.py script won't be triggered. (Since git cl upload will look for all files named PRESUBMIT.py in folders enclosing the files in your change, up to the repository root.) - The script checks for changes to components/url_pattern_index/flat using input_api.AffectedFiles(). But input_api.AffectedFiles only contains changes in the PRESUBMIT sub-directories.
,
Aug 23
Correction: We'll still need IndexedRuleset::kIndexedFormatVersion. But the bug still stands.
,
Aug 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ed729ec90e988bede17483a1895e9225a971d3e commit 7ed729ec90e988bede17483a1895e9225a971d3e Author: Karan Bhatia <karandeepb@chromium.org> Date: Sat Aug 25 01:41:42 2018 UrlPatternIndex: Introduce format version and add correctness checks. This CL - Introduces url_pattern_index::kUrlPatternIndexFormatVersion to keep a track of the current format version of the UrlPatternIndex schema. - Adds checks to url_pattern_index clients (Declarative net request and subresource_filter) to ensure that their ruleset format versions are updated whenever kUrlPatternIndexFormatVersion is changed. - Adds presubmit checks to components/url_pattern_index to ensure that kUrlPatternIndexFormatVersion is kept updated. BUG= 877214 , 755717 Change-Id: Ib82b775e3da2112d5f17a2040ec495a7ff8e18d9 Reviewed-on: https://chromium-review.googlesource.com/1187706 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Cr-Commit-Position: refs/heads/master@{#586098} [modify] https://crrev.com/7ed729ec90e988bede17483a1895e9225a971d3e/components/subresource_filter/core/common/indexed_ruleset.cc [add] https://crrev.com/7ed729ec90e988bede17483a1895e9225a971d3e/components/url_pattern_index/PRESUBMIT.py [modify] https://crrev.com/7ed729ec90e988bede17483a1895e9225a971d3e/components/url_pattern_index/flat/url_pattern_index.fbs [modify] https://crrev.com/7ed729ec90e988bede17483a1895e9225a971d3e/components/url_pattern_index/url_pattern_index.h [modify] https://crrev.com/7ed729ec90e988bede17483a1895e9225a971d3e/extensions/browser/api/declarative_net_request/utils.cc
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0eb34855a914cd4d90326fcac3b6fa7bbdcc20b2 commit 0eb34855a914cd4d90326fcac3b6fa7bbdcc20b2 Author: Karan Bhatia <karandeepb@chromium.org> Date: Mon Aug 27 18:00:29 2018 Subresource filter: Fix presubmit script. This CL makes some fixes to the subresource filter presubmit script: - Don't take files in url_pattern_index component into account. input_api.AffectedFiles() only returns affected files in the PRESUBMIT directory. - Fix typo in new_indexed_ruleset_version computation which currently leads to an exception. BUG= 877214 Change-Id: I11a71be5747201a69bb49f422b3081b9347490bf Reviewed-on: https://chromium-review.googlesource.com/1188770 Commit-Queue: Karan Bhatia <karandeepb@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#586313} [modify] https://crrev.com/0eb34855a914cd4d90326fcac3b6fa7bbdcc20b2/components/subresource_filter/core/common/PRESUBMIT.py
,
Aug 27
|
||
►
Sign in to add a comment |
||
Comment 1 by karandeepb@chromium.org
, Aug 23Components: UI>Browser>AdFilter
Labels: -Pri-3 Pri-2
Owner: karandeepb@chromium.org
Status: Assigned (was: Untriaged)