Closed
Bug 1130101
Opened 10 years ago
Closed 10 years ago
Implement support for Service-Worker-Allowed header and associated rules
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: nsm, Assigned: ehsan.akhgari)
References
Details
Attachments
(3 files)
3.93 KB,
patch
|
nsm
:
review+
|
Details | Diff | Splinter Review |
7.20 KB,
patch
|
nsm
:
review+
|
Details | Diff | Splinter Review |
6.86 KB,
patch
|
nsm
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•10 years ago
|
Blocks: ServiceWorkers-B2G
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8602152 -
Flags: review?(nsm.nikhil)
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8602153 -
Flags: review?(nsm.nikhil)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8602154 -
Flags: review?(nsm.nikhil)
Reporter | ||
Updated•10 years ago
|
Attachment #8602152 -
Flags: review?(nsm.nikhil) → review+
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8602153 [details] [diff] [review]
Part 2: Honor the Service-Worker-Allowed header when prefix matching the service worker scope
Review of attachment 8602153 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/workers/ServiceWorkerManager.cpp
@@ +485,5 @@
>
> namespace {
> +enum ScopeStringPrefixMode {
> + eUseDirectory,
> + eUsePath
Nit: Could you add a comment that this enum is used to distinguish between a unset SWAllowed header and a set header. The directory/path difference isn't very obvious. Best would be quoting parts of the spec verbatim.
@@ +500,5 @@
>
> + if (aPrefixMode == eUseDirectory) {
> + nsCOMPtr<nsIURL> scriptURL(do_QueryInterface(aScriptURI));
> + if (NS_WARN_IF(!scriptURL)) {
> + return rv;
Nit: rv is not affected by the QI. NS_ERROR_FAILURE?
Attachment #8602153 -
Flags: review?(nsm.nikhil) → review+
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8602154 [details] [diff] [review]
Part 3: Add unit tests for the handling of the Service-Worker-Allowed header
Review of attachment 8602154 [details] [diff] [review]:
-----------------------------------------------------------------
The actual worker files are missing from the patch! Please add them before landing.
::: dom/workers/test/serviceworkers/test_service_worker_allowed.html
@@ +63,5 @@
> + }
> +
> + SimpleTest.waitForExplicitFinish();
> + SpecialPowers.pushPrefEnv({"set": [
> + ["dom.serviceWorkers.enabled", true],
also set exemptFromPerDomainMax
Attachment #8602154 -
Flags: review?(nsm.nikhil) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #5)
> Comment on attachment 8602154 [details] [diff] [review]
> Part 3: Add unit tests for the handling of the Service-Worker-Allowed header
>
> Review of attachment 8602154 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> The actual worker files are missing from the patch! Please add them before
> landing.
They're there, but Splinter doesn't display empty files!
FWIW this patch touches code added in bug 1154494, so in order to not make your life more difficult than it already is, I'll wait for that bug to land first. Please let me know if you don't expect that to happen soon-ish.
Depends on: 1154494
https://hg.mozilla.org/mozilla-central/rev/7409a86dcf41
https://hg.mozilla.org/mozilla-central/rev/78224d2bdde4
https://hg.mozilla.org/mozilla-central/rev/db23832bc281
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•