Closed Bug 1316914 Opened 8 years ago Closed 8 years ago

runChannelListener is still too complex.

Categories

(WebExtensions :: Request Handling, defect)

51 Branch
defect
Not set
normal

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 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?
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 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 on attachment 8809896 [details] Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. https://reviewboard.mozilla.org/r/92386/#review92480
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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/67858d51f777ad68685a7d0cca0355229fe17716 Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. r=mixedpuppy
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)
https://hg.mozilla.org/integration/mozilla-inbound/rev/9220ac85e9f1789cb1a41abe23946b31080a86a9 Bug 1316914: Refactor runChannelListener to decrease complexity to a manageable level. r=mixedpuppy
Flags: needinfo?(kmaglione+bmo)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: