Android Browser Compositor: Add ScheduleComposite() callback.

content/shell/android/shell_manager.cc

Index: content/shell/android/shell_manager.cc
diff --git a/content/shell/android/shell_manager.cc b/content/shell/android/shell_manager.cc
index a7b6b263b25615e9c5307cded799bfe974496c03..e1fdf2ab1a9f00f387b2e56fe0c9d95857ad165d 100644
--- a/content/shell/android/shell_manager.cc
+++ b/content/shell/android/shell_manager.cc
@@ -30,10 +30,16 @@ using content::DrawDelegate;
 
 namespace {
 
+class CompositorClient : public Compositor::Client {
+ public:
+  virtual void ScheduleComposite() OVERRIDE;
+};
+
 struct GlobalState {
   base::android::ScopedJavaGlobalRef<jobject> j_obj;
   scoped_ptr<content::Compositor> compositor;
   scoped_ptr<WebKit::WebLayer> root_layer;
+  CompositorClient client;

[piman, 2012/10/12 20:53:06]

On destruction, you will destroy client - which is still referenced by
compositor - before you destroy compositor, thus violating the contract that the
client must outlive the compositor.
I'm pretty sure destroying compositor can involve doing layer modifications
which will call ScheduleComposite, which will then crash.

You can probably trivially solve this by moving client before compositor.

(note: same applies to testshell/tab_manager)

[no sievers, 2012/10/12 22:33:51]

Done.

 };
 
 base::LazyInstance<GlobalState> g_global_state = LAZY_INSTANCE_INITIALIZER;
@@ -42,18 +48,10 @@ content::Compositor* GetCompositor() {
   return g_global_state.Get().compositor.get();
 }
 
-static void SurfacePresented(
-    const DrawDelegate::SurfacePresentedCallback& callback,
-    uint32 sync_point) {
-  callback.Run(sync_point);
-}
-
-static void SurfaceUpdated(
-    uint64 texture,
-    content::RenderWidgetHostView* view,
-    const DrawDelegate::SurfacePresentedCallback& callback) {
-  GetCompositor()->OnSurfaceUpdated(base::Bind(
-      &SurfacePresented, callback));
+void CompositorClient::ScheduleComposite() {
+  // Composite immediately

[piman, 2012/10/12 20:53:06]

I'm pretty sure you don't want to do that.
This means that whenever you change any layer property, you will draw a new
frame.
E.g. changing the layer size and then the layer transform in the same function
will draw one frame with the new size/old transform, and then one frame with the
new size/new transform.
It'll be very slow too.

I suggest posting a task instead.

(note: same applies to testshell/tab_manager)

[no sievers, 2012/10/12 22:33:51]

Done.

+  if (GetCompositor())
+    GetCompositor()->Composite();
 }
 
 } // anonymous namespace
@@ -64,7 +62,8 @@ jobject CreateShellView() {
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!GetCompositor()) {
     Compositor::Initialize();
-    g_global_state.Get().compositor.reset(Compositor::Create());
+    g_global_state.Get().compositor.reset(Compositor::Create(
+        &g_global_state.Get().client));
     DCHECK(!g_global_state.Get().root_layer.get());
     g_global_state.Get().root_layer.reset(WebKit::WebLayer::create());
   }
@@ -80,9 +79,6 @@ bool RegisterShellManager(JNIEnv* env) {
 static void Init(JNIEnv* env, jclass clazz, jobject obj) {
   g_global_state.Get().j_obj.Reset(
       base::android::ScopedJavaLocalRef<jobject>(env, obj));
-  DrawDelegate::SurfaceUpdatedCallback cb = base::Bind(
-      &SurfaceUpdated);
-  DrawDelegate::GetInstance()->SetUpdateCallback(cb);
 }
 
 static void SurfaceCreated(