Adding compositor callbacks to RenderWidgetHostViewAndroid

content/browser/android/content_view_core_impl.cc

Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index 6285a0bd87989cdb869bbeed1454ae88ba80c86a..8865d7ea057a630cc93c8bfaf3f2152e1012fb22 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -170,7 +170,8 @@ ContentViewCoreImpl::ContentViewCoreImpl(JNIEnv* env, jobject obj,
           kDefaultVSyncIntervalMicros * kDefaultBrowserCompositeVSyncFraction)),
       view_android_(view_android),
       window_android_(window_android),
-      device_orientation_(0) {
+      device_orientation_(0),
+      compositor_(NULL) {
   CHECK(web_contents) <<
       "A ContentViewCoreImpl should be created with a valid WebContents.";
 
@@ -1595,6 +1596,16 @@ void ContentViewCoreImpl::SendOrientationChangeEventInternal() {
   RenderViewHostImpl* rvhi = static_cast<RenderViewHostImpl*>(
       web_contents_->GetRenderViewHost());
   rvhi->SendOrientationChangeEvent(device_orientation_);
+
+void ContentViewCoreImpl::SetCompositor(Compositor* compositor) {
+  compositor_ = compositor;
+  if (web_contents_) {
+    // Calling SetContentViewCore triggers the call set the ContentViewCore on

[jdduke (slow), 2013/10/16 23:12:26]

Nit: "triggers the call to set"

[powei, 2013/10/19 00:05:18]

Done. Remove.

+    // RenderWidgetHostView, which also allows RenderWidgetHostView to attach
+    // itsetlf as an observer on the compositor.
+    static_cast<WebContentsViewAndroid*>(web_contents_->GetView())
+        ->SetContentViewCore(this);

[no sievers, 2013/10/16 23:34:23]

Can we avoid piggy-backing on this other function? In the past we had a lot of
problems with the complicated relationship between our objects (and unclear
roles), and this is not intuitive. Setting the compositor should have nothing to
do with setting the CVC-WC-RWHV relationship.

See my other comment - if we pass the compositor into the constructor, we don't
need to do this, right?

[powei, 2013/10/19 00:05:18]

Done.

+  }

[jdduke (slow), 2013/10/16 23:12:26]

So, the desired effect here is simply to trigger RWHVA to add/remove itself as
an observer to the Compositor? Why don't we do that directly right here
(RWHVA::SetCompositor()), instead of relying on side effects of
SetContentViewCore()? Or do we need to go through the attach/detach layers cycle
when SetCompositor() is called?

[powei, 2013/10/19 00:05:18]

Done.

 }
 
 // This is called for each ContentView.
@@ -1615,4 +1626,6 @@ bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
+

[powei, 2013/10/16 22:42:35]

empty lines to be removed.

[powei, 2013/10/19 00:05:18]

Done.

+
 }  // namespace content