ServiceWorker: Make '.ready' return a promise to be resolved with ServiceWorkerRegistration (2/3)

ServiceWorker: Make '.ready' return a promise to be resolved with ServiceWorkerRegistration (2/3)

Potential controllees need to observe version change events to make a ready
registration, but, before this patch, those events are sent not to potential
controllees but to pages that have the corresponding registration object
(to be more precise, pages that have the reference to the registration).

After this patch, ServiceWorkerProviderHost sends an IPC message to the
corresponding ServiceWorkerProviderContext when the host gets associated with
a registration and the provider context retains a reference of the registration
so that the potential controllee can observe the version change events.

1) Blink: https://codereview.chromium.org/476043002/
2) Chromium: THIS PATCH
3) Blink: https://codereview.chromium.org/532653002/


BUG=399533
TEST=content_unittests --gtest_filter=ServiceWorker*
TEST=run_webkit_tests.py --debug http/tests/serviceworker/ (with the patch (3))

Committed: https://crrev.com/71840871e1f8499d843d6a21dee0e361c3ba64f0
Cr-Commit-Position: refs/heads/master@{#294147}

[nhiroki, 2014-09-02 11:13:04]

nhiroki@chromium.org changed reviewers:
+ horo@chromium.org, michaeln@chromium.org

[nhiroki, 2014-09-02 11:13:04]

Ptal, thanks!

[nhiroki, 2014-09-02 15:52:01]

Hmmm... I might have found a corner case of the association introduced by this
CL. In the following case, the ready promise never be resolved and the test is
timed out.

    var promise = navigator.serviceWorker.register(url, {scope:scope});
    with_iframe(scope)
      .then(function(f) {
          frame = f;
          return p;
        })
      .then(function() {
          return frame.contentWindow.navigator.serviceWorker.ready;
        })
      .then(function(registration) {
          unload_iframe(frame);
          service_worker_unregister_and_done(t, scope);
        })

The association happens in...
  - ServiceWorkerRegisterJob::RegisterAndContinue
  - ServiceWorkerControlleeRequestHandler::DidLookupRegistrationForMainResource
  - ServiceWorkerControlleeRequestHandler::OnVersionStatusChanged

I speculate that the potential controllee requested while the potential
controller is in the installing state could fail to get associated with the
registration. I'll take a deeper look tomorrow.

[michaeln, 2014-09-04 00:27:23]

i can't really reason about the renderer-side of this code :( but the changes to
unreg_job look amiss to me.

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

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:388:
GetOrCreateRegistrationHandle(provider_id, registration);
seems like GetOrCreateRegistrationHandle() should return handle ptr instead of
an info struct given how its named, info easy easy enough to get with the
handle->GetObject() method

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
File content/browser/service_worker/service_worker_unregister_job.cc (right):

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
content/browser/service_worker/service_worker_unregister_job.cc:113:
host->UnassociateRegistration();
This does not look right?

The unreg doesn't really stick until there are no controllees where there are no
controllees. That's what "ClearWhenReady()" means.

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
File content/child/service_worker/web_service_worker_provider_impl.cc (right):

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:55:
context_->registration()->info(), false);
I know this is because things are in an intermediary state, but this is kinda
bizarre reading code. Here its assuming that if active_handle_id then there must
be a reg->info. Ok, if you say so?

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:58:
registration->SetInstalling(GetDispatcher()->GetServiceWorker(
Registration may be a preexising object accoding to the impl, yet here we'll
overwrite whatever it may contain but only if we're over writing with non-null
values.

This is all so arbitrary over here in the renderer-side of things.

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:69:
context_->registration()->info(), false));
Is there a reason to call GetServiceWorkerRegistration() again or could you use
the |registration| local variable? Sounds like that may be a rhetorical
question, but its not, the client side of this makes no sense to me.

[nhiroki, 2014-09-08 15:55:52]

Remade the CL. Can you take another look?

I tried to make it clearer that ServiceWorkerProviderContext retains references
of the associated registration and its version attributes.

