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

chrome/browser/android/compositor/tab_content_manager.cc

Index: chrome/browser/android/compositor/tab_content_manager.cc
diff --git a/chrome/browser/android/compositor/tab_content_manager.cc b/chrome/browser/android/compositor/tab_content_manager.cc
index df810095174c07a2559b9178577d6536a06a1b1a..5542f82f4e327c30dfa2374056472b0efe881393 100644
--- a/chrome/browser/android/compositor/tab_content_manager.cc
+++ b/chrome/browser/android/compositor/tab_content_manager.cc
@@ -20,7 +20,6 @@
 #include "chrome/browser/android/compositor/layer/thumbnail_layer.h"
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/browser/android/thumbnail/thumbnail.h"
-#include "content/public/browser/android/content_view_core.h"
 #include "content/public/browser/readback_types.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
@@ -45,43 +44,21 @@ namespace android {
 
 class TabContentManager::TabReadbackRequest {
  public:
-  TabReadbackRequest(jobject content_view_core,
+  TabReadbackRequest(content::WebContents* web_contents,
                      float thumbnail_scale,
                      const TabReadbackCallback& end_callback)
       : thumbnail_scale_(thumbnail_scale),
         end_callback_(end_callback),
         drop_after_readback_(false),
         weak_factory_(this) {
-    JNIEnv* env = base::android::AttachCurrentThread();
-    j_content_view_core_.Reset(env, content_view_core);
-  }
-
-  virtual ~TabReadbackRequest() {}
-
-  void Run() {
-    JNIEnv* env = base::android::AttachCurrentThread();
+    DCHECK(web_contents);
     content::ReadbackRequestCallback result_callback =
         base::Bind(&TabReadbackRequest::OnFinishGetTabThumbnailBitmap,
                    weak_factory_.GetWeakPtr());
 
-    if (j_content_view_core_.is_null()) {
-      result_callback.Run(SkBitmap(), content::READBACK_FAILED);
-      return;
-    }
-
-    content::ContentViewCore* view =
-        content::ContentViewCore::GetNativeContentViewCore(
-            env, j_content_view_core_.obj());
-
-    if (!view) {
-      result_callback.Run(SkBitmap(), content::READBACK_FAILED);
-      return;
-    }
-
-    DCHECK(view->GetWebContents());
-    content::RenderWidgetHost* rwh = view->GetWebContents()
-                                         ->GetRenderViewHost()
-                                         ->GetWidget();
+    content::RenderWidgetHost* rwh =
+        web_contents->GetRenderViewHost()->GetWidget();
+    DCHECK(rwh);
 
     SkColorType color_type = kN32_SkColorType;
     gfx::Rect src_rect = rwh->GetView()->GetViewBounds();
@@ -90,9 +67,10 @@ class TabContentManager::TabReadbackRequest {
     rwh->CopyFromBackingStore(src_rect, dst_size, result_callback, color_type);
   }
 
+  virtual ~TabReadbackRequest() {}
+
   void OnFinishGetTabThumbnailBitmap(const SkBitmap& bitmap,
                                      content::ReadbackResponse response) {
-    DCHECK(!j_content_view_core_.is_null());
     if (response != content::READBACK_SUCCESS || drop_after_readback_) {
       end_callback_.Run(0.f, SkBitmap());
       return;
@@ -106,7 +84,6 @@ class TabContentManager::TabReadbackRequest {
   void SetToDropAfterReadback() { drop_after_readback_ = true; }
 
  private:
-  base::android::ScopedJavaGlobalRef<jobject> j_content_view_core_;
   const float thumbnail_scale_;
   TabReadbackCallback end_callback_;
   bool drop_after_readback_;
@@ -234,23 +211,21 @@ jboolean TabContentManager::HasFullCachedThumbnail(
 void TabContentManager::CacheTab(JNIEnv* env,
                                  const JavaParamRef<jobject>& obj,
                                  const JavaParamRef<jobject>& tab,
-                                 const JavaParamRef<jobject>& content_view_core,
+                                 const JavaParamRef<jobject>& jweb_contents,
                                  jfloat thumbnail_scale) {
   TabAndroid* tab_android = TabAndroid::GetNativeTab(env, tab);
   DCHECK(tab_android);
   int tab_id = tab_android->GetAndroidId();
   GURL url = tab_android->GetURL();
 
-  content::ContentViewCore* view =
-      content::ContentViewCore::GetNativeContentViewCore(env,
-                                                         content_view_core);
+  content::WebContents* web_contents =
+      content::WebContents::FromJavaWebContents(jweb_contents.obj());

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

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?

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

Good point, even better. Done.

+  DCHECK(web_contents);
 
   if (thumbnail_cache_->CheckAndUpdateThumbnailMetaData(tab_id, url)) {
-    if (!view ||
-        !view->GetWebContents()->GetRenderViewHost() ||
-        !view->GetWebContents()->GetRenderViewHost()->GetWidget() ||
-        !view->GetWebContents()
-             ->GetRenderViewHost()
+    if (!web_contents->GetRenderViewHost() ||
+        !web_contents->GetRenderViewHost()->GetWidget() ||
+        !web_contents->GetRenderViewHost()
              ->GetWidget()
              ->CanCopyFromBackingStore() ||
         pending_tab_readbacks_.find(tab_id) != pending_tab_readbacks_.end() ||
@@ -262,11 +237,9 @@ void TabContentManager::CacheTab(JNIEnv* env,
     TabReadbackCallback readback_done_callback =
         base::Bind(&TabContentManager::PutThumbnailIntoCache,
                    weak_factory_.GetWeakPtr(), tab_id);
-    std::unique_ptr<TabReadbackRequest> readback_request =
-        base::WrapUnique(new TabReadbackRequest(
-            content_view_core, thumbnail_scale, readback_done_callback));
-    pending_tab_readbacks_.set(tab_id, std::move(readback_request));

[Ted C, 2016/07/25 23:46:53]

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.

[no sievers, 2016/07/29 20:54:24]

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.

-    pending_tab_readbacks_.get(tab_id)->Run();
+    pending_tab_readbacks_.set(
+        tab_id, base::WrapUnique(new TabReadbackRequest(
+                    web_contents, thumbnail_scale, readback_done_callback)));

[Ted C, 2016/07/25 23:46:53]

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:24]

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

   }
 }