Android: Remove ContentViewCore::GetLayer() from public interface

Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764
Committed: https://crrev.com/77e87e6a068ab2297d4107bd38f81de4d24eebee
Cr-Commit-Position: refs/heads/master@{#408772}

[no sievers, 2016-07-23 00:36:54]

Description was changed from

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some call sites too and native ContentViewCore
references, which were mostly unnecessary indirections through
ContentViewCore which can use WebContents directly.

BUG=626764
==========

to

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some call sites too and native ContentViewCore
references, which were mostly unnecessary indirections through
ContentViewCore which can use WebContents directly.

BUG=626764
==========

[no sievers, 2016-07-23 00:36:55]

sievers@chromium.org changed reviewers:
+ tedchoc@chromium.org

[no sievers, 2016-07-23 00:37:04]

Ted, ptal! Thanks.

[no sievers, 2016-07-23 00:37:56]

Description was changed from

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some call sites too and native ContentViewCore
references, which were mostly unnecessary indirections through
ContentViewCore which can use WebContents directly.

BUG=626764
==========

to

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764
==========

[no sievers, 2016-07-23 00:38:26]

The CQ bit was checked by sievers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-23 00:38:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-23 00:58:56]

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

[commit-bot: I haz the power, 2016-07-23 00:58:57]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Ted C, 2016-07-25 23:46:54]

lgtm but it looks like you have some broken tests as well

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
File chrome/browser/android/compositor/tab_content_manager.cc (left):

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:268:
pending_tab_readbacks_.set(tab_id, std::move(readback_request));
any idea why it was done this way in the past?  does the in place set change the
destruction ordering (old way was, destroy old one, run new one -- the new code
looks like, run new one, destroy old one)...but I might be way off here.

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
File chrome/browser/android/compositor/tab_content_manager.cc (right):

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:222:
content::WebContents::FromJavaWebContents(jweb_contents.obj());
not an entirely fair comment since you're just changing the params, but
webcontents is available from tab via web_contents(), can we just use that
instead of passing tab and web contents?

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:242: web_contents,
thumbnail_scale, readback_done_callback)));
this indenting looks off, shouldn't it be like it was before?  or is 4 from it's
"logical" parent the correct thing to do in c++ land?

[no sievers, 2016-07-29 20:54:25]

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
File chrome/browser/android/compositor/tab_content_manager.cc (left):

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:268:
pending_tab_readbacks_.set(tab_id, std::move(readback_request));
On 2016/07/25 23:46:53, Ted C wrote:
> any idea why it was done this way in the past?  does the in place set change
the
> destruction ordering (old way was, destroy old one, run new one -- the new
code
> looks like, run new one, destroy old one)...but I might be way off here.

Yes, true, but no idea why it did it that way. It shouldn't matter since
anything in the map would have already done its calls to kick off the readback,
and also doesn't do anything in the destructor.

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
File chrome/browser/android/compositor/tab_content_manager.cc (right):

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:222:
content::WebContents::FromJavaWebContents(jweb_contents.obj());
On 2016/07/25 23:46:54, Ted C wrote:
> not an entirely fair comment since you're just changing the params, but
> webcontents is available from tab via web_contents(), can we just use that
> instead of passing tab and web contents?

Good point, even better. Done.

https://codereview.chromium.org/2179563003/diff/1/chrome/browser/android/comp...
chrome/browser/android/compositor/tab_content_manager.cc:242: web_contents,
thumbnail_scale, readback_done_callback)));
On 2016/07/25 23:46:53, Ted C wrote:
> this indenting looks off, shouldn't it be like it was before?  or is 4 from
it's
> "logical" parent the correct thing to do in c++ land?

I'm actually not sure, but clang-format did this, so it *should* be legit.
It's possible that both are ok.

[no sievers, 2016-07-29 20:54:33]

The CQ bit was checked by sievers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 20:55:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 21:26:07]

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

[commit-bot: I haz the power, 2016-07-29 21:26:07]

Dry run: This issue passed the CQ dry run.

[no sievers, 2016-07-29 21:29:21]

The CQ bit was checked by sievers@chromium.org

[no sievers, 2016-07-29 21:29:21]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2179563003/#ps40001
(title: "address comments")

[commit-bot: I haz the power, 2016-07-29 21:29:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 21:34:16]

Description was changed from

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764
==========

to

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764
==========

[commit-bot: I haz the power, 2016-07-29 21:34:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-29 21:36:18]

Description was changed from

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764
==========

to

==========
Android: Remove ContentViewCore::GetLayer() from public interface

This removes some native references to ContentViewCore
from the embedder, which were mostly unnecessary indirections
and can use WebContents directly.

BUG=626764

Committed: https://crrev.com/77e87e6a068ab2297d4107bd38f81de4d24eebee
Cr-Commit-Position: refs/heads/master@{#408772}
==========

[commit-bot: I haz the power, 2016-07-29 21:36:20]

Patchset 3 (id:??) landed as
https://crrev.com/77e87e6a068ab2297d4107bd38f81de4d24eebee
Cr-Commit-Position: refs/heads/master@{#408772}