Thanks!

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

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:388:
GetOrCreateRegistrationHandle(provider_id, registration);
On 2014/09/04 00:27:23, michaeln wrote:
> seems like GetOrCreateRegistrationHandle() should return handle ptr instead of
> an info struct given how its named, info easy easy enough to get with the
> handle->GetObject() method

Done in the separate CL (https://codereview.chromium.org/552693002/)

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
File content/browser/service_worker/service_worker_unregister_job.cc (right):

https://codereview.chromium.org/477593007/diff/420001/content/browser/service...
content/browser/service_worker/service_worker_unregister_job.cc:113:
host->UnassociateRegistration();
On 2014/09/04 00:27:23, michaeln wrote:
> This does not look right?
> 
> The unreg doesn't really stick until there are no controllees where there are
no
> controllees. That's what "ClearWhenReady()" means.
> 

Ah, yes. This is not correct. Removed.

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
File content/child/service_worker/web_service_worker_provider_impl.cc (right):

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:55:
context_->registration()->info(), false);
On 2014/09/04 00:27:23, michaeln wrote:
> I know this is because things are in an intermediary state, but this is kinda
> bizarre reading code. Here its assuming that if active_handle_id then there
must
> be a reg->info. Ok, if you say so?
> 

Straightened the logic in the latest patchset. How does it look to you?

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:58:
registration->SetInstalling(GetDispatcher()->GetServiceWorker(
On 2014/09/04 00:27:23, michaeln wrote:
> Registration may be a preexising object accoding to the impl, yet here we'll
> overwrite whatever it may contain but only if we're over writing with non-null
> values.
> 
> This is all so arbitrary over here in the renderer-side of things.

Updated. The latest patchset just reuses the existing registration without any
overwrites if possible.

https://codereview.chromium.org/477593007/diff/420001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:69:
context_->registration()->info(), false));
On 2014/09/04 00:27:23, michaeln wrote:
> Is there a reason to call GetServiceWorkerRegistration() again or could you
use
> the |registration| local variable? Sounds like that may be a rhetorical
> question, but its not, the client side of this makes no sense to me.

Oops... that's just my mistake. The local variable must be ok. Sorry for making
you confused.

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_controllee_request_handler.cc:142:
provider_host_->AssociateRegistration(registration.get());
To associate a potential controllee (ie. |provider_host_|) with the
registration, I separated SetControllerVersionAttribute() from
AssociateRegistration().

[michaeln, 2014-09-09 02:22:02]

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_controllee_request_handler.cc:142:
provider_host_->AssociateRegistration(registration.get());
On 2014/09/08 15:55:52, nhiroki wrote:
> To associate a potential controllee (ie. |provider_host_|) with the
> registration, I separated SetControllerVersionAttribute() from
> AssociateRegistration().

Does this have to happen prior to the pre-existing AssociateRegistration
callsites below? Why is this separation needed? Generally, the client-side
doesn't see associations with a registration with an active version that is
still activating. With the early call to AssocReg here, that condition now
exists in the client-side which opens the door to corner cases that maybe don't
need to be there. So if possible i'd like to keep the regassocation and
controllingversion setting as close together as possible.

Are you concerned about the other call to AssociateProviderHostsToRegistration
in the UpdateJob class inadvertently setting the controlling version when it
shouldn't? If so, thats not a concern that callsite is only used for newly
created registrations which do not have an active version.

Are you concerned about the raciness between main doc loading settling on an
association and an update job creating a new registration? If so, i think i see
a bug with that regardless of this earlier call to AssociateRegistration(). I'll
upload a patch for it (has to do with having the docurl being set in advance of
a reg getting associated).

