Add support for ContentViewCore helpers on Tab

chrome/browser/android/tab_android.cc

Index: chrome/browser/android/tab_android.cc
diff --git a/chrome/browser/android/tab_android.cc b/chrome/browser/android/tab_android.cc
index b36830d8aa290eda14c8d6d7ceacc38b33af271a..dbd21bea2b5ac9c8960bd33be4179ad30a2f67c8 100644
--- a/chrome/browser/android/tab_android.cc
+++ b/chrome/browser/android/tab_android.cc
@@ -9,7 +9,9 @@
 #include "base/android/jni_string.h"
 #include "base/metrics/histogram.h"
 #include "base/trace_event/trace_event.h"
+#include "cc/layers/layer.h"
 #include "chrome/browser/android/chrome_web_contents_delegate_android.h"
+#include "chrome/browser/android/compositor/tab_content_manager.h"
 #include "chrome/browser/android/uma_utils.h"
 #include "chrome/browser/browser_about_handler.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ -110,11 +112,14 @@ void TabAndroid::AttachTabHelpers(content::WebContents* web_contents) {
 
 TabAndroid::TabAndroid(JNIEnv* env, jobject obj)
     : weak_java_tab_(env, obj),
+      content_layer_(cc::Layer::Create()),
+      tab_content_manager_(NULL),
       synced_tab_delegate_(new browser_sync::SyncedTabDelegateAndroid(this)) {
   Java_Tab_setNativePtr(env, obj, reinterpret_cast<intptr_t>(this));
 }
 
 TabAndroid::~TabAndroid() {
+  GetContentLayer()->RemoveAllChildren();

[Ted C, 2015/02/06 01:52:39]

will this not happen when it gets destroyed?

[David Trainor- moved to gerrit, 2015/02/06 18:37:04]

It's a scoped_refptr, so if it's attached to another object it could stay around
(ie: if it's stored as a parent of another tab or a child of another tab?).

   JNIEnv* env = base::android::AttachCurrentThread();
   Java_Tab_clearNativePtr(env, weak_java_tab_.get(env).obj());
 }
@@ -124,6 +129,10 @@ base::android::ScopedJavaLocalRef<jobject> TabAndroid::GetJavaObject() {
   return weak_java_tab_.get(env);
 }
 
+scoped_refptr<cc::Layer> TabAndroid::GetContentLayer() const {
+  return content_layer_;
+}
+
 int TabAndroid::GetAndroidId() const {
   JNIEnv* env = base::android::AttachCurrentThread();
   return Java_Tab_getId(env, weak_java_tab_.get(env).obj());
@@ -439,6 +448,8 @@ void TabAndroid::InitWebContents(JNIEnv* env,
       InstantServiceFactory::GetForProfile(GetProfile());
   if (instant_service)
     instant_service->AddObserver(this);
+
+  content_layer_->InsertChild(content_view_core->GetLayer(), 0);
 }
 
 void TabAndroid::DestroyWebContents(JNIEnv* env,
@@ -446,6 +457,10 @@ void TabAndroid::DestroyWebContents(JNIEnv* env,
                                     jboolean delete_native) {
   DCHECK(web_contents());
 
+  content::ContentViewCore* content_view_core = GetContentViewCore();
+  if (content_view_core)
+    content_view_core->GetLayer()->RemoveFromParent();

[Ted C, 2015/02/06 01:52:39]

is this not what the change in CVCImpl.cc is doing?  Just trying to figure out
what is different between the two.

[David Trainor- moved to gerrit, 2015/02/06 18:37:04]

This doesn't explicitly call delete content_view_core, so I wanted it to clean
it's own state up gracefully (I believe the logic is the same as pre-patch
here).

content_view_core_impl is a little different, but more safe when used with
overlays I think.  I added the method to content_view_core_impl's destructor
because for things like overlays destroy()'ing the cvc shouldn't leak the cc
layer by leaving it around as a child node of something else.

+
   notification_registrar_.Remove(
       this,
       chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
@@ -742,6 +757,60 @@ void TabAndroid::SetInterceptNavigationDelegate(JNIEnv* env, jobject obj,
       make_scoped_ptr(new ChromeInterceptNavigationDelegate(env, delegate)));
 }
 
+void TabAndroid::AttachToTabContentManager(JNIEnv* env,
+                                           jobject obj,
+                                           jobject jtab_content_manager) {
+  chrome::android::TabContentManager* tab_content_manager =
+      chrome::android::TabContentManager::FromJavaObject(jtab_content_manager);
+  if (tab_content_manager == tab_content_manager_)
+    return;
+
+  if (tab_content_manager_)
+    tab_content_manager_->DetachLiveLayer(GetAndroidId(), GetContentLayer());
+  tab_content_manager_ = tab_content_manager;
+  if (tab_content_manager_)
+    tab_content_manager_->AttachLiveLayer(GetAndroidId(), GetContentLayer());
+}
+
+void TabAndroid::AttachOverlayContentViewCore(JNIEnv* env,
+                                              jobject obj,
+                                              jobject jcontent_view_core,
+                                              jboolean visible) {
+  content::ContentViewCore* content_view_core =
+      content::ContentViewCore::GetNativeContentViewCore(env,
+                                                         jcontent_view_core);
+  if (!content_view_core)

[Ted C, 2015/02/06 01:52:39]

should we allow this? same below

[David Trainor- moved to gerrit, 2015/02/06 18:37:04]

Should we not allow null to be even passed in here?  I can DCHECK, we should
also catch this in Java :(.

+    return;
+
+  content_view_core->GetLayer()->SetHideLayerAndSubtree(!visible);
+  content_layer_->AddChild(content_view_core->GetLayer());
+}
+
+void TabAndroid::DetachOverlayContentViewCore(JNIEnv* env,
+                                              jobject obj,
+                                              jobject jcontent_view_core) {
+  content::ContentViewCore* content_view_core =
+      content::ContentViewCore::GetNativeContentViewCore(env,
+                                                         jcontent_view_core);
+  if (!content_view_core)
+    return;
+
+  content_view_core->GetLayer()->RemoveFromParent();
+}
+
+void TabAndroid::SetOverlayContentViewCoreVisibility(JNIEnv* env,
+                                                     jobject obj,
+                                                     jobject jcontent_view_core,
+                                                     jboolean visible) {
+  content::ContentViewCore* content_view_core =

[Ted C, 2015/02/06 01:52:39]

seems odd to have this method on tab when it doesn't actually need tab at all
and could be static.

[David Trainor- moved to gerrit, 2015/02/06 18:37:04]

Yeah I thought about that too.  I can move it to ContentViewUtils or something. 
The main issue is that I don't want the helpers to have to spin up a native
object/JNI stuff just to set the visibility of their ContentViewCore.  Actually
maybe this should be a method on ContentViewCore.java itself...

+      content::ContentViewCore::GetNativeContentViewCore(env,
+                                                         jcontent_view_core);
+  if (!content_view_core)
+    return;
+
+  content_view_core->GetLayer()->SetHideLayerAndSubtree(!visible);
+}
+
 static void Init(JNIEnv* env, jobject obj) {
   TRACE_EVENT0("native", "TabAndroid::Init");
   // This will automatically bind to the Java object and pass ownership there.