Clean up DelegatedFrameHost's interface to its client

Clean up DelegatedFrameHost's interface to its client

Rename client methods to start with the prefix DelegatedFrame, to indicate
to which interface they belong (not doing this had resulted in each RWHV
having IsHidden and IsVisible functions, which weren't opposites of each other).

Do not access the RenderWidgetHostImpl directly in DelegatedFrameHost, but
rather create client methods for all functions that will need to access it (and,
for most of these methods, send the IPCs directly on the RWHI, rather than
re-looking it up by its process and routing).

Clean up the interfaces used for testing.

BUG=(paying down technical debt)

Committed: https://crrev.com/529de3d7572e558713a21e199de51e771ce2e8b3
Cr-Commit-Position: refs/heads/master@{#313028}

[ccameron, 2015-01-23 21:45:33]

ccameron@chromium.org changed reviewers:
+ danakj@chromium.org

[ccameron, 2015-01-23 23:20:48]

There also appears to be consensus that Android to switch over to using DFH
(barring unexpected complications), so doing some long-overdue cleanup in the
neighborhood.

I also plan to break DFH tests off from RHWVAura tests, but that was a bit too
much motion for one patch.

[danakj, 2015-01-24 00:06:06]

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
File content/browser/compositor/delegated_frame_host.cc (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.cc:105: if (!resize_lock_)
This can't return null if ShouldCreate returns true, can it?

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.cc:152: if
(!intercept_copy_of_output_callback_for_testing_.is_null())
Im a little meh about this as a pattern, but, also, not sure what's better. We
could add a client RCOO() call back and have it just call through to the layer..
kinda meh. So I guess this is fine, if you don't have any other cool ideas
kicking around :)

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
File content/browser/compositor/delegated_frame_host.h (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.h:47: virtual bool
DelegatedFrameIsVisible() const = 0;
I think DelegatedFrameHost would be a slightly better name here than
DelegatedFrame, at least I think it would be a bit more clear for some of these.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2597: // On
Windows and Linux, holding pointer moves will not help throttling
indenting. git cl format?

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2626: void
RenderWidgetHostViewAura::DelegatedFrameSwapWasAcked(
I think I'd prefer more like SendFrameSwapAck instead of "WasAcked". Ie say what
the function will do.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2633: void
RenderWidgetHostViewAura::DelegatedFrameResourcesWereReturned(
ditto

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2640: void
RenderWidgetHostViewAura::DelegatedFrameLostResources() {
on the fence for this one.. not sure so you can leave it

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1145: 
random whitespace

[ccameron, 2015-01-24 01:26:00]

Patchset #6 (id:100001) has been deleted

[ccameron, 2015-01-24 01:27:21]

Thanks -- updated.

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
File content/browser/compositor/delegated_frame_host.cc (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.cc:105: if (!resize_lock_)
On 2015/01/24 00:06:06, danakj wrote:
> This can't return null if ShouldCreate returns true, can it?

Nope, it can't -- I changed that between the last 2 patchsets, and didn't update
this. Fixed.

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.cc:152: if
(!intercept_copy_of_output_callback_for_testing_.is_null())
On 2015/01/24 00:06:06, danakj wrote:
> Im a little meh about this as a pattern, but, also, not sure what's better. We
> could add a client RCOO() call back and have it just call through to the
layer..
> kinda meh. So I guess this is fine, if you don't have any other cool ideas
> kicking around :)

I'm really meh about this pattern too ... prior to this change, there was the
RCOO call in the client, and that seemed worse (it complicated 2 classes instead
of one).

I updated this to use a helper function to at least eliminate the repeated
if-then-else.

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
File content/browser/compositor/delegated_frame_host.h (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/composit...
content/browser/compositor/delegated_frame_host.h:47: virtual bool
DelegatedFrameIsVisible() const = 0;
On 2015/01/24 00:06:06, danakj wrote:
> I think DelegatedFrameHost would be a slightly better name here than
> DelegatedFrame, at least I think it would be a bit more clear for some of
these.

I agree -- the motivation was to save the 4 characters, which is pretty weak.
Changed to DFH.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2597: // On
Windows and Linux, holding pointer moves will not help throttling
On 2015/01/24 00:06:06, danakj wrote:
> indenting. git cl format?

Yup .. get cl format messes that up. Fixed.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2626: void
RenderWidgetHostViewAura::DelegatedFrameSwapWasAcked(
On 2015/01/24 00:06:06, danakj wrote:
> I think I'd prefer more like SendFrameSwapAck instead of "WasAcked". Ie say
what
> the function will do.

Done. I was on the fence about "what it does" versus "when it's called", but
"what it does" is better.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2633: void
RenderWidgetHostViewAura::DelegatedFrameResourcesWereReturned(
On 2015/01/24 00:06:06, danakj wrote:
> ditto

Done.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura.cc:2640: void
RenderWidgetHostViewAura::DelegatedFrameLostResources() {
On 2015/01/24 00:06:06, danakj wrote:
> on the fence for this one.. not sure so you can leave it

DFH-OnLostCompositorResources.

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
(right):

https://codereview.chromium.org/864333004/diff/80001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1145: 
On 2015/01/24 00:06:06, danakj wrote:
> random whitespace

Done.

[danakj, 2015-01-24 01:45:02]

LGTM

https://codereview.chromium.org/864333004/diff/120001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/864333004/diff/120001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2597: // On
Windows and Linux, holding pointer moves will not help throttling
You probably need to move the comment inside the #if to make Git cl format work.
Maybe worth doing to save others awkwardness.

[ccameron, 2015-01-24 04:07:17]

Thanks!

Updated the #if order.

[ccameron, 2015-01-24 04:07:20]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-01-24 04:07:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/864333004/140001

[commit-bot: I haz the power, 2015-01-24 05:10:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-24 05:10:35]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ccameron, 2015-01-24 07:52:57]

Patchset #10 (id:200001) has been deleted

[ccameron, 2015-01-24 07:53:07]

Patchset #9 (id:180001) has been deleted

[ccameron, 2015-01-24 07:53:16]

Patchset #8 (id:160001) has been deleted

[ccameron, 2015-01-24 07:56:17]

Patchset #9 (id:240001) has been deleted

[ccameron, 2015-01-24 07:56:22]

Patchset #8 (id:220001) has been deleted

[ccameron, 2015-01-24 18:32:10]

It turns out it's not quite possible to make the ui::Layer be an immutable
property of the class. It's very unclear what the lifetime of the ui::Layer is
(were we close, or not even close?). Anyway, I reverted that part of the patch,
and everything appears to work, so I'll send it through.

[ccameron, 2015-01-24 18:33:10]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-01-24 18:34:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/864333004/280001

[commit-bot: I haz the power, 2015-01-24 19:44:22]

Committed patchset #9 (id:280001)

[commit-bot: I haz the power, 2015-01-24 19:45:42]

Patchset 9 (id:??) landed as
https://crrev.com/529de3d7572e558713a21e199de51e771ce2e8b3
Cr-Commit-Position: refs/heads/master@{#313028}