Would it make sense to keep them as separate methods but to invoke them on
consecutive lines in the .cc file?

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:103: void
ServiceWorkerProviderHost::SetControllerVersionAttribute(
if this this method remains public, can you move it up so it's position in the
.cc file matches the .h file, and add a dcheck to assert that the version
provided is either NULL or the 'active' version of the associated registration?

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:176:
SetControllerVersionAttribute(NULL);
does the change in order of operations matter here, just curious?

https://codereview.chromium.org/477593007/diff/460001/content/child/service_w...
File content/child/service_worker/web_service_worker_provider_impl.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/child/service_w...
content/child/service_worker/web_service_worker_provider_impl.cc:73:
client->setReadyRegistration(registration);
an example of a corner case, this logic assumes that if there's have an active
version, we must be ready but its state may still be activating

[nhiroki, 2014-09-09 08:49:08]

Thank you for your comments. Reply only.

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_controllee_request_handler.cc:142:
provider_host_->AssociateRegistration(registration.get());
On 2014/09/09 02:22:01, michaeln wrote:
> On 2014/09/08 15:55:52, nhiroki wrote:
> > To associate a potential controllee (ie. |provider_host_|) with the
> > registration, I separated SetControllerVersionAttribute() from
> > AssociateRegistration().
> 
> Does this have to happen prior to the pre-existing AssociateRegistration
> callsites below? Why is this separation needed? Generally, the client-side
> doesn't see associations with a registration with an active version that is
> still activating. With the early call to AssocReg here, that condition now
> exists in the client-side which opens the door to corner cases that maybe
don't
> need to be there. So if possible i'd like to keep the regassocation and
> controllingversion setting as close together as possible.

Hmmm... I think the condition would have already existed.
SWRegisterJob::RegisterAndContinue associates hosts with a newly created
registration and then the registration takes an active version whose state is
'activating'. I'm not really sure why we don't have to associate unactivated
registration here like the register job... (or is the register job wrong??)

> Are you concerned about the other call to AssociateProviderHostsToRegistration
> in the UpdateJob class inadvertently setting the controlling version when it
> shouldn't? If so, thats not a concern that callsite is only used for newly
> created registrations which do not have an active version.

No, I'm not concerned about it.

> Are you concerned about the raciness between main doc loading settling on an
> association and an update job creating a new registration? If so, i think i
see
> a bug with that regardless of this earlier call to AssociateRegistration().
I'll
> upload a patch for it (has to do with having the docurl being set in advance
of
> a reg getting associated).

Do you mean that the existing callsite of AssociateProviderHostsToRegistration
(ie. SWRegisterJob::RegisterAndContinue) is wrong 
? If so, I think this is just what I'm concerned. As I commented above, I was
confused why we don't have to associate unactivated registration during the main
loading, whereas we do so in the update job. I guess
ServiceWorkerRegistration::OnActivateEventFinished or a subsequent function is
the correct callsite... is this right?

> Would it make sense to keep them as separate methods but to invoke them on
> consecutive lines in the .cc file?

If my previous inline comment is correct, it makes sense.

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:176:
SetControllerVersionAttribute(NULL);
On 2014/09/09 02:22:02, michaeln wrote:
> does the change in order of operations matter here, just curious?

Just to make consistent with AssociateRegistration() (reversed order)

[nhiroki, 2014-09-09 11:48:35]

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_controllee_request_handler.cc:142:
provider_host_->AssociateRegistration(registration.get());
> > Are you concerned about the raciness between main doc loading settling on an
> > association and an update job creating a new registration? If so, i think i
> see
> > a bug with that regardless of this earlier call to AssociateRegistration().
> I'll
> > upload a patch for it (has to do with having the docurl being set in advance
> of
> > a reg getting associated).
> 
> Do you mean that the existing callsite of AssociateProviderHostsToRegistration
> (ie. SWRegisterJob::RegisterAndContinue) is wrong 
> ? If so, I think this is just what I'm concerned. As I commented above, I was
> confused why we don't have to associate unactivated registration during the
main
> loading, whereas we do so in the update job. I guess
> ServiceWorkerRegistration::OnActivateEventFinished or a subsequent function is
> the correct callsite... is this right?

As per the spec discussion
(https://github.com/slightlyoff/ServiceWorker/issues/357#issuecomment-53397325),
an active worker of the ready registration could be in the activating state, so
ServiceWorkerRegistration::ActivateWaitingVersion could be better place to
associate...?

[michaeln, 2014-09-09 23:30:25]

lgtm

> As per the spec discussion
>
(https://github.com/slightlyoff/ServiceWorker/issues/357#issuecomment-53397325),
> an active worker of the ready registration could be in the activating state,
so
> ServiceWorkerRegistration::ActivateWaitingVersion could be better place to
> associate...?

That might be? But if we do that, probably best to take it up in another cl(s).

[michaeln, 2014-09-09 23:30:33]

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_controllee_request_handler.cc
(right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_controllee_request_handler.cc:142:
provider_host_->AssociateRegistration(registration.get());
> Hmmm... I think the condition would have already existed.

Ah, I think your right and it definitely will have to exist when we get around
to implementing swap() too.

[nhiroki, 2014-09-10 00:48:15]

Thanks! I’ll commit this after reverting the early call (changes of
ServiceWorkerControlleeRequestHandler) and addressing your comments. I know this
doesn’t work in some cases (see c#3) without tweaking the association timing,
but it seems to be OK for now. I’ll fix it in subsequent CLs.

[nhiroki, 2014-09-10 00:55:21]

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

[nhiroki, 2014-09-10 00:55:22]

+nasko@, please take a look at service_worker_messages.h. Thanks.

[nasko, 2014-09-10 01:15:56]

Some comments.

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:162:
kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
nit: Is this expected to change in the immediate future? If not, it is hardcoded
in all callsites, so it should be removed.

https://codereview.chromium.org/477593007/diff/460001/content/common/service_...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/477593007/diff/460001/content/common/service_...
content/common/service_worker/service_worker_messages.h:194:
IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_UnassociateRegistration,
The inverse of "associate" is "dissociate".

[nhiroki, 2014-09-10 01:50:47]

nasko@: Thank you for reviewing. Can you take another look?

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:162:
kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
On 2014/09/10 01:15:56, nasko wrote:
> nit: Is this expected to change in the immediate future? If not, it is
hardcoded
> in all callsites, so it should be removed.

Good point. I think we don't have any plan to change it in the near future
(horo@, is this correct?)

Removing the constant would need more work (changing all message formats,
SWMessageFilter and so on), so for now I'd like to keep this CL as is and fix
them in a separate CL.

https://codereview.chromium.org/477593007/diff/460001/content/common/service_...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/477593007/diff/460001/content/common/service_...
content/common/service_worker/service_worker_messages.h:194:
IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_UnassociateRegistration,
On 2014/09/10 01:15:56, nasko wrote:
> The inverse of "associate" is "dissociate".

Done.

[nasko, 2014-09-10 04:15:10]

LGTM

I just realized that "disassociate" is probably more common, but feel free to
leave things as is.

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:162:
kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
On 2014/09/10 01:50:47, nhiroki wrote:
> On 2014/09/10 01:15:56, nasko wrote:
> > nit: Is this expected to change in the immediate future? If not, it is
> hardcoded
> > in all callsites, so it should be removed.
> 
> Good point. I think we don't have any plan to change it in the near future
> (horo@, is this correct?)
> 
> Removing the constant would need more work (changing all message formats,
> SWMessageFilter and so on), so for now I'd like to keep this CL as is and fix
> them in a separate CL.

Fixing in another CL is fine. Please file a bug so it doesn't get forgotten.

[nhiroki, 2014-09-10 05:28:47]

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:162:
kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
On 2014/09/10 04:15:10, nasko wrote:
> On 2014/09/10 01:50:47, nhiroki wrote:
> > On 2014/09/10 01:15:56, nasko wrote:
> > > nit: Is this expected to change in the immediate future? If not, it is
> > hardcoded
> > > in all callsites, so it should be removed.
> > 
> > Good point. I think we don't have any plan to change it in the near future
> > (horo@, is this correct?)
> > 
> > Removing the constant would need more work (changing all message formats,
> > SWMessageFilter and so on), so for now I'd like to keep this CL as is and
fix
> > them in a separate CL.
> 
> Fixing in another CL is fine. Please file a bug so it doesn't get forgotten.

I took a deeper look and found that this is not always constant. Some messages
(eg. ServiceWorkerMsg_ServiceWorkerRegistered) are set to a thread id sent from
the requester (SWDispatcher).

[nasko, 2014-09-10 05:33:56]

On 2014/09/10 05:28:47, nhiroki wrote:
>
https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
> File content/browser/service_worker/service_worker_provider_host.cc (right):
> 
>
https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
> content/browser/service_worker/service_worker_provider_host.cc:162:
> kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
> On 2014/09/10 04:15:10, nasko wrote:
> > On 2014/09/10 01:50:47, nhiroki wrote:
> > > On 2014/09/10 01:15:56, nasko wrote:
> > > > nit: Is this expected to change in the immediate future? If not, it is
> > > hardcoded
> > > > in all callsites, so it should be removed.
> > > 
> > > Good point. I think we don't have any plan to change it in the near future
> > > (horo@, is this correct?)
> > > 
> > > Removing the constant would need more work (changing all message formats,
> > > SWMessageFilter and so on), so for now I'd like to keep this CL as is and
> fix
> > > them in a separate CL.
> > 
> > Fixing in another CL is fine. Please file a bug so it doesn't get forgotten.
> 
> I took a deeper look and found that this is not always constant. Some messages
> (eg. ServiceWorkerMsg_ServiceWorkerRegistered) are set to a thread id sent
from
> the requester (SWDispatcher).

Ok, so it makes sense then to not include the constant parameter, as it will
never change and is redundant. Let's do it in this CL.

[nhiroki, 2014-09-10 05:47:37]

On 2014/09/10 05:33:56, nasko wrote:
> On 2014/09/10 05:28:47, nhiroki wrote:
> >
>
https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
> > File content/browser/service_worker/service_worker_provider_host.cc (right):
> > 
> >
>
https://codereview.chromium.org/477593007/diff/460001/content/browser/service...
> > content/browser/service_worker/service_worker_provider_host.cc:162:
> > kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));
> > On 2014/09/10 04:15:10, nasko wrote:
> > > On 2014/09/10 01:50:47, nhiroki wrote:
> > > > On 2014/09/10 01:15:56, nasko wrote:
> > > > > nit: Is this expected to change in the immediate future? If not, it is
> > > > hardcoded
> > > > > in all callsites, so it should be removed.
> > > > 
> > > > Good point. I think we don't have any plan to change it in the near
future
> > > > (horo@, is this correct?)
> > > > 
> > > > Removing the constant would need more work (changing all message
formats,
> > > > SWMessageFilter and so on), so for now I'd like to keep this CL as is
and
> > fix
> > > > them in a separate CL.
> > > 
> > > Fixing in another CL is fine. Please file a bug so it doesn't get
forgotten.
> > 
> > I took a deeper look and found that this is not always constant. Some
messages
> > (eg. ServiceWorkerMsg_ServiceWorkerRegistered) are set to a thread id sent
> from
> > the requester (SWDispatcher).
> 
> Ok, so it makes sense then to not include the constant parameter, as it will
> never change and is redundant. Let's do it in this CL.

Sorry, my previous comment needs to further explanation...

I think it's not easy to remove the constant from this message type. Messages to
be handled by SWMessageFilter are requires to have thread_id in its first
argument so that the filter can parse it regardless of message types (see
ServiceWorkerMessageFilter::OverrideTaskRunnerForMessage). So I'd prefer to keep
this constant as is.

[nhiroki, 2014-09-10 08:05:39]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2014-09-10 08:06:36]

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

[commit-bot: I haz the power, 2014-09-10 09:01:26]

Committed patchset #6 (id:520001) as f42eac9167b0b69fee2bcb26ac889e940d8e327c

[commit-bot: I haz the power, 2014-09-10 10:56:13]

Patchset 6 (id:??) landed as
https://crrev.com/71840871e1f8499d843d6a21dee0e361c3ba64f0
Cr-Commit-Position: refs/heads/master@{#294147}