Consolidate UpdateFrameInfo and SwapCompositorFrame.

Consolidate UpdateFrameInfo and SwapCompositorFrame.

The 'frame info' comes from the CompositorFrame message, and the fields
are useful only if the update implies a Swap (--composite-to-mailbox or
ubercomp). (In other words, a scroll offset is not useful if you don't
know what frame it is for.)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194669

[no sievers, 2013-04-15 23:10:53]

ptal.

[piman, 2013-04-15 23:21:05]

LGTM - this makes me very happy :)

[Ted C, 2013-04-15 23:33:09]

lgtm

[no sievers, 2013-04-16 00:02:55]

+Chris Palmer for removal of unused message in content/common/view_messages.h

[Sami, 2013-04-16 09:44:21]

Great cleanup, lgtm.

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
content/browser/renderer_host/render_widget_host_view_android.cc:507: // Always
let ContentViewCore know about the new frame first, so it can decide
Should this be after the early-out below?

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:251:
// Whether we received a new frame, since consumePendingRendererFrame() was last
called.
Nit: no comma.

[no sievers, 2013-04-16 16:06:32]

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
content/browser/renderer_host/render_widget_host_view_android.cc:507: // Always
let ContentViewCore know about the new frame first, so it can decide
On 2013/04/16 09:44:21, Sami wrote:
> Should this be after the early-out below?

ContentViewCoreImpl has these UpdateFrameInfo callbacks. They are used by
WebView, and I think would break since that uses SurfaceTexture transport. But
hopefully those can go away in the new path.

[Sami, 2013-04-16 16:19:20]

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/14210004/diff/1/content/browser/renderer_host...
content/browser/renderer_host/render_widget_host_view_android.cc:507: // Always
let ContentViewCore know about the new frame first, so it can decide
On 2013/04/16 16:06:32, Daniel Sievers wrote:
> On 2013/04/16 09:44:21, Sami wrote:
> > Should this be after the early-out below?
> 
> ContentViewCoreImpl has these UpdateFrameInfo callbacks. They are used by
> WebView, and I think would break since that uses SurfaceTexture transport. But
> hopefully those can go away in the new path.

Ah, gotcha. We still need to support the SurfaceTexture path.

[palmer, 2013-04-16 17:21:30]

lgtm

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:802:
* Indicate that the browser compositor has consumed a pending renderer frame.
This doesn't quite agree with the @return comment. I'd get rid of this and just
say:

@return Whether or not the browser has consumed a pending renderer frame.

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:806:
public boolean consumePendingRendererFrame() {
Should be named |hasConsumed...|?

[no sievers, 2013-04-16 18:37:04]

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
(right):

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:251:
// Whether we received a new frame, since consumePendingRendererFrame() was last
called.
On 2013/04/16 09:44:21, Sami wrote:
> Nit: no comma.

Done.

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:802:
* Indicate that the browser compositor has consumed a pending renderer frame.
On 2013/04/16 17:21:30, Chromium Palmer wrote:
> This doesn't quite agree with the @return comment. I'd get rid of this and
just
> say:
> 
> @return Whether or not the browser has consumed a pending renderer frame.

I have updated the comment....

https://codereview.chromium.org/14210004/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java:806:
public boolean consumePendingRendererFrame() {
On 2013/04/16 17:21:30, Chromium Palmer wrote:
> Should be named |hasConsumed...|?

...and related to this question: Yes and no :)
The reason I got to cleaning this up is that I was considering making this a
didConsume/hasConsumed() method (for the purpose of swap throttling), but the
problem is that Chrome currently calls this before it actually consumes it (it
calls it when it has the intention of consuming it).

(But at the same time I'm not fully sure yet this is the right place, because
there is a one compositor to many views  relationship, but now I'm
derailing....)

[no sievers, 2013-04-17 21:02:00]

Committed patchset #3 manually as r194669 (presubmit successful).