[ServiceWorker] Don't allow registration of the ServiceWorker scope outside the script directory.

[ServiceWorker] Don't allow registration of the ServiceWorker scope outside the script directory.

As per discussion on https://github.com/slightlyoff/ServiceWorker/issues/468

BUG=423983
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184336

[horo, 2014-10-24 05:29:40]

Patchset #1 (id:1) has been deleted

[horo, 2014-10-24 06:08:42]

horo@chromium.org changed reviewers:
+ falken@chromium.org

[horo, 2014-10-24 06:08:43]

falken@
Could you please review this?

[falken, 2014-10-24 06:50:44]

looks good...

1. Can you add a note like "As per discussion on
https://github.com/slightlyoff/ServiceWorker/issues/468" in your CL description?


2. We should also have a Chromium-side check in
ServiceWorkerDispatcherHost::CanRegisterServiceWorker. That will of course be a
separate patch, but we should note it in the bug tracker so we don't forget.

https://codereview.chromium.org/672383003/diff/30001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/scope-default.html
(right):

https://codereview.chromium.org/672383003/diff/30001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/scope-default.html:15:
}, 'Registering default scope');
This test should have a better name. "Register a script not in the root
directory for the default scope"

We also now lose the test coverage of scope_test for a default scope. But I
think now it's impossible unless we put the script in LayoutTest/http/tests.

https://codereview.chromium.org/672383003/diff/30001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp (right):

https://codereview.chromium.org/672383003/diff/30001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp:229: }
can you add a test here for Register failing due to the path restriction?
(it'll be in addition to the end-to-end layout tests)

[horo, 2014-10-24 07:21:43]

> 1. Can you add a note like "As per discussion on
> https://github.com/slightlyoff/ServiceWorker/issues/468" in your CL
description?
done

> 2. We should also have a Chromium-side check in
> ServiceWorkerDispatcherHost::CanRegisterServiceWorker. That will of course be
a
> separate patch, but we should note it in the bug tracker so we don't forget.

Ah. I didn't know this.
I will create a patch for this.

https://codereview.chromium.org/672383003/diff/30001/LayoutTests/http/tests/s...
File
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/scope-default.html
(right):

https://codereview.chromium.org/672383003/diff/30001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/scope-default.html:15:
}, 'Registering default scope');
On 2014/10/24 06:50:44, falken wrote:
> This test should have a better name. "Register a script not in the root
> directory for the default scope"
> 
> We also now lose the test coverage of scope_test for a default scope. But I
> think now it's impossible unless we put the script in LayoutTest/http/tests.

Done.

https://codereview.chromium.org/672383003/diff/30001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp (right):

https://codereview.chromium.org/672383003/diff/30001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp:229: }
On 2014/10/24 06:50:44, falken wrote:
> can you add a test here for Register failing due to the path restriction?
> (it'll be in addition to the end-to-end layout tests)

Done.

[falken, 2014-10-24 07:30:36]

LGTM

https://codereview.chromium.org/672383003/diff/50001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp (right):

https://codereview.chromium.org/672383003/diff/50001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp:231:
TEST_F(ServiceWorkerContainerTest, Register_DiffentDirectryThanScript)
spelling: Different and Directory

[horo, 2014-10-24 07:46:38]

https://codereview.chromium.org/672383003/diff/50001/Source/modules/servicewo...
File Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp (right):

https://codereview.chromium.org/672383003/diff/50001/Source/modules/servicewo...
Source/modules/serviceworkers/ServiceWorkerContainerTest.cpp:231:
TEST_F(ServiceWorkerContainerTest, Register_DiffentDirectryThanScript)
On 2014/10/24 07:30:36, falken wrote:
> spelling: Different and Directory

Done.

[horo, 2014-10-24 07:47:33]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-24 07:48:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/672383003/70001

[commit-bot: I haz the power, 2014-10-24 08:48:29]

Committed patchset #4 (id:70001) as 184336

[Peter Beverloo, 2014-10-24 15:32:36]

A revert of this CL (patchset #4 id:70001) has been created in
https://codereview.chromium.org/672353003/ by peter@chromium.org.

The reason for reverting is: This patch seems to have broken the following two
browser_tests on all platforms, as can be seen on the non-layout bots on the
Blink waterfall. We won't be able to roll Blink in Chromium until this is
resolved.

PushMessagingBrowserTest.RegisterFailureNoPermission
PushMessagingBrowserTest.RegisterSuccess

Example output:
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%2...

[INFO:CONSOLE(7)] "SecurityError - The scope must be under the directory of the
script URL.", source: https://127.0.0.1:39935/files/push_messaging/test.html (7)
../../chrome/browser/services/gcm/push_messaging_browsertest.cc:142: Failure
Value of: register_worker_result
  Actual: "SecurityError - The scope must be under the directory of the script
URL."
Expected: "ok".