ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
Committed: https://crrev.com/3cd90229cb808989ac8a468beb93ea5f372fefd6
Cr-Commit-Position: refs/heads/master@{#431833}

[nhiroki, 2016-11-14 02:22:29]

The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 02:22:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nhiroki, 2016-11-14 02:23:56]

Patchset #1 (id:1) has been deleted

[nhiroki, 2016-11-14 02:24:09]

The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 02:24:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nhiroki, 2016-11-14 02:25:06]

Description was changed from

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
==========

to

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
==========

[nhiroki, 2016-11-14 02:25:06]

nhiroki@chromium.org changed reviewers:
+ falken@chromium.org, shimazu@chromium.org

[nhiroki, 2016-11-14 02:25:37]

PTAL, thanks!

[falken, 2016-11-14 02:32:19]

https://codereview.chromium.org/2496073002/diff/20001/content/renderer/servic...
File content/renderer/service_worker/embedded_worker_instance_client_impl.cc
(right):

https://codereview.chromium.org/2496073002/diff/20001/content/renderer/servic...
content/renderer/service_worker/embedded_worker_instance_client_impl.cc:79:
LOG(WARNING) << "Got StopWorker for stopping worker or nonexistent worker";
The comment and LOG are now slightly inconsistent: the comment doesn't mention
Stop called before Start, and also now it's unclear whether a warning is really
needed since the comment seems to say this can legitimately happen.

Maybe we should remove the LOG and update the comment?

[nhiroki, 2016-11-14 04:17:41]

Thank you. Updated.

https://codereview.chromium.org/2496073002/diff/20001/content/renderer/servic...
File content/renderer/service_worker/embedded_worker_instance_client_impl.cc
(right):

https://codereview.chromium.org/2496073002/diff/20001/content/renderer/servic...
content/renderer/service_worker/embedded_worker_instance_client_impl.cc:79:
LOG(WARNING) << "Got StopWorker for stopping worker or nonexistent worker";
On 2016/11/14 02:32:19, falken wrote:
> The comment and LOG are now slightly inconsistent: the comment doesn't mention
> Stop called before Start, and also now it's unclear whether a warning is
really
> needed since the comment seems to say this can legitimately happen.
> 
> Maybe we should remove the LOG and update the comment?

Done.

[falken, 2016-11-14 04:18:31]

lgtm

[nhiroki, 2016-11-14 04:22:39]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2016-11-14 04:22:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 05:48:55]

Description was changed from

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
==========

to

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
==========

[commit-bot: I haz the power, 2016-11-14 05:48:57]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-11-14 05:50:48]

Description was changed from

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014
==========

to

==========
ServiceWorker: Initialize EmbeddedWorkerInstanceClientImpl::wrapper_

This is a follow-up patch for https://codereview.chromium.org/2498573002

EmbeddedWorkerInstanceClientImpl::wrapper_ should be initialized with nullptr
because EmbeddedWorkerInstance::StopWorker() can be called before
EmbeddedWorkerInstance::StartWorker() and uninitialized |wrapper_| causes a
crash.

This is possible when EmbeddedWorkerInstance::StopWorker() is called between
ProcessAllocated() and SendStartWorker() on EmbeddedWorkerInstance::StartWorker.

BUG=664014

Committed: https://crrev.com/3cd90229cb808989ac8a468beb93ea5f372fefd6
Cr-Commit-Position: refs/heads/master@{#431833}
==========

[commit-bot: I haz the power, 2016-11-14 05:50:49]

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