[Contextual Search] Fix Promo behavior.

chrome/browser/android/compositor/layer/contextual_search_layer.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/compositor/layer/contextual_search_layer.h"
 
 #include "cc/layers/layer.h"
 #include "cc/layers/nine_patch_layer.h"
 #include "cc/layers/solid_color_layer.h"
 #include "cc/layers/ui_resource_layer.h"
@@ skipping 123 matching lines @@
       search_icon_resource->size.height() / 2;
   search_icon_->SetUIResourceId(search_icon_resource->ui_resource->id());
   search_icon_->SetBounds(search_icon_resource->size);
   search_icon_->SetPosition(
       gfx::PointF(search_icon_padding_left, search_icon_padding_top));
   search_icon_->SetOpacity(search_icon_opacity);
 
   // ---------------------------------------------------------------------------
   // Search Promo
   // ---------------------------------------------------------------------------
+  bool is_search_promo_resource_ready = false;
   if (search_promo_visible) {
     // Grab the Search Opt Out Promo resource.
     ui::ResourceManager::Resource* search_promo_resource =
         resource_manager_->GetResource(ui::ANDROID_RESOURCE_TYPE_DYNAMIC,
                                        search_promo_resource_id);
 
     if (search_promo_resource) {
+      is_search_promo_resource_ready = true;
+
       // Search Promo Container
       if (search_promo_container_->parent() != layer_)
         layer_->AddChild(search_promo_container_);
 
       int search_promo_content_height = search_promo_resource->size.height();
       gfx::Size search_promo_size(search_panel_width, search_promo_height);
       search_promo_container_->SetBounds(search_promo_size);
       search_promo_container_->SetPosition(gfx::PointF(0.f, search_bar_height));
       search_promo_container_->SetMasksToBounds(true);
 
@@ skipping 27 matching lines @@
     scoped_refptr<cc::Layer> content_view_layer = content_view_core->GetLayer();
     if (content_view_layer->parent() != content_view_container_)
       content_view_container_->AddChild(content_view_layer);
   } else {
     content_view_container_->RemoveAllChildren();
   }
 
   // ---------------------------------------------------------------------------
   // Search Bar Shadow
   // ---------------------------------------------------------------------------
-  if (search_bar_shadow_visible) {
+  // NOTE(pedrosimonetti): Since the Promo resource is a dynamic one (it will
+  // take a snapshot of the Promo View) it will always be available before

[Donn Denman, 2015/04/14 23:50:04]

This is confusing to me.  If the promo will always be available before the
shadow, why do we need to check if it's ready?

[pedro (no code reviews), 2015/04/16 19:05:29]

It was supposed to read: "it will NOT always be available before". There was
still a potential bug in the code so I reimplemented a fix for this problem and
updated the comment.

See my patch comment for more details.

+  // the Shadow resource. Therefore, we need to confirm that the Promo resource
+  // is ready before adding the Shadow layer, otherwise the Promo layer will
+  // be placed over the Shadow layer, making it not visible.

[David Trainor- moved to gerrit, 2015/04/15 07:36:15]

You can insert children at specific points in the child list.  That might be
simpler.

[pedro (no code reviews), 2015/04/16 19:05:29]

Good idea. I'm using InsertChild() now.

On the other hand, I think the existing API is not flexible. I wanted to place
the Promo layer immediately before the Search Bar shadow one, but there are no
InsertBefore() or InsertAfter() methods. I could do that with InsertChild() but
I would have to know the position of the Shadow layer, which is not possible to
do since the IndexOfChild() method is private.

See my patch comment for more details.

[David Trainor- moved to gerrit, 2015/04/16 20:51:45]

Yeah, I wonder if we could make that public.  I don't see any reason not to :). 
But I'm fine with this way too, as long as putting the promo at 0 works :).

+  if (is_search_promo_resource_ready && search_bar_shadow_visible) {
     ui::ResourceManager::Resource* search_bar_shadow_resource =
         resource_manager_->GetResource(ui::ANDROID_RESOURCE_TYPE_STATIC,
                                        search_bar_shadow_resource_id);
 
     if (search_bar_shadow_resource) {
       if (search_bar_shadow_->parent() != layer_)
         layer_->AddChild(search_bar_shadow_);
+
+      int shadow_height = search_bar_shadow_resource->size.height();
+      gfx::Size shadow_size(search_panel_width, shadow_height);
+
+      search_bar_shadow_->SetUIResourceId(
+          search_bar_shadow_resource->ui_resource->id());
+      search_bar_shadow_->SetBounds(shadow_size);
+      search_bar_shadow_->SetPosition(gfx::PointF(0.f, search_bar_height));
+      search_bar_shadow_->SetOpacity(search_bar_shadow_opacity);
     }
-
-    int shadow_height = search_bar_shadow_resource->size.height();
-    gfx::Size shadow_size(search_panel_width, shadow_height);
-
-    search_bar_shadow_->SetUIResourceId(
-        search_bar_shadow_resource->ui_resource->id());
-    search_bar_shadow_->SetBounds(shadow_size);
-    search_bar_shadow_->SetPosition(gfx::PointF(0.f, search_bar_height));
-    search_bar_shadow_->SetOpacity(search_bar_shadow_opacity);
   } else {
     if (search_bar_shadow_.get() && search_bar_shadow_->parent())
       search_bar_shadow_->RemoveFromParent();
   }
 
   // ---------------------------------------------------------------------------
   // Search Panel.
   // ---------------------------------------------------------------------------
   layer_->SetPosition(gfx::PointF(0.f, search_panel_y));
 
@@ skipping 127 matching lines @@
 
 ContextualSearchLayer::~ContextualSearchLayer() {
 }
 
 scoped_refptr<cc::Layer> ContextualSearchLayer::layer() {
   return layer_;
 }
 
 }  //  namespace android
 }  //  namespace chrome