Closed Bug 1233644 Opened 9 years ago Closed 8 years ago

use pattern matching when listening clear-origin-data in ServiceWorkerManager

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: allstars.chh, Assigned: allstars.chh)

References

Details

Attachments

(1 file, 4 obsolete files)

See Honza's comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1233136#c0

"ServiceWorkerManager [2], bug 1191647, that doesn't build OriginAttributesPattern from aData"
Attached patch WIP Patch (obsolete) — Splinter Review
WIP, still have problems on writing tests.
Attached patch Patch. (obsolete) — Splinter Review
Thanks to :kanru's help, I get the tests running. :D
Attachment #8699940 - Attachment is obsolete: true
Attached patch Patch (obsolete) — Splinter Review
fixed some try failure
Attachment #8700559 - Attachment is obsolete: true
Attached patch Patch. (obsolete) — Splinter Review
Attachment #8700931 - Attachment is obsolete: true
Comment on attachment 8700932 [details] [diff] [review]
Patch.

Review of attachment 8700932 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Ben
The handling of clear-origin-data landed in Bug 1191647 has some problems,
for example, when the data is {appId: 1} in clear-origin-data,
we should clear all registrations for appId = 1, whether the inBrowser flag is true or not.
Right now the code will only clear the registration {appId: 1, inBrowser: false}

Also test is included, 
in the test I need a mozbrowser frame which needs 'browser' permission,
to prevent adding more permissions into the original app,
so I create another test app, which is copied the test app from dom/workers/test/serviceworkers/app/

Could you review this for me?

Thanks
Attachment #8700932 - Flags: review?(bkelly)
Comment on attachment 8700932 [details] [diff] [review]
Patch.

Review of attachment 8700932 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I'm buried trying to get a beta crasher fixed before going on holiday.  Andrea, can you take this review?

::: dom/workers/test/serviceworkers/app3/sw.js
@@ +1,1 @@
> +// Useless service worker.

Please use: dom/workers/test/serviceworkers/empty.js
Attachment #8700932 - Flags: review?(bkelly) → review?(amarchesini)
Attachment #8700932 - Flags: review?(amarchesini) → review+
Attached patch Patch. v2Splinter Review
use empty.js
Attachment #8700932 - Attachment is obsolete: true
Attachment #8704550 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/b53aa421ca19
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: