[Contextual Search] Fetch and display thumbnails returned in resolution response

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

Index: chrome/browser/android/compositor/layer/contextual_search_layer.cc
diff --git a/chrome/browser/android/compositor/layer/contextual_search_layer.cc b/chrome/browser/android/compositor/layer/contextual_search_layer.cc
index eee52f3ff39cd9243bc66ad3208676ca865e2257..20e8308ef883a5d982540d385449d25421dc4796 100644
--- a/chrome/browser/android/compositor/layer/contextual_search_layer.cc
+++ b/chrome/browser/android/compositor/layer/contextual_search_layer.cc
@@ -39,6 +39,22 @@ scoped_refptr<ContextualSearchLayer> ContextualSearchLayer::Create(
 }
 
 scoped_refptr<cc::Layer> ContextualSearchLayer::GetIconLayer() {

[Donn Denman, 2016/09/08 18:13:39]

nit: I think this section could benefit from some refactoring.  There's some
code duplication removing the sprite layer, there are returns in multiple places
(which seems error-prone), and it's hard to reason about what's happening.  

Is there a way to restructure this to say:
  layer present = sprite or thumbnail;
  layer desired = sprite or thumbnail;
  if (present) remove;
  add desired;
  return desired;

Just a suggestion, adding a TODO is fine too!

[Theresa, 2016/09/08 20:18:22]

I got rid off the duplicate code removing the sprite layer and multiple returns.
Does it look better?

[Donn Denman, 2016/09/08 20:41:51]

Yes, that's most of what I was looking for, and if we do more then readability
may suffer.  Thanks!

+  if (thumbnail_visible_) {
+    if (search_provider_icon_sprite_->layer().get() &&
+        search_provider_icon_sprite_->layer()->parent()) {
+      search_provider_icon_sprite_->layer()->RemoveFromParent();
+    }
+
+    if (thumbnail_layer_->parent() != layer_)
+          layer_->AddChild(thumbnail_layer_);
+
+    return thumbnail_layer_;
+  }
+
+  if (thumbnail_layer_->parent()) {
+    thumbnail_layer_->RemoveFromParent();
+  }
+
   // Search Provider Icon Sprite (Animated)
   if (search_provider_icon_sprite_visible_) {
     if (search_provider_icon_sprite_->layer()->parent() != layer_) {
@@ -102,6 +118,8 @@ void ContextualSearchLayer::SetProperties(
     float search_bar_shadow_opacity,
     bool search_provider_icon_sprite_visible,
     float search_provider_icon_sprite_completion_percentage,
+    bool thumbnail_visible,
+    int thumbnail_size,
     float arrow_icon_opacity,
     float arrow_icon_rotation,
     float close_icon_opacity,
@@ -116,6 +134,9 @@ void ContextualSearchLayer::SetProperties(
   search_provider_icon_sprite_completion_percentage_ =
       search_provider_icon_sprite_completion_percentage;
 
+  thumbnail_visible_ = thumbnail_visible;
+  thumbnail_size_ = thumbnail_size;
+
   // Grabs the dynamic Search Context resource.
   ui::ResourceManager::Resource* search_context_resource =
       resource_manager_->GetResource(ui::ANDROID_RESOURCE_TYPE_DYNAMIC,
@@ -455,11 +476,87 @@ void ContextualSearchLayer::SetProperties(
   }
 }
 
+void ContextualSearchLayer::SetThumbnail(const SkBitmap* thumbnail) {

[Donn Denman, 2016/09/08 18:21:44]

Can this scale-and-crop logic be shared with similar code in the download code?

[Theresa, 2016/09/08 20:18:22]

Unfortunately not. Downloads displays its thumbnails using Java views, not CC
layers. The few lines that determine the min dimension and scaled width/height
are the same, but I don't think it make sense to pull out ~4 lines of code into
a helper method.

[Donn Denman, 2016/09/08 20:41:52]

Agreed!

+  // Determine the scaled thumbnail width and height. If both the height and

[Donn Denman, 2016/09/08 18:13:39]

Don't we need scaling if *either* height or width is beyond thumbnail_size?

[Theresa, 2016/09/08 20:18:23]

No, only if they're both greater than thumbnail_size. This scaling is to make
the minimum side (height or width) equal to thumbnail_size. If one of them is
already less than or equal to thumbnail size, we don't scale, only crop.

Example: We get a long horizontal image, the height is < thumbnail size. We
won't scale, but we will crop the edges off. This will result in some white
space on the top and bottom of the image. We *should* get back images that are
large enough that this doesn't happen, but I want the code to handle it
correctly anyway.

[Donn Denman, 2016/09/08 20:41:51]

Acknowledged.  Sorry you wasted some time cuz I didn't remove this comment after
we agreed on scale&crop.

+  // width of |thumbnail| are larger than |thumbnail_size_|, the thumbnail will
+  // be scaled down by a call to Layer::SetBounds() below.
+  int min_dimension = std::min(thumbnail->width(), thumbnail->height());
+  int scaled_thumbnail_width = thumbnail->width();
+  int scaled_thumbnail_height = thumbnail->height();
+  if (min_dimension > thumbnail_size_) {
+    scaled_thumbnail_width =

[Donn Denman, 2016/09/08 18:13:39]

nit: just use *= here, and below?  Now that I think about it, since this is
integer math you may get a different outcome due to different ordering of the
calculations.  So I think good choices include computing a float for
thumbnail_size_ / min_dimension, OR just use scaled_thumbnail_width instead of
thumbnail->width() here. 

[Theresa, 2016/09/08 20:18:23]

Done.

+        thumbnail->width() * thumbnail_size_ / min_dimension;
+    scaled_thumbnail_height =
+        thumbnail->height() * thumbnail_size_ / min_dimension;
+  }
+
+  // Determine the UV transform coordinates. This will crop the thumbnail.
+  // (0, 0) is the default top left corner. (1, 1) is the default bottom
+  // right corner.
+  float top_left_x = 0;
+  float top_left_y = 0;
+  float bottom_right_x = 1;
+  float bottom_right_y = 1;
+
+  if (scaled_thumbnail_width > thumbnail_size_) {
+    // Crop an even amount on the left and right sides of the thumbnail.
+    float top_left_x_px = (scaled_thumbnail_width - thumbnail_size_) / 2.f;
+    float bottom_right_x_px = top_left_x_px + thumbnail_size_;
+
+    top_left_x = top_left_x_px / scaled_thumbnail_width;
+    bottom_right_x = bottom_right_x_px / scaled_thumbnail_width;
+  } else if (scaled_thumbnail_height > thumbnail_size_) {
+    // Crop an even amount on the top and bottom of the thumbnail.
+    float top_left_y_px = (scaled_thumbnail_height - thumbnail_size_) / 2.f;
+    float bottom_right_y_px = top_left_y_px + thumbnail_size_;
+
+    top_left_y = top_left_y_px / scaled_thumbnail_height;
+    bottom_right_y = bottom_right_y_px / scaled_thumbnail_height;
+  }
+
+  // If the original |thumbnail| height or width is smaller than
+  // |thumbnail_size_| determine the x, y offset needed to center
+  // the thumbnail.
+  float x_pos = 0;
+  float y_pos = 0;
+
+  if (scaled_thumbnail_width < thumbnail_size_) {
+    x_pos = (thumbnail_size_ - scaled_thumbnail_width) / 2.f;
+  }
+
+  if (scaled_thumbnail_height < thumbnail_size_) {
+    y_pos = (thumbnail_size_ - scaled_thumbnail_height) / 2.f;
+  }
+
+  // Determine the layer bounds. This will down scale the thumbnail if
+  // necessary and ensure it is displayed at |thumbnail_size_|. If
+  // either the original |thumbnail| height or width is smaller than
+  // |thumbnail_size_|, the thumbnail will not be scaled.
+  int layer_width = std::min(thumbnail_size_, scaled_thumbnail_width);
+  int layer_height = std::min(thumbnail_size_, scaled_thumbnail_height);
+
+  // UIResourceLayer requires an immutable copy of the input |thumbnail|.
+  SkBitmap thumbnail_copy;
+  if (thumbnail->isImmutable()) {
+    thumbnail_copy = *thumbnail;
+  } else {
+    thumbnail->copyTo(&thumbnail_copy);
+    thumbnail_copy.setImmutable();
+  }
+
+  thumbnail_layer_->SetBitmap(thumbnail_copy);
+  thumbnail_layer_->SetBounds(gfx::Size(layer_width, layer_height));
+  thumbnail_layer_->SetPosition(gfx::PointF(x_pos, y_pos));
+  thumbnail_layer_->SetUV(gfx::PointF(top_left_x, top_left_y),
+                          gfx::PointF(bottom_right_x, bottom_right_y));
+}
+
 ContextualSearchLayer::ContextualSearchLayer(
     ui::ResourceManager* resource_manager)
     : OverlayPanelLayer(resource_manager),
       search_context_(cc::UIResourceLayer::Create()),
       search_provider_icon_sprite_(CrushedSpriteLayer::Create()),
+      thumbnail_layer_(cc::UIResourceLayer::Create()),
       arrow_icon_(cc::UIResourceLayer::Create()),
       search_promo_(cc::UIResourceLayer::Create()),
       search_promo_container_(cc::SolidColorLayer::Create()),
@@ -502,6 +599,9 @@ ContextualSearchLayer::ContextualSearchLayer(
   // Progress Bar
   progress_bar_->SetIsDrawable(true);
   progress_bar_->SetFillCenter(true);
+
+  // Thumbnail
+  thumbnail_layer_->SetIsDrawable(true);
 }
 
 ContextualSearchLayer::~ContextualSearchLayer() {