InstantExtended(gtk): Hide top match if told to so.

chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc

 // Copyright (c) 2012 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/ui/gtk/omnibox/omnibox_popup_view_gtk.h"
 
 #include <gtk/gtk.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 80 matching lines @@
 // UTF-8 Left-to-right embedding.
 const char* kLRE = "\xe2\x80\xaa";
 
 // Return a Rect covering the whole area of |window|.
 gfx::Rect GetWindowRect(GdkWindow* window) {
   gint width = gdk_window_get_width(window);
   gint height = gdk_window_get_height(window);
   return gfx::Rect(width, height);
 }
 
-// Return a Rect for the space for a result line.  This excludes the border,
-// but includes the padding.  This is the area that is colored for a selection.
-gfx::Rect GetRectForLine(size_t line, int width) {
-  return gfx::Rect(kBorderThickness,
-                   (line * kHeightPerResult) + kBorderThickness,
-                   width - (kBorderThickness * 2),
-                   kHeightPerResult);
-}
-
 // TODO(deanm): Find some better home for this, and make it more efficient.
 size_t GetUTF8Offset(const string16& text, size_t text_offset) {
   return UTF16ToUTF8(text.substr(0, text_offset)).length();
 }
 
 // Generates the normal URL color, a green color used in unhighlighted URL
 // text. It is a mix of |kURLTextColor| and the current text color.  Unlike the
 // selected text color, it is more important to match the qualities of the
 // foreground typeface color instead of taking the background into account.
 GdkColor NormalURLColor(GdkColor foreground) {
@@ skipping 43 matching lines @@
   // The luminance should match the luminance of the foreground text.  Again,
   // we clamp so as to have at some amount of color (green) in the text.
   double opposite_l = fg_hsl.l;
   double l = std::max(0.1, std::min(0.9, opposite_l));
 
   color_utils::HSL output = { hue_hsl.h, s, l };
   return gfx::SkColorToGdkColor(color_utils::HSLToSkColor(output, 255));
 }
 }  // namespace
 
-void OmniboxPopupViewGtk::SetupLayoutForMatch(
-    PangoLayout* layout,
-    const string16& text,
-    const AutocompleteMatch::ACMatchClassifications& classifications,
-    const GdkColor* base_color,
-    const GdkColor* dim_color,
-    const GdkColor* url_color,
-    const std::string& prefix_text) {
-  // In RTL, mark text with left-to-right embedding mark if there is no strong
-  // RTL characters inside it, so the ending punctuation displays correctly
-  // and the eliding ellipsis displays correctly. We only mark the text with
-  // LRE. Wrapping it with LRE and PDF by calling AdjustStringForLocaleDirection
-  // or WrapStringWithLTRFormatting will render the elllipsis at the left of the
-  // elided pure LTR text.
-  bool marked_with_lre = false;
-  string16 localized_text = text;
-  // Pango is really easy to overflow and send into a computational death
-  // spiral that can corrupt the screen. Assume that we'll never have more than
-  // 2000 characters, which should be a safe assumption until we all get robot
-  // eyes. http://crbug.com/66576
-  if (localized_text.length() > 2000)
-    localized_text = localized_text.substr(0, 2000);
-  bool is_rtl = base::i18n::IsRTL();
-  if (is_rtl && !base::i18n::StringContainsStrongRTLChars(localized_text)) {
-    localized_text.insert(0, 1, base::i18n::kLeftToRightEmbeddingMark);
-    marked_with_lre = true;
-  }
-
-  // We can have a prefix, or insert additional characters while processing the
-  // classifications.  We need to take this in to account when we translate the
-  // UTF-16 offsets in the classification into text_utf8 byte offsets.
-  size_t additional_offset = prefix_text.length();  // Length in utf-8 bytes.
-  std::string text_utf8 = prefix_text + UTF16ToUTF8(localized_text);
-
-  PangoAttrList* attrs = pango_attr_list_new();
-
-  // TODO(deanm): This is a hack, just to handle coloring prefix_text.
-  // Hopefully I can clean up the match situation a bit and this will
-  // come out cleaner.  For now, apply the base color to the whole text
-  // so that our prefix will have the base color applied.
-  PangoAttribute* base_fg_attr = pango_attr_foreground_new(
-      base_color->red, base_color->green, base_color->blue);
-  pango_attr_list_insert(attrs, base_fg_attr);  // Ownership taken.
-
-  // Walk through the classifications, they are linear, in order, and should
-  // cover the entire text.  We create a bunch of overlapping attributes,
-  // extending from the offset to the end of the string.  The ones created
-  // later will override the previous ones, meaning we will still setup each
-  // portion correctly, we just don't need to compute the end offset.
-  for (ACMatchClassifications::const_iterator i = classifications.begin();
-       i != classifications.end(); ++i) {
-    size_t offset = GetUTF8Offset(localized_text, i->offset) +
-                    additional_offset;
-
-    // TODO(deanm): All the colors should probably blend based on whether this
-    // result is selected or not.  This would include the green URLs.  Right
-    // now the caller is left to blend only the base color.  Do we need to
-    // handle things like DIM urls?  Turns out DIM means something different
-    // than you'd think, all of the description text is not DIM, it is a
-    // special case that is not very common, but we should figure out and
-    // support it.
-    const GdkColor* color = base_color;
-    if (i->style & ACMatchClassification::URL) {
-      color = url_color;
-      // Insert a left to right embedding to make sure that URLs are shown LTR.
-      if (is_rtl && !marked_with_lre) {
-        std::string lre(kLRE);
-        text_utf8.insert(offset, lre);
-        additional_offset += lre.length();
-      }
-    }
-
-    if (i->style & ACMatchClassification::DIM)
-      color = dim_color;
-
-    PangoAttribute* fg_attr = pango_attr_foreground_new(
-        color->red, color->green, color->blue);
-    fg_attr->start_index = offset;
-    pango_attr_list_insert(attrs, fg_attr);  // Ownership taken.
-
-    // Matched portions are bold, otherwise use the normal weight.
-    PangoWeight weight = (i->style & ACMatchClassification::MATCH) ?
-        PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
-    PangoAttribute* weight_attr = pango_attr_weight_new(weight);
-    weight_attr->start_index = offset;
-    pango_attr_list_insert(attrs, weight_attr);  // Ownership taken.
-  }
-
-  pango_layout_set_text(layout, text_utf8.data(), text_utf8.length());
-  pango_layout_set_attributes(layout, attrs);  // Ref taken.
-  pango_attr_list_unref(attrs);
-}
-
 OmniboxPopupViewGtk::OmniboxPopupViewGtk(const gfx::Font& font,
                                          OmniboxView* omnibox_view,
                                          OmniboxEditModel* edit_model,
                                          GtkWidget* location_bar)
     : signal_registrar_(new ui::GtkSignalRegistrar),
