ServiceWorker: Rename 'pending version' to 'installing' or 'waiting'

ServiceWorker: Rename 'pending version' to 'installing' or 'waiting'

'pending version' seems outdated naming and the current spec uses
'installing version' or 'waiting version' instead.
This renames them mechanically.

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/


BUG=379012
TEST=compile

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273799

[nhiroki, 2014-05-29 06:35:13]

Hi falken@, can you review this? Thanks!

[falken, 2014-05-29 07:23:34]

I think this is right, the spec and GitHub issues seem settled on "waiting"
instead of what used to be "pending".

However, within SWRegisterJob "pending" could mean both "installing" and
"waiting". Let's keep that one as "pending" to help call that out. Also the spec
quotes in the comments should be updated to match the latest spec.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:217: //
TODO(falken): "If serviceWorkerRegistration.waitingWorker is not null..."
this is "installingWorker" in the spec

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:267: // "Set
serviceWorkerRegistration.waitingWorker._state to installing."
the spec now reads "installingWorker.state"

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.h (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.h:80:
scoped_refptr<ServiceWorkerVersion> waiting_version;
pending_version here doesn't necessarily correspond to the spec's "waiting"; it
can hold the spec's "installing" or "waiting" version. So I think it's better to
keep it as "pending" (we should have a comment saying such too)

[nhiroki, 2014-05-29 09:02:39]

Thank you for reviewing!

On 2014/05/29 07:23:34, falken wrote:
> However, within SWRegisterJob "pending" could mean both "installing" and
> "waiting". Let's keep that one as "pending" to help call that out. Also the
spec
> quotes in the comments should be updated to match the latest spec.

I see... apparently this should be considered along with 'installing' support to
make better conformance with the spec's algorithm, so let me suspend this CL for
now. I'll revisit it later.

[michaeln1, 2014-05-29 21:16:51]

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:411:
provider_host->SetWaitingVersion(version);
Can you put a todo here about how this should be host->SetInstallingVersion()
but we don't have that method yet.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.h (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.h:80:
scoped_refptr<ServiceWorkerVersion> waiting_version;
On 2014/05/29 07:23:34, falken wrote:
> pending_version here doesn't necessarily correspond to the spec's "waiting";
it
> can hold the spec's "installing" or "waiting" version. So I think it's better
to
> keep it as "pending" (we should have a comment saying such too)

+1 keeping this as "pending" (or something else thats not "installing" or
"waiting"). "pending", "inprogress", or "new".

[michaeln1, 2014-05-29 21:19:27]

> I see... apparently this should be considered along with 'installing' support
to
> make better conformance with the spec's algorithm, so let me suspend this CL
for
> now. I'll revisit it later.

This seems ok to land? Its not yet complete (with 'installing' vs 'waiting') but
it is a nice rename step in the right direction.

[nhiroki, 2014-05-30 00:38:20]

Updated. Can you take another look?

I've looked over the spec and impl, and now I'm thinking it's worth landing this
apart from supporting 'installing' version.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:217: //
TODO(falken): "If serviceWorkerRegistration.waitingWorker is not null..."
On 2014/05/29 07:23:34, falken wrote:
> this is "installingWorker" in the spec

Done.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:267: // "Set
serviceWorkerRegistration.waitingWorker._state to installing."
On 2014/05/29 07:23:34, falken wrote:
> the spec now reads "installingWorker.state"

Done.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:411:
provider_host->SetWaitingVersion(version);
On 2014/05/29 21:16:52, michaeln1 wrote:
> Can you put a todo here about how this should be host->SetInstallingVersion()
> but we don't have that method yet.

Done.

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.h (right):

https://codereview.chromium.org/309453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.h:80:
scoped_refptr<ServiceWorkerVersion> waiting_version;
On 2014/05/29 21:16:52, michaeln1 wrote:
> On 2014/05/29 07:23:34, falken wrote:
> > pending_version here doesn't necessarily correspond to the spec's "waiting";
> it
> > can hold the spec's "installing" or "waiting" version. So I think it's
better
> to
> > keep it as "pending" (we should have a comment saying such too)
> 
> +1 keeping this as "pending" (or something else thats not "installing" or
> "waiting"). "pending", "inprogress", or "new".

Reverted this to 'pending' and added comment.

[falken, 2014-05-30 01:59:10]

lgtm, thanks

[nhiroki, 2014-05-30 06:05:13]

michaeln@: I'll check the CQ box. Please feel free to uncheck it, if you have
any comments on the latest patchset. Thanks!

[nhiroki, 2014-05-30 06:05:22]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2014-05-30 06:08:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nhiroki@chromium.org/309453002/90001

[commit-bot: I haz the power, 2014-05-30 10:52:15]

Change committed as 273799