Service Worker: Migrate to version_uuid and surface ServiceWorker.id. (Blink 1/3)

Service Worker: Migrate to version_uuid and surface ServiceWorker.id. (Blink 1/3)

Migrate ServiceWorkerVerion's int64 type version_id to std::string type
version_uuid:
  - Replace next_avail_version_id semantic with a UUID based versioning.
  - Migrate leveldb schema
    . Deprecate INITDATA_NEXT_VERSION_ID
    . Change data format for ServiceWorkerResourceRecord

Surface ServiceWorker.id attribute as per spec:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#client-id-attribute

Spec issue: https://github.com/slightlyoff/ServiceWorker/issues/635

Companion CL (Chromium): https://codereview.chromium.org/1221643014/
Companion CL (Blink layout test): https://codereview.chromium.org/1216933008/

BUG=470031

[jungkees, 2015-07-01 13:54:01]

jungkee.song@samsung.com changed reviewers:
+ dw.im@samsung.com, l.gombos@samsung.com

[jungkees, 2015-07-01 13:57:07]

PTAL

[lgombos, 2015-07-02 00:29:41]

peer review looks good to me (I deffer the detailed review to OWNERS).

[jungkees, 2015-07-02 01:17:25]

jungkee.song@samsung.com changed reviewers:
+ falken@chromium.org, kinuko@chromium.org, michaeln@chromium.org,
nhiroki@chromium.org

[jungkees, 2015-07-02 01:22:40]

PTAL I think we might additionally need a dynamic db migration for running
instances that's using the old db schema?

[kinuko, 2015-07-03 14:00:04]

I think you/we need a bit clearer transition plan?  E.g. how you plan to land
these multi-sided patches and what you think should happen for existing versions
that are stored in the db?

For this particular patch it looks it'd temporarily put all resources in a
single cache bucket until the corresponding chromium patch lands, is my
understanding right?  I worry it's a bit unsafe even if it would be exposed only
on Canary.

[jungkees, 2015-07-06 06:15:39]

On 2015/07/03 14:00:04, kinuko wrote:
> I think you/we need a bit clearer transition plan?  E.g. how you plan to land
> these multi-sided patches and what you think should happen for existing
versions
> that are stored in the db?
> 

Absolutely. Happy to discuss the plan. :-) I think we need to add
ServiceWorkerDatabase::UpgradeDatabaseSchemaFromV2ToV3() (being invoked in
LazyOpen()) to update the incumbent database records that needs migration. The
steps would roughly be: 
  1. For each reg_record in database whose key is "REG:" + <GURL 'origin'> +
'\x00' + <int64 'registration_id'>:
    1. Let old_id be reg_record.value's version_id.
    2. Let new_id be the result of running GetGUID().
    2. Set new_value to the new serialized ServiceWorkerRegistrationData (using
new_id).
    3. Update reg_record.value to new_value.
    4. Find a record res_record in database whose key is "RES:" + <int64
'version_id'> + '\x00' + <int64 'resource_id'> (using old_id here):
      1. Let resource_id be res_record.key's resource_id.
      2. Let value be res_record.value.
      3. Delete res_record.
      3. Let new_key be "RES:" + serialized new_id + '\x00' + serialized
resource_id.
      4. Put a new record (new_key, value) in the database.
  2. Delete the record keyed by INITDATA_NEXT_VERSION_ID which will not be used
any more. (GenerateGUID() will do it instead.)

Re the order of the patches:
  1. Blink side 1: Files for exposing ServiceWorker.id (Source/modules,
public/platform).
  2. Chromium side. (i.e. including database migration code BUT retain the
database version to *2*).
  3. Blink side 2: Files that use serviceWorkerID() (Source/core, Source/web,
public/web).
  4. Change the database version to 3.
  5. Blink side 3: Add the layout test.
  ** 4 followed by 3 should be instant.

Please let me know whether the above plan makes sense, or there are some better
ways to do.

> For this particular patch it looks it'd temporarily put all resources in a
> single cache bucket until the corresponding chromium patch lands, is my
> understanding right?  I worry it's a bit unsafe even if it would be exposed
only
> on Canary.

Yes the db migration part was missing. With the above plan, I'd expect the first
LazyOpen() call would trigger the db migration before any other access to the
new schema.

[michaeln, 2015-07-08 22:58:59]

Does exposing the id to script require changing from a sequence num to a uuid?

[jungkees, 2015-07-09 01:35:33]

Using a uuid seems not a functional requirement. A serialized string of a unique
number should be fine to be working correctly. But in the standard (and
compatibility / testing) point of view, requiring a uuid seems better than just
letting different browsers returning different types of information for ids.

[jungkees, 2015-07-24 02:06:04]

Closed the CL as per the consensus at f2f to punt ServiceWorker.id to v2 or
since there's not a clear
use case yet.