[ServiceWorker] Introduce the directory restriction of ServiceWorker scope in chromium side.

Introduce the directory restriction of ServiceWorker scope in chromium side.

This restriction doesn'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://crrev.com/8782077ef772d6eec4b3e308a9351e2495db7412
Cr-Commit-Position: refs/heads/master@{#301547}

[horo, 2014-10-24 10:10:37]

Patchset #1 (id:1) has been deleted

[horo, 2014-10-25 01:40:00]

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

[horo, 2014-10-25 01:40:00]

falken@
Could you please review this?
https://codereview.chromium.org/672383003/ was reverted because of the chromium
side test break.

[horo, 2014-10-26 04:00:46]

Patchset #2 (id:40001) has been deleted

[horo, 2014-10-26 04:03:32]

Patchset #1 (id:20001) has been deleted

[falken, 2014-10-27 03:54:18]

looks good, with some suggestions. 

> https://codereview.chromium.org/672383003/ was reverted because of the
chromium
side test break.

I think you already know, but for the record, this CL wouldn't fix the test. The
test just has to be updated (already done in
https://codereview.chromium.org/644763005/).

The point of both Blink-side and Chromium-side checks is that the renderer can
display a user-friendly error message if the user gives bad input, and not send
the request to Chromium. If Chromium gets a bad request, it means the renderer
is buggy or compromised so Chromium will terminate it.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:61: 0,
slash_pos + 1, script_url.spec(), 0, slash_pos + 1) == 0;
i think this can just be: pattern.spec().compare(0, slash_pos + 1,
script_url.spec())

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:66: const GURL&
script_url) {
Using spec() in this CL reminds me that we should be checking the URLs for
validity. We can't trust the renderer to send valid URLs.

I think we need to add checking here for all the URLs being is_valid(). We could
then also add DCHECKs in the helper functions for is_valid.

Or should that be a separate CL? WDYT?

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:73: const GURL&
pattern) {
Check for validity here too.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:79: const GURL&
given_document_url) {
And here

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:303:
"ServiceWorkerDispatcherHost::OnUnregisterServiceWorker");
Here too, let's check for validity.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:362:
"ServiceWorkerDispatcherHost::OnGetRegistration");
And here.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
(right):

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host_unittest.cc:216: 
Can you add checks for invalid GURLs too, like the empty GURL?

[horo, 2014-10-27 09:13:27]

Patchset #2 (id:80001) has been deleted

[horo, 2014-10-27 09:14:12]

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:61: 0,
slash_pos + 1, script_url.spec(), 0, slash_pos + 1) == 0;
On 2014/10/27 03:54:17, falken wrote:
> i think this can just be: pattern.spec().compare(0, slash_pos + 1,
> script_url.spec())

No. We need subpos and sublen.
http://www.cplusplus.com/reference/string/string/compare/

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:66: const GURL&
script_url) {
On 2014/10/27 03:54:17, falken wrote:
> Using spec() in this CL reminds me that we should be checking the URLs for
> validity. We can't trust the renderer to send valid URLs.
> 
> I think we need to add checking here for all the URLs being is_valid(). We
could
> then also add DCHECKs in the helper functions for is_valid.
> 
> Or should that be a separate CL? WDYT?

Done.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:73: const GURL&
pattern) {
On 2014/10/27 03:54:17, falken wrote:
> Check for validity here too.

Done.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:79: const GURL&
given_document_url) {
On 2014/10/27 03:54:17, falken wrote:
> And here

Done.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:303:
"ServiceWorkerDispatcherHost::OnUnregisterServiceWorker");
On 2014/10/27 03:54:17, falken wrote:
> Here too, let's check for validity.

Done.

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:362:
"ServiceWorkerDispatcherHost::OnGetRegistration");
On 2014/10/27 03:54:17, falken wrote:
> And here.

Done.

[falken, 2014-10-28 00:55:45]

lgtm

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/677003002/diff/60001/content/browser/service_...
content/browser/service_worker/service_worker_dispatcher_host.cc:61: 0,
slash_pos + 1, script_url.spec(), 0, slash_pos + 1) == 0;
On 2014/10/27 09:14:12, horo wrote:
> On 2014/10/27 03:54:17, falken wrote:
> > i think this can just be: pattern.spec().compare(0, slash_pos + 1,
> > script_url.spec())
> 
> No. We need subpos and sublen.
> http://www.cplusplus.com/reference/string/string/compare/

Ah, my mistake. I thought if they were the same as pos and len they'd have no
effect.

[horo, 2014-10-28 01:12:16]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-28 01:15:39]

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

[commit-bot: I haz the power, 2014-10-28 02:13:42]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2014-10-28 02:14:09]

Patchset 2 (id:??) landed as
https://crrev.com/8782077ef772d6eec4b3e308a9351e2495db7412
Cr-Commit-Position: refs/heads/master@{#301547}