Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Issue 835673006: Use caller's document url to resolve scriptURL/patternURL in registerServiceWorker/getRegistration (Closed)

Created:
5 years, 11 months ago by jungkees
Modified:
5 years, 9 months ago
CC:
blink-reviews, horo+watch_chromium.org, jsbell+serviceworker_chromium.org, kinuko+serviceworker, michaeln, nhiroki, serviceworker-reviews, tzik
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Service Worker: Use the caller's context to resolve URLs passed to register(). The scriptURL and patternURL passed to ServiceWorkerContainer.registerServiceWorker/getRegistration should be resolved relative to the caller's document url rather than the ServiceWorkerContainer's. Corresponding AUTHORS change: https://codereview.chromium.org/851103006/ BUG=449422 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=191059

Patch Set 1 #

Total comments: 3

Patch Set 2 : Use calling frame's document's url to parse script url and scope url; Add tests #

Patch Set 3 : Remove unnecessary forward class declaration - class DOMWindow; in NavigatorServiceWorker.h #

Total comments: 10

Patch Set 4 : Always use calling document's URL as a base URL for parsing script URL and scope URL. #

Patch Set 5 : Use callingExecutionContext(scriptState->isolate()) instead of callingDOMWindow(scriptState->isolat… #

Patch Set 6 : Fixed two related tests which are using iframe #

Total comments: 6

Patch Set 7 : Address comments for registration-iframe.html and rebase #

Total comments: 4

Patch Set 8 : Address comments in registration-iframe.html #

Unified diffs Side-by-side diffs Delta from patch set Stats (+115 lines, -11 lines) Patch
M LayoutTests/http/tests/serviceworker/chromium/force-refresh-registration.html View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/http/tests/serviceworker/controller-on-reload.html View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
A LayoutTests/http/tests/serviceworker/registration-iframe.html View 1 2 3 4 5 6 7 1 chunk +108 lines, -0 lines 0 comments Download
M Source/modules/serviceworkers/ServiceWorkerContainer.cpp View 1 2 3 4 5 6 5 chunks +3 lines, -7 lines 0 comments Download

Messages

Total messages: 38 (6 generated)
jungkees
Please review my first Service Worker patch! :-)
5 years, 11 months ago (2015-01-16 06:26:46 UTC) #2
dominicc (has gone to gerrit)
Hi Jungkees, thanks for your contributions on the spec side to date; it's exciting to ...
5 years, 11 months ago (2015-01-19 03:19:08 UTC) #3
jungkees
On 2015/01/19 03:19:08, dominicc wrote: > Hi Jungkees, thanks for your contributions on the spec ...
5 years, 11 months ago (2015-01-19 11:22:06 UTC) #4
dominicc (has gone to gerrit)
> After writing the related tests, I'll upload the new patch. Thanks! > FYI, https://codereview.chromium.org/851103006/ ...
5 years, 11 months ago (2015-01-20 04:04:20 UTC) #5
jungkees
On 2015/01/20 04:04:20, dominicc wrote: > > After writing the related tests, I'll upload the ...
5 years, 11 months ago (2015-01-21 13:02:06 UTC) #6
jungkees
On 2015/01/21 13:02:06, jungkees wrote: > On 2015/01/20 04:04:20, dominicc wrote: > > > After ...
5 years, 10 months ago (2015-02-05 09:00:46 UTC) #7
dominicc (has gone to gerrit)
Sorry for the slow reply; I was travelling. Back now. https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html File LayoutTests/http/tests/serviceworker/registration-iframe.html (right): https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html#newcode78 ...
5 years, 10 months ago (2015-02-09 07:31:48 UTC) #8
jungkees
On 2015/02/09 07:31:48, dominicc wrote: > Sorry for the slow reply; I was travelling. Back ...
5 years, 10 months ago (2015-02-10 05:28:22 UTC) #9
jungkees
https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html File LayoutTests/http/tests/serviceworker/registration-iframe.html (right): https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html#newcode78 LayoutTests/http/tests/serviceworker/registration-iframe.html:78: // Set scope url such that it does not ...
5 years, 10 months ago (2015-02-13 08:12:38 UTC) #10
dominicc (has gone to gerrit)
https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html File LayoutTests/http/tests/serviceworker/registration-iframe.html (right): https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html#newcode78 LayoutTests/http/tests/serviceworker/registration-iframe.html:78: // Set scope url such that it does not ...
5 years, 10 months ago (2015-02-16 06:54:45 UTC) #11
jungkees
https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html File LayoutTests/http/tests/serviceworker/registration-iframe.html (right): https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html#newcode78 LayoutTests/http/tests/serviceworker/registration-iframe.html:78: // Set scope url such that it does not ...
5 years, 10 months ago (2015-02-16 09:06:11 UTC) #12
dominicc (has gone to gerrit)
On 2015/02/16 at 09:06:11, jungkee.song wrote: > https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html > File LayoutTests/http/tests/serviceworker/registration-iframe.html (right): > > https://codereview.chromium.org/835673006/diff/40001/LayoutTests/http/tests/serviceworker/registration-iframe.html#newcode78 ...
5 years, 10 months ago (2015-02-17 01:21:58 UTC) #13
jungkees
On 2015/02/17 01:21:58, dominicc wrote: > Dereferencing null and crashing is wrong, so it is ...
5 years, 10 months ago (2015-02-17 04:50:30 UTC) #14
jungkees
In order to consider worker clients, it'd make more sense to use callingExecutionContext(scriptState->isolate()) instead of ...
5 years, 10 months ago (2015-02-17 08:57:32 UTC) #15
jungkees
On 2015/02/17 08:57:32, jungkees wrote: > In order to consider worker clients, it'd make more ...
5 years, 10 months ago (2015-02-17 09:02:27 UTC) #16
jungkees
Two regression errors are found from the tests that are using iframe. Fixed and included ...
5 years, 10 months ago (2015-02-17 09:34:46 UTC) #17
dominicc (has gone to gerrit)
On 2015/02/17 at 09:34:46, jungkee.song wrote: > Two regression errors are found from the tests ...
5 years, 10 months ago (2015-02-26 06:55:31 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/835673006/100001
5 years, 10 months ago (2015-02-26 06:56:31 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: android_blink_compile_dbg on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_dbg/builds/29630) android_blink_compile_rel on tryserver.blink (JOB_FAILED, ...
5 years, 10 months ago (2015-02-26 07:02:29 UTC) #22
falken
the CL description seems out of date, can you update it for the patch you're ...
5 years, 10 months ago (2015-02-26 07:16:17 UTC) #24
jungkees
On 2015/02/26 07:16:17, falken wrote: > the CL description seems out of date, can you ...
5 years, 10 months ago (2015-02-26 10:46:58 UTC) #25
falken
Also regarding the CLA, can you let me know about when you signed it? I ...
5 years, 9 months ago (2015-02-27 01:51:57 UTC) #26
falken
On 2015/02/27 01:51:57, falken wrote: > Also regarding the CLA, can you let me know ...
5 years, 9 months ago (2015-02-27 02:14:02 UTC) #27
jungkees
On 2015/02/27 02:14:02, falken wrote: > On 2015/02/27 01:51:57, falken wrote: > > > Regardless, ...
5 years, 9 months ago (2015-02-27 02:21:00 UTC) #28
jungkees
On 2015/02/27 02:21:00, jungkees wrote: > On 2015/02/27 02:14:02, falken wrote: > > On 2015/02/27 ...
5 years, 9 months ago (2015-02-27 05:52:04 UTC) #29
falken
On 2015/02/27 05:52:04, jungkees wrote: > On 2015/02/27 02:21:00, jungkees wrote: > > On 2015/02/27 ...
5 years, 9 months ago (2015-02-27 05:58:53 UTC) #30
jungkees
On 2015/02/27 05:58:53, falken wrote: > On 2015/02/27 05:52:04, jungkees wrote: > > On 2015/02/27 ...
5 years, 9 months ago (2015-02-27 06:01:29 UTC) #31
falken
lgtm with nits It's a bit more orthodox to move the "related issue" (which is ...
5 years, 9 months ago (2015-02-27 12:30:03 UTC) #32
jungkees
On 2015/02/27 12:30:03, falken wrote: > lgtm with nits > Thank you so much for ...
5 years, 9 months ago (2015-02-27 13:10:06 UTC) #33
falken
still lgtm, thanks
5 years, 9 months ago (2015-03-01 15:35:53 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/835673006/140001
5 years, 9 months ago (2015-03-01 15:38:07 UTC) #37
commit-bot: I haz the power
5 years, 9 months ago (2015-03-01 17:23:44 UTC) #38
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191059

Powered by Google App Engine
This is Rietveld 408576698