Tiling priorities in Android Webview.

android_webview/browser/shared_renderer_state.cc

Index: android_webview/browser/shared_renderer_state.cc
diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc
index 8845b761cd252c16592d4480003288e53b07de09..f68a4b1517d033205b0852fd41f2053ad8371c30 100644
--- a/android_webview/browser/shared_renderer_state.cc
+++ b/android_webview/browser/shared_renderer_state.cc
@@ -7,6 +7,8 @@
 #include "android_webview/browser/browser_view_renderer_client.h"
 #include "base/bind.h"
 #include "base/location.h"
+#include "cc/output/compositor_frame.h"
+#include "content/public/browser/android/synchronous_compositor.h"
 
 namespace android_webview {
 
@@ -24,6 +26,7 @@ SharedRendererState::SharedRendererState(
       weak_factory_on_ui_thread_(this),
       ui_thread_weak_ptr_(weak_factory_on_ui_thread_.GetWeakPtr()),
       hardware_allowed_(false),
+      parent_draw_constraints_dirty_(false),
       share_context_(NULL) {
   DCHECK(ui_loop_->BelongsToCurrentThread());
   DCHECK(client_on_ui_);
@@ -51,6 +54,22 @@ void SharedRendererState::ClientRequestDrawGLOnUIThread() {
   }
 }
 
+void SharedRendererState::ClientPostInvalidate() {
+  if (ui_loop_->BelongsToCurrentThread()) {

[aelias_OOO_until_Jul13, 2014/07/16 00:32:56]

I don't think this is a good pattern, there should be a set contract about
whether things are run synchronously or not in advance, since some stuff may not
react correctly to reentrancy.  I suggest making this always async.

[hush (inactive), 2014/07/16 20:45:31]

This feels like refatoring. I will do that in a separate code review.
On 2014/07/16 00:32:56, aelias wrote:

[aelias_OOO_until_Jul13, 2014/07/16 21:23:31]

Sorry, that's not how we do things on Chrome-team unless there's high urgency
(which, although this needs to go in soonish, this doesn't reach).  Also, keep
in mind that if you refactor later, you will either have to cherry-pick the
refactoring to 37 or leave trunk different from 37, which will get painful.

[aelias_OOO_until_Jul13, 2014/07/16 21:26:33]

Sorry, I wrote this before seeing Bo's comment that he's planning to change this
area later, which you're probably reacting to.  In that case, it's fine.  Please
keep in mind the refactoring problem though.

[aelias_OOO_until_Jul13, 2014/07/16 21:26:33]

Sorry, I wrote this before seeing Bo's comment that he's planning to change it
later, which you're probably reacting to.  So never mind.  But keep in mind the
cherry-picking problem as you work on this.

[hush (inactive), 2014/07/17 00:05:53]

Yes. I was reacting to Bo's comment about postInvalidate.
Thanks!

On 2014/07/16 21:26:33, aelias wrote:

+    ClientPostInvalidateOnUIThread();
+  } else {
+    ui_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&SharedRendererState::ClientPostInvalidateOnUIThread,
+                   ui_thread_weak_ptr_));
+  }
+}
+
+void SharedRendererState::ClientPostInvalidateOnUIThread() {
+  DCHECK(ui_loop_->BelongsToCurrentThread());
+  client_on_ui_->PostInvalidate();
+}
+
 void SharedRendererState::SetDrawGLInput(scoped_ptr<DrawGLInput> input) {
   base::AutoLock lock(lock_);
   DCHECK(!draw_gl_input_.get());
@@ -62,6 +81,37 @@ scoped_ptr<DrawGLInput> SharedRendererState::PassDrawGLInput() {
   return draw_gl_input_.Pass();
 }
 
+bool SharedRendererState::IsParentDrawConstraintsDirty() {
+  base::AutoLock lock(lock_);
+  return parent_draw_constraints_dirty_;
+}
+
+void SharedRendererState::SetParentDrawConstraintsDirty(bool dirty) {
+  base::AutoLock lock(lock_);
+  parent_draw_constraints_dirty_ = dirty;
+}
+
+void SharedRendererState::PostExternalDrawConstraintsToChildCompositor(
+    const gfx::Rect& clip,
+    const gfx::Transform& transform) {
+  base::AutoLock lock(lock_);

[aelias_OOO_until_Jul13, 2014/07/16 00:32:56]

Chromium coding conventions are to avoid mutexes when possible.  Could we change
this to post tasks on the UI-thread message loop instead?  (It was unavoidable
in the other direction because we don't have a handle to the RenderThread
message-loop, but here we do.)

[hush (inactive), 2014/07/16 20:45:32]

I will do that in a separate code review
On 2014/07/16 00:32:56, aelias wrote:

[boliu, 2014/07/17 01:51:58]

Actually I prefer using lock here over post task. Android frameworks scheduling
of UI and RT really sucks. If we do a post task, we could miss multiple onDraws,
which is pretty bad. So I'd prefer DrawGL update this immediately, and onDraw
always pick up the latest transform. Only use post task to schedule invalidate
when needed.

+  parent_transform_ = transform;
+  parent_clip_ = clip;
+  parent_draw_constraints_dirty_ = true;
+
+  ClientPostInvalidate();
+}
+
+const gfx::Rect& SharedRendererState::ParentClip() const {
+  base::AutoLock lock(lock_);
+  return parent_clip_;
+}
+
+const gfx::Transform& SharedRendererState::ParentTransform() const {
+  base::AutoLock lock(lock_);
+  return parent_transform_;
+}
+
 void SharedRendererState::SetHardwareAllowed(bool allowed) {
   base::AutoLock lock(lock_);
   hardware_allowed_ = allowed;