-      model_(new OmniboxPopupModel(this, edit_model)),
       omnibox_view_(omnibox_view),
       location_bar_(location_bar),
       window_(gtk_window_new(GTK_WINDOW_POPUP)),
       layout_(NULL),
-      theme_service_(GtkThemeService::GetFrom(edit_model->profile())),
+      theme_service_(NULL),
       font_(font.DeriveFont(kEditFontAdjust)),
       ignore_mouse_drag_(false),
       opened_(false) {
+  // edit_model may be NULL in unit tests.
+  if (edit_model) {
+    model_.reset(new OmniboxPopupModel(this, edit_model));
+    theme_service_ = GtkThemeService::GetFrom(edit_model->profile());
+  }
+}
+
+OmniboxPopupViewGtk::~OmniboxPopupViewGtk() {
+  // Stop listening to our signals before we destroy the model. I suspect that
+  // we can race window destruction, otherwise.
+  signal_registrar_.reset();

[Evan Stade, 2013/08/15 18:57:29]

This should not be necessary. I realize you're just moving this code, but it
still doesn't make sense. I've just created a patch to fix this:

https://codereview.chromium.org/23257002/

+
+  // Explicitly destroy our model here, before we destroy our GTK widgets.
+  // This is because the model destructor can call back into us, and we need
+  // to make sure everything is still valid when it does.
+  model_.reset();
+  // layout_ may be NULL in unit tests.
+  if (layout_) {
+    g_object_unref(layout_);
+    gtk_widget_destroy(window_);
+  }
+}
+
+void OmniboxPopupViewGtk::Init() {
   gtk_widget_set_can_focus(window_, FALSE);
   // Don't allow the window to be resized.  This also forces the window to
   // shrink down to the size of its child contents.
   gtk_window_set_resizable(GTK_WINDOW(window_), FALSE);
   gtk_widget_set_app_paintable(window_, TRUE);
   // Have GTK double buffer around the expose signal.
   gtk_widget_set_double_buffered(window_, TRUE);
 
   // Cache the layout so we don't have to create it for every expose.  If we
   // were a real widget we should handle changing directions, but we're not
@@ skipping 28 matching lines @@
   // corners. This worked on the standard xorg server (both locally and
   // remotely), but broke over NX. My current hypothesis is that it can't
   // handle shaping top-level windows during an expose event, but I'm not sure
   // how else to get accurate shaping information.
   //
   // r25080 (the original patch that added rounded corners here) should
   // eventually be cherry picked once I know what's going
   // on. http://crbug.com/22015.
 }
 
