Closed
Bug 1397536
Opened 7 years ago
Closed 7 years ago
Accept URL strings in MatchPattern methods
Categories
(WebExtensions :: Request Handling, enhancement)
WebExtensions
Request Handling
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
This turns out to be another big hidden chunk of overhead in webRequest processing. Creating URIs is always expensive, but the XPConnect overhead of doing it from the JS side is more expensive than creating the URIs themselves. Accepting URL strings directly in the match methods improves things considerably.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8905310 [details] Bug 1397536: Avoid newURI overhead for MatchPattern. https://reviewboard.mozilla.org/r/177108/#review182094 Another approach could be to pass through the URIs from WebRequest.jsm to ext-webrequest, and only convert them to urls prior to calling the listener. Then we'd avoid having to convert back to nsiuri. Having matches accept either is good too.
Attachment #8905310 -
Flags: review?(mixedpuppy) → review+
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8905310 [details] Bug 1397536: Avoid newURI overhead for MatchPattern. https://reviewboard.mozilla.org/r/177108/#review182094 Hm. You're right, re-using the existing nsIURIs is considerably faster.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8905310 -
Flags: review+ → review?(mixedpuppy)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8905310 [details] Bug 1397536: Avoid newURI overhead for MatchPattern. https://reviewboard.mozilla.org/r/177108/#review182108 ::: toolkit/modules/addons/WebRequest.jsm:677 (Diff revision 2) > > getRequestData(channel, extraData) { > let data = { > requestId: String(channel.id), > url: channel.finalURL, > + uri: channel.finalURI, can we uppercase URI? it's hard on the eyes especially next to url. Also wish finalURL was finalUrl.
Attachment #8905310 -
Flags: review?(mixedpuppy) → review+
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #5) > Also wish finalURL was finalUrl. So do I, but it's a public API at this point. :(
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8905310 [details] Bug 1397536: Avoid newURI overhead for MatchPattern. https://reviewboard.mozilla.org/r/177108/#review182130 Great find!
Attachment #8905310 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 8•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7282bbabab15133686eef40434da0940a9244cea Bug 1397536: Avoid newURI overhead for MatchPattern. r=ehsan,mixedpuppy
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7282bbabab15
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Updated•7 years ago
|
Blocks: webext-perf
Comment 10•7 years ago
|
||
Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(kmaglione+bmo)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(kmaglione+bmo) → qe-verify-
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•