Remove data decoder use in declarative net request ruleset indexing [chromium/src : main]
0 views
Skip to first unread message
Daniel Cheng (Gerrit)
Jul 30, 2026, 1:31:53 PM (16 hours ago) Jul 30
to Daniel Cheng, Reilly Grant, chromium...@chromium.org, Hans Wennborg, Nate Chapin, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, chromeos-kio...@google.com, feature-me...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, mac-r...@chromium.org, sloboda...@chromium.org, tommyw+w...@chromium.org, video-networking...@google.com, zol...@webkit.org, extension...@chromium.org, chromium-a...@chromium.org
Attention needed from Reilly Grant
Daniel Cheng voted and added 3 comments![]( "Open in Gerrit")
Votes added by Daniel Cheng
| Commit-Queue | +1 |
3 comments
File extensions/browser/api/declarative_net_request/file_backed_ruleset_source.cc
Line 253, Patchset 25 (Parent):
base::ElapsedTimer timer;Daniel Cheng . resolved
I'm not sure if all the timers still measure the exact same thing; in particular, this timer didn't used to include json parsing time...
Line 313, Patchset 25 (Latest):
base::ElapsedTimer timer;Daniel Cheng . resolved
Though I guess it's a bit of a mixed bag, because it did include JSON parsing time previously on the "unsafe" path so you could reasonably argue that it's just unifying things.
Line 380, Patchset 25 (Latest):
if (!base::PathExists(json_path_)) {Daniel Cheng . resolved
This is a little optimization that jetski really really wanted so I didn't try ot fight it :)
Related details
Attention is currently required from:
- Reilly Grant
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reilly Grant (Gerrit)
Jul 30, 2026, 3:48:08 PM (14 hours ago) Jul 30
to Daniel Cheng, Reilly Grant, chromium...@chromium.org, Hans Wennborg, Nate Chapin, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, chromeos-kio...@google.com, feature-me...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, mac-r...@chromium.org, sloboda...@chromium.org, tommyw+w...@chromium.org, video-networking...@google.com, zol...@webkit.org, extension...@chromium.org, chromium-a...@chromium.org
Attention needed from Daniel Cheng
Reilly Grant voted and added 1 comment![]( "Open in Gerrit")
Votes added by Reilly Grant
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Daniel Cheng
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Jul 30, 2026, 4:47:20 PM (13 hours ago) Jul 30
to Daniel Cheng, Reilly Grant, chromium...@chromium.org, Hans Wennborg, Nate Chapin, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, chromeos-kio...@google.com, feature-me...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, mac-r...@chromium.org, sloboda...@chromium.org, tommyw+w...@chromium.org, video-networking...@google.com, zol...@webkit.org, extension...@chromium.org, chromium-a...@chromium.org
Daniel Cheng voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jul 30, 2026, 4:51:12 PM (13 hours ago) Jul 30
to Daniel Cheng, Reilly Grant, chromium...@chromium.org, Hans Wennborg, Nate Chapin, android-bu...@system.gserviceaccount.com, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, chromeos-kio...@google.com, feature-me...@chromium.org, gavinp...@chromium.org, loading...@chromium.org, mac-r...@chromium.org, sloboda...@chromium.org, tommyw+w...@chromium.org, video-networking...@google.com, zol...@webkit.org, extension...@chromium.org, chromium-a...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
26 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Remove data decoder use in declarative net request ruleset indexing
`base::JSONReader` is backed by a memory-safe parser, so using a
sandboxed data decoder process is no longer necessary–in fact, all the
former entry points to the data decoder's JSON parser simply
synchronously parse on the calling thread and then post a task to the
same sequence to report the result.
While converting everything to be synchronous would be straightforward,
this CL preserves the async path where it already exists today: the
95%ile time on Extensions.DeclarativeNetRequest.IndexAndPersistRulesTime
in the seconds range. To avoid blocking a task runner continuously for
seconds at a time, the async paths process rulesets for up to 50
milliseconds before yielding and posting a task to continue where it
left off.
Despite still having some asynchronous control flows, this still
significantly simplifies the code overall, e.g. `RulesetInfo` no longer
needs to persist whether or not the indexing was successful in a field,
as the actual parsing/indexing path for an individual rulset is fully
synchronous.
Bug: 40538050
Bug: 459848478
Change-Id: Ifcec8d56f51e470e277efec46171ecff499efff1
Reviewed-by: Reilly Grant <rei...@chromium.org>
Commit-Queue: Daniel Cheng <dch...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1671388}
Files:
- M extensions/browser/api/declarative_net_request/constants.h
- M extensions/browser/api/declarative_net_request/declarative_net_request_api.cc
- M extensions/browser/api/declarative_net_request/file_backed_ruleset_source.cc
- M extensions/browser/api/declarative_net_request/file_backed_ruleset_source.h
- M extensions/browser/api/declarative_net_request/file_sequence_helper.cc
- M extensions/browser/api/declarative_net_request/file_sequence_helper.h
- M extensions/browser/api/declarative_net_request/file_sequence_helper_unittest.cc
- M extensions/browser/api/declarative_net_request/install_index_helper.cc
- M extensions/browser/api/declarative_net_request/install_index_helper.h
- M extensions/browser/api/declarative_net_request/test_utils.cc
- M extensions/browser/unpacked_installer.cc
Change size: L
Delta: 11 files changed, 272 insertions(+), 458 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Reilly Grant
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages