Closed
Bug 1316914
Opened 8 years ago
Closed 8 years ago
runChannelListener is still too complex.
Categories
(WebExtensions :: Request Handling, defect)
Tracking
(firefox52 fixed)
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
Details
Attachments
(1 file)
At least according to ESLint. And I'm inclined to agree.
Comment hidden (mozreview-request) |
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92388/#review92460 ::: toolkit/modules/addons/WebRequest.jsm:612 (Diff revision 1) > - } > - > - if (!commonData) { > - commonData = { > - requestId: RequestId.get(channel), > + requestId: RequestId.get(channel), > - url: uri.spec, > + url: channel.URI.spec.spec, is that right? .spec.spec? ::: toolkit/modules/addons/WebRequest.jsm:630 (Diff revision 1) > - } > + } > > - let {isSystemPrincipal} = Services.scriptSecurityManager; > + let {isSystemPrincipal} = Services.scriptSecurityManager; > > - commonData.isSystemPrincipal = (isSystemPrincipal(loadInfo.triggeringPrincipal) || > + data.isSystemPrincipal = (isSystemPrincipal(loadInfo.triggeringPrincipal) || > - isSystemPrincipal(loadInfo.loadingPrincipal)); > + isSystemPrincipal(loadInfo.loadingPrincipal)); If triggeringPrincipal is supposed to always be defined, do we need to check laodingPrincipal any longer?
Assignee | ||
Comment 3•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92388/#review92460 > is that right? .spec.spec? Nope. I think I accidentally hit '.' before I saved :/ > If triggeringPrincipal is supposed to always be defined, do we need to check laodingPrincipal any longer? `loadingPrincipal` and `triggeringPrincipal` are required to be defined, and that's enforced by assertions in Necko code. If `triggeringPrincipal` is not provided, it automatically falls back to `loadingPrincipal`.
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92386/#review92478 Fix the spec issue, then go for it.
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92386/#review92480
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92388/#review92488 per last comment, fix spec, then r+
Attachment #8809896 -
Flags: review?(mixedpuppy) → review+
Assignee | ||
Comment 7•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/67858d51f777ad68685a7d0cca0355229fe17716 Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. r=mixedpuppy
Comment 8•8 years ago
|
||
Backed out bug 1316914, bug 1316784 and bug 1305217 for frequent timeouts in test_ext_webrequest_upload.html: Bug 1305217: https://hg.mozilla.org/integration/mozilla-inbound/rev/217ae1d5285328611de99e2e48042b19751dbb54 https://hg.mozilla.org/integration/mozilla-inbound/rev/6cbde9c5e058999ba95319391b59a7d67649ca53 Bug 1316784: https://hg.mozilla.org/integration/mozilla-inbound/rev/9381a00d19f81c21114f9d24cafee791fa0d9dbe Bug 1316914: https://hg.mozilla.org/integration/mozilla-inbound/rev/ddec56f2c469512ba99e424540f6f77a447fdea0 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=39104153&repo=mozilla-inbound 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Binary upload size matches - Expected: 16, Actual: 16 21:16:58 INFO - onCompleted 32 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=16+bytes&enctype=multipart%2Fform-data&xhr=1 21:16:58 INFO - onBeforeRequest 33 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded 21:16:58 INFO - onUpload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded {"formData":{"textInput":["value1","value2"]}} 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Intercepted upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded #33 {"textInput":["value1","value2"]} have a requestBody 21:16:58 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22textInput%22%3A%5B%22value1%22%2C%22value2%22%5D%7D&enctype=application%2Fx-www-form-urlencoded #33 matches form data. - Expected: {"textInput":["value1","value2"]}, Actual: {"textInput":["value1","value2"]} 21:16:58 INFO - Buffered messages finished 21:16:58 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Test timed out. 21:16:58 INFO - reportError@SimpleTest/TestRunner.js:114:7 21:16:58 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:135:7
Flags: needinfo?(kmaglione+bmo)
Assignee | ||
Comment 9•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9220ac85e9f1789cb1a41abe23946b31080a86a9 Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. r=mixedpuppy
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(kmaglione+bmo)
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9220ac85e9f1
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•