Move SynchronousCompositor into content/browser

Move SynchronousCompositor into content/browser

This means all the hookup can remain in content/ so less internal
details need to be revealed in the public API long-term.
Follow-up steps will rationalize the embedder API for these features
(currently its split across numerous switches and
ContentRendererClient upcalls, that can now be unified).

Initial move implemented with:-

$ tools/git/move_source_file.py \
   content/renderer/android/synchronous_compositor_* \
   content/browser/android/in_process/

BUG=

[joth, 2013-05-29 06:05:10]

Doesn't even compile yet!! So not ready for nits...
just wanted to pass this by you  for initial thought on whether this will hang
together for both your needs? Basically, with this in place, we can then find
our SynchronousCompositorImpl instance whenever we want for a given WebContents
instance, all from within content/browser.


I've built this on top of your refactor
(https://codereview.chromium.org/16119003) Marcin, but depending how it falls
together, maybe we just squash them both into this one patch.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:99:
capabilities_.deferred_gl_initialization = true;
// Do not call out to SynchronousCompositorImpl here as we're on the wrong
thread.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:105:
delegate_->DidDestroySynchronousOutputSurface();
pass |this| for consistency with the DidBindOutputSurface() call?

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:119:
// here.
My current thinking is to delete the SynchronousCompositorOutputSurfaceDelegate
abstraction and have the sync compositor and its output surface be mutually
aware of each other.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:120:
SynchronousCompositorImpl::DidBindOutputSurface(routing_id_, this);
this could be 
SynchronousCompositorImpl::FromRoutingID(routing_id_)->DidBindOutputSurface(this);

or even
delegate = SynchronousCompositorImpl::FromRoutingID(routing_id_);
delegate->DidBindOutputSurface(this);

[mkosiba (inactive), 2013-05-29 15:31:58]

SGTM. My original CL is still blocked on an OWNER's review so if you want to
squash this one and mine together I'm fine with that.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:105:
delegate_->DidDestroySynchronousOutputSurface();
On 2013/05/29 06:05:10, joth wrote:
> pass |this| for consistency with the DidBindOutputSurface() call?

sounds like a good idea. we can add a DCHECK to check that the calls match up.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:119:
// here.
On 2013/05/29 06:05:10, joth wrote:
> My current thinking is to delete the
SynchronousCompositorOutputSurfaceDelegate
> abstraction and have the sync compositor and its output surface be mutually
> aware of each other.

Umm.. SynchronousCompositorImpl will end up containing quite a few methods, so
having separate intefaces to organize those seems like a nice idea to me. Does
removing the delegate buy us any flexibility or is it just a convenience thing?

IOW. The pattern for getting the SCImpl bellow is fine, it's just that I'd
perfer this class view SCImpl via the delegate interface, so maybe add a method
like this

SynchronousCompositorOutputSurfaceDeleagate* delegate() {
  return SynchronousCompositorImpl::FromRoutingId(routing_id);
}

and use it instead of delegate_ ?

[joth, 2013-05-29 18:19:00]

On 29 May 2013 08:31, <mkosiba@chromium.org> wrote:

> SGTM. My original CL is still blocked on an OWNER's review so if you want
> to
> squash this one and mine together I'm fine with that.
>
> OK I'll get this to where I'm happy, then see where we are.


>
>
> https://codereview.chromium.**org/16119003/diff/1/content/**
>
browser/android/in_process/**synchronous_compositor_output_**surface.cc<https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_process/synchronous_compositor_output_surface.cc>
> File
> content/browser/android/in_**process/synchronous_**
> compositor_output_surface.cc
> (right):
>
> https://codereview.chromium.**org/16119003/diff/1/content/**
> browser/android/in_process/**synchronous_compositor_output_**
>
surface.cc#newcode105<https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_process/synchronous_compositor_output_surface.cc#newcode105>
> content/browser/android/in_**process/synchronous_**
> compositor_output_surface.cc:**105:
> delegate_->**DidDestroySynchronousOutputSur**face();
> On 2013/05/29 06:05:10, joth wrote:
>
>> pass |this| for consistency with the DidBindOutputSurface() call?
>>
>
> sounds like a good idea. we can add a DCHECK to check that the calls
> match up.
>
> Also I realized this is now the analog to aw/browser
InProcessViewRenderer's hand-over logic:

  // Allow for transient hand-over when two compositors may reference
// a single client.  if (compositor_ == compositor)    compositor_ =
NULL;


So I'll add that here. (in practice we only have one RVH over a WebContents
lifetime these days due to single-process, but I'd rather not bake that
assumption more deeply than I need to given it's not true for the rest of
chrome)



> https://codereview.chromium.**org/16119003/diff/1/content/**
> browser/android/in_process/**synchronous_compositor_output_**
>
surface.cc#newcode119<https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_process/synchronous_compositor_output_surface.cc#newcode119>
> content/browser/android/in_**process/synchronous_**
> compositor_output_surface.cc:**119:
> // here.
> On 2013/05/29 06:05:10, joth wrote:
>
>> My current thinking is to delete the
>>
> SynchronousCompositorOutputSur**faceDelegate
>
>> abstraction and have the sync compositor and its output surface be
>>
> mutually
>
>> aware of each other.
>>
>
> Umm.. SynchronousCompositorImpl will end up containing quite a few
> methods, so having separate intefaces to organize those seems like a
> nice idea to me. Does removing the delegate buy us any flexibility or is
> it just a convenience thing?
>
> IOW. The pattern for getting the SCImpl bellow is fine, it's just that
> I'd perfer this class view SCImpl via the delegate interface, so maybe
> add a method like this
>
> SynchronousCompositorOutputSur**faceDeleagate* delegate() {
>   return SynchronousCompositorImpl::**FromRoutingId(routing_id);
> }
>
> yes, that's seems a good compromise - collects the static dependency
knowledge in one place. I'll give that a go.
Thanks!



> and use it instead of delegate_ ?
>
>
https://codereview.chromium.**org/16119003/<https://codereview.chromium.org/1...
>

[joth, 2013-05-29 20:19:07]

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
File content/browser/android/in_process/synchronous_compositor_output_surface.cc
(right):

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:99:
capabilities_.deferred_gl_initialization = true;
On 2013/05/29 06:05:10, joth wrote:
> // Do not call out to SynchronousCompositorImpl here as we're on the wrong
> thread.

Done.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:105:
delegate_->DidDestroySynchronousOutputSurface();
On 2013/05/29 15:31:58, mkosiba wrote:
> On 2013/05/29 06:05:10, joth wrote:
> > pass |this| for consistency with the DidBindOutputSurface() call?
> 
> sounds like a good idea. we can add a DCHECK to check that the calls match up.

Done.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:119:
// here.
On 2013/05/29 15:31:58, mkosiba wrote:
> On 2013/05/29 06:05:10, joth wrote:
> > My current thinking is to delete the
> SynchronousCompositorOutputSurfaceDelegate
> > abstraction and have the sync compositor and its output surface be mutually
> > aware of each other.
> 
> Umm.. SynchronousCompositorImpl will end up containing quite a few methods, so
> having separate intefaces to organize those seems like a nice idea to me. Does
> removing the delegate buy us any flexibility or is it just a convenience
thing?
> 
> IOW. The pattern for getting the SCImpl bellow is fine, it's just that I'd
> perfer this class view SCImpl via the delegate interface, so maybe add a
method
> like this
> 
> SynchronousCompositorOutputSurfaceDeleagate* delegate() {
>   return SynchronousCompositorImpl::FromRoutingId(routing_id);
> }
> 
> and use it instead of delegate_ ?

Done.

https://codereview.chromium.org/16119003/diff/1/content/browser/android/in_pr...
content/browser/android/in_process/synchronous_compositor_output_surface.cc:120:
SynchronousCompositorImpl::DidBindOutputSurface(routing_id_, this);
On 2013/05/29 06:05:10, joth wrote:
> this could be 
>
SynchronousCompositorImpl::FromRoutingID(routing_id_)->DidBindOutputSurface(this);
> 
> or even
> delegate = SynchronousCompositorImpl::FromRoutingID(routing_id_);
> delegate->DidBindOutputSurface(this);

(obsolete)

[joth, 2013-05-29 21:45:53]

+jam, jamesr

jam:  this is my proposed solution to the issues you mentioned in
https://codereview.chromium.org/15720007/ : it will avoid bouncing InputEvents
around the houses, from content/browser -> android_webview/browser ->
content/render, and also completely removes need to promote the internal
InputEventAckState type into the public API.
This means content/ is now fully complicit of the single process sync
compositing flow but this means we can avoid a lot of mess like this into
content public API, and the more we work on this the more natural it seems to
handle more of this fully inside content/


I intend to make a followup patch to this to move  the public API from
content/public/renderer/android/synchronous_compositor* over to
content/public/browser/android, and so can delete a_w/browser/in_process/  but
would appreciate initial comments you may have on this overall approach.

Thanks

[joth, 2013-05-30 01:04:49]

This patch is obsolete! please look at https://codereview.chromium.org/15851006/
 instead which includes the corresponding API file moves too.