-OmniboxPopupViewGtk::~OmniboxPopupViewGtk() {
-  // Stop listening to our signals before we destroy the model. I suspect that
-  // we can race window destruction, otherwise.
-  signal_registrar_.reset();
-
-  // Explicitly destroy our model here, before we destroy our GTK widgets.
-  // This is because the model destructor can call back into us, and we need
-  // to make sure everything is still valid when it does.
-  model_.reset();
-  g_object_unref(layout_);
-  gtk_widget_destroy(window_);
-}
-
 bool OmniboxPopupViewGtk::IsOpen() const {
   return opened_;
 }
 
 void OmniboxPopupViewGtk::InvalidateLine(size_t line) {
   // TODO(deanm): Is it possible to use some constant for the width, instead
   // of having to query the width of the window?
   GdkWindow* gdk_window = gtk_widget_get_window(GTK_WIDGET(window_));
   GdkRectangle line_rect = GetRectForLine(
       line, GetWindowRect(gdk_window).width()).ToGdkRectangle();
   gdk_window_invalidate_rect(gdk_window, &line_rect, FALSE);
 }
 
 void OmniboxPopupViewGtk::UpdatePopupAppearance() {
-  const AutocompleteResult& result = model_->result();
-  if (result.empty()) {
+  const AutocompleteResult& result = GetResult();
+  const size_t hidden_matches = GetHiddenMatchCount();
+  if (result.size() <= hidden_matches) {
     Hide();
     return;
   }
 
-  Show(result.size());
+  Show(result.size() - hidden_matches);
   gtk_widget_queue_draw(window_);
 }
 
 gfx::Rect OmniboxPopupViewGtk::GetTargetBounds() {
   if (!gtk_widget_get_realized(window_))
     return gfx::Rect();
 
   gfx::Rect retval = ui::GetWidgetScreenBounds(window_);
 
   // The widget bounds don't update synchronously so may be out of sync with
@@ skipping 54 matching lines @@
       gtk_util::AverageColors(content_text_color_,
                               background_color_);
   selected_content_dim_text_color_ =
       gtk_util::AverageColors(selected_content_text_color_,
                               selected_background_color_);
 
   // Set the background color, so we don't need to paint it manually.
   gtk_widget_modify_bg(window_, GTK_STATE_NORMAL, &background_color_);
 }
 
+size_t OmniboxPopupViewGtk::LineFromY(int y) const {
+  size_t line = std::max(y - kBorderThickness, 0) / kHeightPerResult;
+  return std::min(line + GetHiddenMatchCount(), GetResult().size() - 1);
+}
+
+gfx::Rect OmniboxPopupViewGtk::GetRectForLine(size_t line, int width) const {
+  size_t visible_line = line - GetHiddenMatchCount();
+  return gfx::Rect(kBorderThickness,
+                   (visible_line * kHeightPerResult) + kBorderThickness,
+                   width - (kBorderThickness * 2),
+                   kHeightPerResult);
+}
+
+size_t OmniboxPopupViewGtk::GetHiddenMatchCount() const {
+  return GetResult().ShouldHideTopMatch() ? 1 : 0;
+}
+
+const AutocompleteResult& OmniboxPopupViewGtk::GetResult() const {
+  return model_->result();
+}
+
+// static
+void OmniboxPopupViewGtk::SetupLayoutForMatch(
+    PangoLayout* layout,
+    const string16& text,
+    const AutocompleteMatch::ACMatchClassifications& classifications,
+    const GdkColor* base_color,
+    const GdkColor* dim_color,
+    const GdkColor* url_color,
+    const std::string& prefix_text) {
+  // In RTL, mark text with left-to-right embedding mark if there is no strong
+  // RTL characters inside it, so the ending punctuation displays correctly
+  // and the eliding ellipsis displays correctly. We only mark the text with
+  // LRE. Wrapping it with LRE and PDF by calling AdjustStringForLocaleDirection
+  // or WrapStringWithLTRFormatting will render the elllipsis at the left of the
+  // elided pure LTR text.
+  bool marked_with_lre = false;
+  string16 localized_text = text;
+  // Pango is really easy to overflow and send into a computational death
+  // spiral that can corrupt the screen. Assume that we'll never have more than
+  // 2000 characters, which should be a safe assumption until we all get robot
+  // eyes. http://crbug.com/66576
+  if (localized_text.length() > 2000)
+    localized_text = localized_text.substr(0, 2000);
+  bool is_rtl = base::i18n::IsRTL();
+  if (is_rtl && !base::i18n::StringContainsStrongRTLChars(localized_text)) {
+    localized_text.insert(0, 1, base::i18n::kLeftToRightEmbeddingMark);
+    marked_with_lre = true;
+  }
+
+  // We can have a prefix, or insert additional characters while processing the
+  // classifications.  We need to take this in to account when we translate the
+  // UTF-16 offsets in the classification into text_utf8 byte offsets.
+  size_t additional_offset = prefix_text.length();  // Length in utf-8 bytes.
+  std::string text_utf8 = prefix_text + UTF16ToUTF8(localized_text);
+
+  PangoAttrList* attrs = pango_attr_list_new();
+
+  // TODO(deanm): This is a hack, just to handle coloring prefix_text.
+  // Hopefully I can clean up the match situation a bit and this will
+  // come out cleaner.  For now, apply the base color to the whole text
+  // so that our prefix will have the base color applied.
+  PangoAttribute* base_fg_attr = pango_attr_foreground_new(
+      base_color->red, base_color->green, base_color->blue);
+  pango_attr_list_insert(attrs, base_fg_attr);  // Ownership taken.
+
+  // Walk through the classifications, they are linear, in order, and should
+  // cover the entire text.  We create a bunch of overlapping attributes,
+  // extending from the offset to the end of the string.  The ones created
+  // later will override the previous ones, meaning we will still setup each
+  // portion correctly, we just don't need to compute the end offset.
+  for (ACMatchClassifications::const_iterator i = classifications.begin();
+       i != classifications.end(); ++i) {
+    size_t offset = GetUTF8Offset(localized_text, i->offset) +
+                    additional_offset;
+
+    // TODO(deanm): All the colors should probably blend based on whether this
+    // result is selected or not.  This would include the green URLs.  Right
+    // now the caller is left to blend only the base color.  Do we need to
+    // handle things like DIM urls?  Turns out DIM means something different
+    // than you'd think, all of the description text is not DIM, it is a
+    // special case that is not very common, but we should figure out and
+    // support it.
+    const GdkColor* color = base_color;
+    if (i->style & ACMatchClassification::URL) {
+      color = url_color;
+      // Insert a left to right embedding to make sure that URLs are shown LTR.
+      if (is_rtl && !marked_with_lre) {
+        std::string lre(kLRE);
+        text_utf8.insert(offset, lre);
+        additional_offset += lre.length();
+      }
+    }
+
+    if (i->style & ACMatchClassification::DIM)
+      color = dim_color;
+
+    PangoAttribute* fg_attr = pango_attr_foreground_new(
+        color->red, color->green, color->blue);
+    fg_attr->start_index = offset;
+    pango_attr_list_insert(attrs, fg_attr);  // Ownership taken.
+
+    // Matched portions are bold, otherwise use the normal weight.
+    PangoWeight weight = (i->style & ACMatchClassification::MATCH) ?
+        PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
+    PangoAttribute* weight_attr = pango_attr_weight_new(weight);
+    weight_attr->start_index = offset;
+    pango_attr_list_insert(attrs, weight_attr);  // Ownership taken.
+  }
+
+  pango_layout_set_text(layout, text_utf8.data(), text_utf8.length());
+  pango_layout_set_attributes(layout, attrs);  // Ref taken.
+  pango_attr_list_unref(attrs);
+}
+
 void OmniboxPopupViewGtk::Show(size_t num_results) {
   gint origin_x, origin_y;
   GdkWindow* gdk_window = gtk_widget_get_window(location_bar_);
   gdk_window_get_origin(gdk_window, &origin_x, &origin_y);
   GtkAllocation allocation;
   gtk_widget_get_allocation(location_bar_, &allocation);
 
   int horizontal_offset = 1;
   gtk_window_move(GTK_WINDOW(window_),
       origin_x + allocation.x - kBorderThickness + horizontal_offset,
@@ skipping 13 matching lines @@
 }
 
 void OmniboxPopupViewGtk::StackWindow() {
   gfx::NativeView omnibox_view = omnibox_view_->GetNativeView();
   DCHECK(GTK_IS_WIDGET(omnibox_view));
   GtkWidget* toplevel = gtk_widget_get_toplevel(omnibox_view);
   DCHECK(gtk_widget_is_toplevel(toplevel));
   ui::StackPopupWindow(window_, toplevel);
 }
 
-size_t OmniboxPopupViewGtk::LineFromY(int y) {
-  size_t line = std::max(y - kBorderThickness, 0) / kHeightPerResult;
-  return std::min(line, model_->result().size() - 1);
-}
-
 void OmniboxPopupViewGtk::AcceptLine(size_t line,
                                      WindowOpenDisposition disposition) {
   // OpenMatch() may close the popup, which will clear the result set and, by
   // extension, |match| and its contents.  So copy the relevant match out to
   // make sure it stays alive until the call completes.
-  AutocompleteMatch match = model_->result().match_at(line);
+  AutocompleteMatch match = GetResult().match_at(line);
   omnibox_view_->OpenMatch(match, disposition, GURL(), line);
 }
 
 gfx::Image OmniboxPopupViewGtk::IconForMatch(
     const AutocompleteMatch& match,
     bool selected,
     bool is_selected_keyword) {
   const gfx::Image image = model_->GetIconIfExtensionMatch(match);
   if (!image.IsEmpty())
     return image;
@@ skipping 29 matching lines @@
     }
   }
 
   return theme_service_->GetImageNamed(icon);
 }
 
 void OmniboxPopupViewGtk::GetVisibleMatchForInput(
     size_t index,
     const AutocompleteMatch** match,
     bool* is_selected_keyword) {
-  const AutocompleteResult& result = model_->result();
+  const AutocompleteResult& result = GetResult();
 
   if (result.match_at(index).associated_keyword.get() &&
       model_->selected_line() == index &&
       model_->selected_line_state() == OmniboxPopupModel::KEYWORD) {
     *match = result.match_at(index).associated_keyword.get();
     *is_selected_keyword = true;
     return;
   }
 
   *match = &result.match_at(index);
@@ skipping 43 matching lines @@
     default:
       // Don't open the result.
       break;
   }
   return TRUE;
 }
 
 gboolean OmniboxPopupViewGtk::HandleExpose(GtkWidget* widget,
                                            GdkEventExpose* event) {
   bool ltr = !base::i18n::IsRTL();
-  const AutocompleteResult& result = model_->result();
+  const AutocompleteResult& result = GetResult();
 
   gfx::Rect window_rect = GetWindowRect(event->window);
   gfx::Rect damage_rect = gfx::Rect(event->area);
   // Handle when our window is super narrow.  A bunch of the calculations
   // below would go negative, and really we're not going to fit anything
   // useful in such a small window anyway.  Just don't paint anything.
   // This means we won't draw the border, but, yeah, whatever.
   // TODO(deanm): Make the code more robust and remove this check.
   if (window_rect.width() < (kIconAreaWidth * 3))
     return TRUE;
 
   cairo_t* cr = gdk_cairo_create(gtk_widget_get_window(widget));
   gdk_cairo_rectangle(cr, &event->area);
   cairo_clip(cr);
 
   // This assert is kinda ugly, but it would be more currently unneeded work
   // to support painting a border that isn't 1 pixel thick.  There is no point
   // in writing that code now, and explode if that day ever comes.
   COMPILE_ASSERT(kBorderThickness == 1, border_1px_implied);
   // Draw the 1px border around the entire window.
   gdk_cairo_set_source_color(cr, &border_color_);
   cairo_rectangle(cr, 0, 0, window_rect.width(), window_rect.height());
   cairo_stroke(cr);
 
   pango_layout_set_height(layout_, kHeightPerResult * PANGO_SCALE);
 
-  for (size_t i = 0; i < result.size(); ++i) {
+  for (size_t i = GetHiddenMatchCount(); i < result.size(); ++i) {
     gfx::Rect line_rect = GetRectForLine(i, window_rect.width());
     // Only repaint and layout damaged lines.
     if (!line_rect.Intersects(damage_rect))
       continue;
 
     const AutocompleteMatch* match = NULL;
     bool is_selected_keyword = false;
     GetVisibleMatchForInput(i, &match, &is_selected_keyword);
     bool is_selected = (model_->selected_line() == i);
     bool is_hovered = (model_->hovered_line() == i);
@@ skipping 90 matching lines @@
                               theme_service_->GetImageNamed(
                                   is_selected ? IDR_OMNIBOX_TTS_DARK :
                                   IDR_OMNIBOX_TTS),
                               icon_start_x, line_rect.y() + kIconTopPadding);
     }
   }
 
   cairo_destroy(cr);
   return TRUE;
 }