[Contextual Search] Remove layers when no longer needed.

chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc

Index: chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc
diff --git a/chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc b/chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc
index 26d7e8a22b82cfbc55b6ee71f2a5485ea7e1cf6e..a6e3995fe57c3dd05837f292210dc02836bbeae4 100644
--- a/chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc
+++ b/chrome/browser/android/compositor/scene_layer/static_tab_scene_layer.cc
@@ -127,13 +127,20 @@ void StaticTabSceneLayer::SetContentSceneLayer(
     const JavaParamRef<jobject>& jobj,
     const JavaParamRef<jobject>& jcontent_scene_layer) {
   SceneLayer* content_scene_layer = FromJavaObject(env, jcontent_scene_layer);
+  // If |content_scene_layer| is null, or different from the previously added

[David Trainor- moved to gerrit, 2015/12/02 16:34:13]

Would this be easier if we just pulled out the actually new ContentSceneLayer
here and used that?

scoped_refptr<Layer> layer = content_scene_layer ? content_scene_layer->layer()
: nullptr;

if (content_scene_layer_ && content_scene_layer_ != layer) {
  // Remove meh.
}

if (layer) {
  content_scene_layer_ = layer;
  // Add meh.
}

Also, do we want to do this each time we call SetContentSceneLayer?  What if
it's already set?  Even if the layers are equal it looks like we still call
AddChild below, which will remove it and readd it IIUC.

[pedro (no code reviews), 2015/12/02 23:56:14]

As we've discussed offline, the problem is that the content layer is being
re-added to hierarchy on every frame, so if we don't do the same, the content
scene layer will be occluded by the content layer.

For now, since we need to merge this in M48, I'll keep re-adding on every frame.
I put a TODO to revisit this later. We could force the content layer to be added
always at the first position (and only add it once), and then we could add the
content scene layer only once as well, and it will be guaranteed that it will be
placed after the content layer. But I'd rather do that later so we have more
time to properly test the new approach.

+  // layer (which means a new scene layer will replace the old one), then
+  // the layer should be removed from the hierarchy.
+  if (content_scene_layer_ && (!content_scene_layer || (content_scene_layer &&
+      content_scene_layer->layer().get() != content_scene_layer_.get()))) {
+    content_scene_layer_->RemoveFromParent();
+    content_scene_layer_ = nullptr;
+  }
+
   if (content_scene_layer && content_scene_layer->layer()) {
     content_scene_layer_ = content_scene_layer->layer();
-    if (content_scene_layer_.get())
+    if (content_scene_layer_.get()) {
       layer_->AddChild(content_scene_layer_);
-  } else if (content_scene_layer_) {
-    content_scene_layer_->RemoveFromParent();
-    content_scene_layer_ = nullptr;
+    }
   }
 }