Store foreign fetch scopes in database with other SW information.

Store foreign fetch scopes in database with other SW information.

This adds the list of foreign fetch scopes associated with a worker to the
ServiceWorkerRegistrationData protobuf, and separately also stores the list
of all origins which have some foreign fetch scopes registered.

No increase in database version is needed, as absence of this new data is the correct default.

BUG=540509
Committed: https://crrev.com/c7edc05b8028a219bcb92922814147e4fbc0a01f
Cr-Commit-Position: refs/heads/master@{#355588}

[Marijn Kruisselbrink, 2015-10-20 04:17:47]

Patchset #1 (id:1) has been deleted

[Marijn Kruisselbrink, 2015-10-20 04:17:52]

Patchset #1 (id:20001) has been deleted

[Marijn Kruisselbrink, 2015-10-20 23:59:45]

Description was changed from

==========
Store foreign fetch scopes in database with other SW information.

BUG=540509
==========

to

==========
Store foreign fetch scopes in database with other SW information.

This adds the list of foreign fetch scopes associated with a worker to the
ServiceWorkerRegistrationData protobuf, and separately also stores the list
of all origins which have some foreign fetch scopes registered.

No increase in database version is needed, as absence of this new data is the
correct default.

BUG=540509
==========

[Marijn Kruisselbrink, 2015-10-21 00:06:23]

Patchset #1 (id:40001) has been deleted

[Marijn Kruisselbrink, 2015-10-21 00:21:29]

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

[falken, 2015-10-21 01:01:53]

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

[falken, 2015-10-21 01:01:53]

nhiroki, could you look at this storage change?

[nhiroki, 2015-10-21 05:42:54]

Looks good.

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_database.cc (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.cc:218: for (const GURL&
url : input.foreign_fetch_scopes) {
Can you add DCHECK to check if |url| is a subscope of |input.scope|?
ServiceWorkerUtils::ScopeMatches could be available for that.

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.cc:327:
out->foreign_fetch_scopes.push_back(GURL(data.foreign_fetch_scope(i)));
Can you check if |foreign_fetch_scope| is a subscope of |scope_url|, and return
ERROR_CORRUPTED if not?

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_database.proto (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.proto:23: repeated string
foreign_fetch_scope = 9;
nit: how about putting this after |resources_total_size_bytes| to make it easier
to pick up the next available number when we add a new field later?

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_storage.cc:690: const GURL&
origin) {
This should return false if the storage is disabled (IsDisabled() is true)

[Marijn Kruisselbrink, 2015-10-21 19:57:27]

Patchset #2 (id:80001) has been deleted

[Marijn Kruisselbrink, 2015-10-21 20:33:46]

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_database.cc (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.cc:218: for (const GURL&
url : input.foreign_fetch_scopes) {
On 2015/10/21 at 05:42:54, nhiroki wrote:
> Can you add DCHECK to check if |url| is a subscope of |input.scope|?
ServiceWorkerUtils::ScopeMatches could be available for that.

Done

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.cc:327:
out->foreign_fetch_scopes.push_back(GURL(data.foreign_fetch_scope(i)));
On 2015/10/21 at 05:42:54, nhiroki wrote:
> Can you check if |foreign_fetch_scope| is a subscope of |scope_url|, and
return ERROR_CORRUPTED if not?

Done

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_database.proto (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_database.proto:23: repeated string
foreign_fetch_scope = 9;
On 2015/10/21 at 05:42:54, nhiroki wrote:
> nit: how about putting this after |resources_total_size_bytes| to make it
easier to pick up the next available number when we add a new field later?

Good point, done.

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/1411953002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_storage.cc:690: const GURL&
origin) {
On 2015/10/21 at 05:42:54, nhiroki wrote:
> This should return false if the storage is disabled (IsDisabled() is true)

Done

[nhiroki, 2015-10-22 10:32:56]

lgtm

[Marijn Kruisselbrink, 2015-10-22 17:43:32]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2015-10-22 17:43:49]

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

[commit-bot: I haz the power, 2015-10-22 18:31:45]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2015-10-22 18:32:42]

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