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 |
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
•