Move activate event dispatching out of ServiceWorkerVersion.

Move activate event dispatching out of ServiceWorkerVersion.

BUG=570820
Committed: https://crrev.com/6d088aa44292838f9c3f08b86f7fb64c868568ac
Cr-Commit-Position: refs/heads/master@{#372794}

[Marijn Kruisselbrink, 2016-01-29 22:36:29]

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

[nhiroki, 2016-02-01 04:23:19]

lgtm

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_registration.cc (right):

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_registration.cc:352: <<
activating_version->status();
"<< activating_version->status()" wouldn't be necessary because DCHECK_EQ prints
the value when the assertion fails.

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version_unittest.cc (right):

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version_unittest.cc:581:
TEST_F(ServiceWorkerVersionTest, ActivateAndWaitCompletion) {
After this CL, ServiceWorkerRegistrationTest would be more appropriate place for
this test. Can you leave a TODO comment about that?

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version_unittest.cc:590:
base::Bind(&VerifyCalled, &status_change_called));
This should be called before SimulateDispatchEvent() because the function
internally runs the message loop and the 'activate' event is completed before
this line.

[Marijn Kruisselbrink, 2016-02-01 19:43:08]

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_registration.cc (right):

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_registration.cc:352: <<
activating_version->status();
On 2016/02/01 at 04:23:19, nhiroki wrote:
> "<< activating_version->status()" wouldn't be necessary because DCHECK_EQ
prints the value when the assertion fails.

Good point. Not sure why the old code in SWVersion had the << status() part.

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_version_unittest.cc (right):

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version_unittest.cc:581:
TEST_F(ServiceWorkerVersionTest, ActivateAndWaitCompletion) {
On 2016/02/01 at 04:23:19, nhiroki wrote:
> After this CL, ServiceWorkerRegistrationTest would be more appropriate place
for this test. Can you leave a TODO comment about that?

Yes, I agree that this test (and really the one above it for the install case)
don't really make much sense since the changes were made to have SWVersion not
update the status as part of the event dispatching. I added a TODO comment for
that.

https://codereview.chromium.org/1647323002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_version_unittest.cc:590:
base::Bind(&VerifyCalled, &status_change_called));
On 2016/02/01 at 04:23:19, nhiroki wrote:
> This should be called before SimulateDispatchEvent() because the function
internally runs the message loop and the 'activate' event is completed before
this line.

Done

[Marijn Kruisselbrink, 2016-02-01 20:05:02]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-02-01 20:05:03]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1647323002/#ps20001
(title: "address comments")

[commit-bot: I haz the power, 2016-02-01 20:06:38]

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

[commit-bot: I haz the power, 2016-02-01 22:45:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-01 22:46:16]

Description was changed from

==========
Move activate event dispatching out of ServiceWorkerVersion.

BUG=570820
==========

to

==========
Move activate event dispatching out of ServiceWorkerVersion.

BUG=570820

Committed: https://crrev.com/6d088aa44292838f9c3f08b86f7fb64c868568ac
Cr-Commit-Position: refs/heads/master@{#372794}
==========

[commit-bot: I haz the power, 2016-02-01 22:46:17]

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