SyncCompositor wait for client before registration

SyncCompositor wait for client before registration

This makes the guarantee that while SyncCompositor is registered, ie
while renderer objects have access to the SyncCompositor object,
|compositor_client_| member is always valid.

BUG=484897
Committed: https://crrev.com/40d29dc71fa2ca5d218a287bc40752eeadb7a202
Cr-Commit-Position: refs/heads/master@{#328627}

[boliu, 2015-05-06 02:07:26]

boliu@chromium.org changed reviewers:
+ hush@chromium.org

[boliu, 2015-05-06 02:07:26]

ptal

[hush (inactive), 2015-05-06 21:57:50]

lgtm

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
File content/browser/android/in_process/synchronous_compositor_impl.cc (left):

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:153: void
SynchronousCompositorImpl::NotifyDidDestroyCompositorToClient() {
there was no code calling this function before you removed it?

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
File content/browser/android/in_process/synchronous_compositor_impl.cc (right):

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:93:
DCHECK_IMPLIES(!compositor_client, compositor_client_);
I'm wondering if the above 2 lines means:
DCHECK_EQ(!!compositor_client, !compositor_client_)

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:156:
SetInputHandler(nullptr);
why pulling this line up here? I guess no big deal, because it just sets
input_handler_ to null

[boliu, 2015-05-06 22:02:49]

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
File content/browser/android/in_process/synchronous_compositor_impl.cc (left):

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:153: void
SynchronousCompositorImpl::NotifyDidDestroyCompositorToClient() {
On 2015/05/06 21:57:49, hush wrote:
> there was no code calling this function before you removed it?

Right. Should have removed this when I added the registry class.

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
File content/browser/android/in_process/synchronous_compositor_impl.cc (right):

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:93:
DCHECK_IMPLIES(!compositor_client, compositor_client_);
On 2015/05/06 21:57:49, hush wrote:
> I'm wondering if the above 2 lines means:
> DCHECK_EQ(!!compositor_client, !compositor_client_)

Umm...yes, yes it does. DCHECK_IMPLIES is clearer though I think?

https://codereview.chromium.org/1127943002/diff/1/content/browser/android/in_...
content/browser/android/in_process/synchronous_compositor_impl.cc:156:
SetInputHandler(nullptr);
On 2015/05/06 21:57:49, hush wrote:
> why pulling this line up here? I guess no big deal, because it just sets
> input_handler_ to null

It also does SetRootLayerScrollOffsetDelegate(NULL);

InputHandler is a renderer object. So I'm trying to make sure that gets detached
first before compositor_client_ becomes invalid

[hush (inactive), 2015-05-06 22:07:34]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2015-05-06 22:08:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1127943002/1

[commit-bot: I haz the power, 2015-05-06 22:16:51]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-05-06 22:18:33]

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