Create and use BlinkInterfaceProvider WeakPtrs on the same thread

Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645
Committed: https://crrev.com/c6f247ed04bc2e28157968a9f7e8f195012ae614
Cr-Commit-Position: refs/heads/master@{#430663}

[Peter Beverloo, 2016-11-03 19:29:48]

Description was changed from

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

BUG=649645
==========

to

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645
==========

[Peter Beverloo, 2016-11-03 19:35:04]

peter@chromium.org changed reviewers:
+ alexclarke@chromium.org, nasko@chromium.org

[Peter Beverloo, 2016-11-03 19:35:05]

+nasko, alexclarke -- are either of you the right person to review this, or
would know a more appropriate person?

I have no idea whether this is correct. The alternative would be to make
the BlinkInterfaceProviderImpl refcounted, and ensure it gets destroyed
on the main thread. I did verify that the crash went away.

Other pieces of the code use this paradigm too: jingle's thread wrapper,
Remoting's host starter, WebRTC, the NQE and the in-process command
buffer. (cs/ on "weak_ptr_; weak_ptr_factory_").

[alex clarke (OOO till 29th), 2016-11-04 10:07:01]

LGTM This should be safer since the inner Flag is  RefCountedThreadSafe  

+Daniel who is more familiar than me about the inner workings of weak ptr.

[Peter Beverloo, 2016-11-07 21:27:33]

peter@chromium.org changed reviewers:
+ avi@chromium.org

[Peter Beverloo, 2016-11-07 21:27:34]

+avi for //content since Nasko may still be out?

[Avi (use Gerrit), 2016-11-07 21:39:03]

LGTM stamp for when all the detail people are happy.

[dcheng, 2016-11-07 22:12:31]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-11-07 22:12:32]

LGTM

+wez, danakj: I've noticed this come up in several places. It seems a bit
unfortunate to have to do to this: in particular, it's really easy to use the
WeakPtrFactory by accident instead of the saved WeakPtr copy.

I wonder if we should have a WeakPtrFactory variant that:
- We eagerly initialize the flag in the ctor
- Only allow it to be invalidated once

Dunno what we'd call it though...

[Peter Beverloo, 2016-11-08 17:06:28]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2016-11-08 17:06:51]

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-08 18:31:04]

Description was changed from

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645
==========

to

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645
==========

[commit-bot: I haz the power, 2016-11-08 18:31:07]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-08 18:38:52]

Description was changed from

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645
==========

to

==========
Create and use BlinkInterfaceProvider WeakPtrs on the same thread

A race condition seems to be occurring where the reference to
WeakReference::Flag in WeakReferenceOwner is in process of being destroyed
while a new reference is being added. This happens when a document having
Web Workers is being destroyed (due to a reload) while, on the thread
specific to a worker, an API is used that binds to a Mojo Interface
Provider.

Creating the weak pointers on arbitrary threads, even though they
consistently get dereferenced on the main thread, seems to be unsafe
in this situation.

Instead, create a single WeakPtr<> on the main thread during construction,
a copy of which can be made on the worker threads. This will increase the
reference count of the WeakReference::Flag, which is a thread-safe ref
counted object.

BUG=649645

Committed: https://crrev.com/c6f247ed04bc2e28157968a9f7e8f195012ae614
Cr-Commit-Position: refs/heads/master@{#430663}
==========

[commit-bot: I haz the power, 2016-11-08 18:38:53]

Patchset 1 (id:??) landed as
https://crrev.com/c6f247ed04bc2e28157968a9f7e8f195012ae614
Cr-Commit-Position: refs/heads/master@{#430663}