cc: Do not request redraw on commit when impl-side painting

cc/thread_proxy.cc

Index: cc/thread_proxy.cc
diff --git a/cc/thread_proxy.cc b/cc/thread_proxy.cc
index b4c6a079d841126debc19fab76e563ecf09e86c1..1ee6e8636981639ec28f5106385a031b1f6e720e 100644
--- a/cc/thread_proxy.cc
+++ b/cc/thread_proxy.cc
@@ -411,7 +411,8 @@ void ThreadProxy::setNeedsRedraw()
     DCHECK(isMainThread());
     TRACE_EVENT0("cc", "ThreadProxy::setNeedsRedraw");
     Proxy::implThread()->postTask(base::Bind(&ThreadProxy::setFullRootLayerDamageOnImplThread, m_implThreadWeakPtr));
-    Proxy::implThread()->postTask(base::Bind(&ThreadProxy::setNeedsRedrawOnImplThread, m_implThreadWeakPtr));
+    if (!m_layerTreeHost->settings().implSidePainting)

[enne (OOO), 2013/01/10 03:45:31]

I understand moving the conditional in the commit function, but why is this main
thread setNeedsRedraw conditional as well?

[nduca, 2013/01/10 11:01:09]

yeah this makes no sense to me

[brianderson, 2013/01/10 18:55:32]

Ah, I saw this being called after a commit and assumed it was called after every
commit.  It is only called after the first commit after a tab switch.  Will get
rid of this.

+        Proxy::implThread()->postTask(base::Bind(&ThreadProxy::setNeedsRedrawOnImplThread, m_implThreadWeakPtr));
 }
 
 void ThreadProxy::setDeferCommits(bool deferCommits)
@@ -732,6 +733,10 @@ void ThreadProxy::scheduledActionCommit()
 
     // SetVisible kicks off the next scheduler action, so this must be last.
     m_schedulerOnImplThread->setVisible(m_layerTreeHostImpl->visible());
+

[nduca, 2013/01/10 11:01:09]

I dislike this.

I'd rather this was all in the state machine somehow. It smells like what you
need to do is say "if we commit && impl side painting mode, dont set needs
redraw."

[brianderson, 2013/01/10 18:55:32]

I had moved it out here because all the other setNeedsRedraws are initiated from
the this file and it was difficult for me to discover that a redraw was being
initiated from within the state machine itself, two levels lower.

By calling setNeedsRedraw here, do I run the risk of drawing at the wrong time
or drawing when we don't need to?

+    // This will also kick off the a scheduler action.

[enne (OOO), 2013/01/10 03:45:31]

Can you reconcile this with the previous comment that says "this must be last"?

+    if (!m_layerTreeHost->settings().implSidePainting)
+        m_schedulerOnImplThread->setNeedsRedraw();

[enne (OOO), 2013/01/10 03:45:31]

I feel like removing the redraw here needs to correspondingly have a
setNeedsRedraw in activatePendingTree.

[brianderson, 2013/01/10 18:55:32]

This is meant to go on top of my other patch, which does setNeedsRedraw in
onHasPendingTreeStateChanged.

 }
 
 void ThreadProxy::scheduledActionBeginContextRecreation()