Service Worker: Make ServiceWorkerRegistration.update() return a promise. (Chromium 2/3)

Service Worker: Make ServiceWorkerRegistration.update() return a promise. (Chromium 2/3)

As per the resolution of f2f, ServiceWorkerRegistration.update() should return a
promise that transforms the promise returned by Update algorithm.

In this CL, ServiceWorkerContextCore::UpdateServiceWorker method has been
overloaded to cover two invocation paths: a scheduled update without a
provider_host and a callback (Soft Update in the spec) and a call initiated from
the script surface using ServiceWorkerRegistration.update().

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#service-worker-registration-update
Spec discussion: https://github.com/slightlyoff/ServiceWorker/issues/311

Companion CL (Blink): https://codereview.chromium.org/1260833003/
Companion CL (Blink layout test): https://codereview.chromium.org/1268663003/

BUG=513655
Committed: https://crrev.com/2f85d42e860bb2370f411a7a2d3ad665641d6970
Cr-Commit-Position: refs/heads/master@{#341515}

[jungkees, 2015-07-30 11:27:25]

jungkee.song@samsung.com changed reviewers:
+ jinho.bang@samsung.com, l.gombos@samsung.com

[jungkees, 2015-07-30 11:34:58]

Chromium side of the registration.update() change. PTAL.

[zino, 2015-07-31 01:00:47]

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
content/child/service_worker/service_worker_dispatcher.cc:395:
"ServiceWorkerDispatcher::RegisterServiceWorker",
I'm not sure but shouldn't this be "Update" instead of "Register"?

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
content/child/service_worker/service_worker_dispatcher.cc:396: request_id,
"OnRegistered");
ditto.

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
content/child/service_worker/service_worker_dispatcher.cc:398:
"ServiceWorkerDispatcher::RegisterServiceWorker",
ditto.

[jungkees, 2015-07-31 05:49:18]

Addressed the comment. Thanks for peer-review.

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1270513002/diff/1/content/child/service_worke...
content/child/service_worker/service_worker_dispatcher.cc:395:
"ServiceWorkerDispatcher::RegisterServiceWorker",
Right. Thanks for spotting it!

[jungkees, 2015-07-31 05:51:04]

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

[jungkees, 2015-07-31 05:51:38]

SW OWNERs, PTAL.

[nhiroki, 2015-07-31 06:49:11]

Looks good overall. Nit comments only.

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:437:
base::ASCIIToUTF16(kInvalidStateErrorMessage)));
We might need to change the update() algorithm in the spec. The current spec[1]
("4. If newestWorker is null, abort these steps.") does not mention that a
promise is rejected with InvalidStateError.

[1]
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.h (right):

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_register_job.h:59: // candidate
process to run the worker.
nit: This change looks not necessary.

https://codereview.chromium.org/1270513002/diff/20001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1270513002/diff/20001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:562:
callbacks->onError(error.release());
nit: We can directly pass "new WebSWError" w/o scoped_ptr (and clean up other
OnXXXError functions later).

[jungkees, 2015-07-31 07:09:58]

Uploaded a new snapshot having addressed the comments. PTAL.

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:437:
base::ASCIIToUTF16(kInvalidStateErrorMessage)));
I noticed that and was gonna change this. ;-) I will address this in no time.
Thanks!

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.h (right):

https://codereview.chromium.org/1270513002/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_register_job.h:59: // candidate
process to run the worker.
Yes, right. This was changed when I tried to make |provider_host| to
|opt_provider_host| but I rather have done it by overloading the functions after
all. I'll leave the original comment as-is.

https://codereview.chromium.org/1270513002/diff/20001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1270513002/diff/20001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:562:
callbacks->onError(error.release());
Addressed. Thanks.

[nhiroki, 2015-07-31 07:46:55]

LGTM

[jungkees, 2015-07-31 10:17:28]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-07-31 10:17:39]

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

[jungkees, 2015-07-31 10:22:57]

The CQ bit was unchecked by jungkee.song@samsung.com

[jungkees, 2015-07-31 10:28:41]

jungkee.song@samsung.com changed reviewers:
+ mkwst@chromium.org

[jungkees, 2015-07-31 10:30:35]

+mkwst@ PTAL at 'content/common/service_worker/service_worker_messages.h'.
This CL is changing ServiceWorkerContainer.update() method as reviewed by
Service Worker team and the API OWNER.

[Mike West, 2015-07-31 13:06:10]

IPC changes LGTM.

[jungkees, 2015-08-03 05:54:01]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 05:54:33]

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

[commit-bot: I haz the power, 2015-08-03 06:04:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 06:04:26]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jungkees, 2015-08-03 06:44:18]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 06:44:31]

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

[commit-bot: I haz the power, 2015-08-03 06:54:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 06:54:23]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jungkees, 2015-08-03 07:29:21]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 07:29:26]

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

[commit-bot: I haz the power, 2015-08-03 07:41:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 07:41:19]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[jungkees, 2015-08-03 07:54:04]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 07:54:09]

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

[commit-bot: I haz the power, 2015-08-03 08:04:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 08:04:28]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jungkees, 2015-08-03 09:40:43]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 09:41:11]

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

[commit-bot: I haz the power, 2015-08-03 11:01:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-03 11:01:11]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jungkees, 2015-08-03 11:19:46]

The CQ bit was checked by jungkee.song@samsung.com

[commit-bot: I haz the power, 2015-08-03 11:19:57]

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

[commit-bot: I haz the power, 2015-08-03 12:10:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-03 12:11:28]

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