Upgrade Seperator in New Autofill UI

chrome/browser/ui/gtk/autofill/autofill_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 "autofill_popup_view_gtk.h"
 
 #include <gdk/gdkkeysyms.h>
 
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_external_delegate.h"
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #include "chrome/browser/ui/gtk/gtk_theme_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h"
 #include "ui/base/gtk/gtk_compat.h"
 #include "ui/base/gtk/gtk_hig_constants.h"
 #include "ui/base/gtk/gtk_windowing.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
 
+using WebKit::WebAutofillClient;
+
 namespace {
 const GdkColor kBorderColor = GDK_COLOR_RGB(0xc7, 0xca, 0xce);
 const GdkColor kHoveredBackgroundColor = GDK_COLOR_RGB(0x0CD, 0xCD, 0xCD);
 const GdkColor kTextColor = GDK_COLOR_RGB(0x00, 0x00, 0x00);
 
 // The vertical height of each row in pixels.
 const int kRowHeight = 24;
 
+// The bertical height of a separator in pixels.

[Ilya Sherman, 2012/05/23 18:58:08]

nit: "bertical" -> "vertical"

[csharp, 2012/05/24 13:40:10]

Done.

+const int kSeparatorHeight = 1;
+
 // The amount of minimum padding between the Autofill value and label in pixels.
 const int kMiddlePadding = 10;
 
 // The amount of padding between the label and icon or edge and icon in pixels.
 const int kIconPadding = 5;
 
 // We have a 1 pixel border around the entire results popup.
 const int kBorderThickness = 1;
 
 // Width of the icons in pixels.
 const int kIconWidth = 25;
 
 // Height of the icons in pixels.
 const int kIconHeight = 16;
 
 gfx::Rect GetWindowRect(GdkWindow* window) {
   return gfx::Rect(gdk_window_get_width(window),
                    gdk_window_get_height(window));
 }
 
-// Returns the rectangle containing the item at position |index| in the popup.
-gfx::Rect GetRectForRow(size_t index, int width) {
-  return gfx::Rect(0, (index * kRowHeight), width, kRowHeight);
+int GetRowHeight(int unique_id) {
+  if (unique_id == WebAutofillClient::MenuItemIDSeparator)
+    return kSeparatorHeight;
+
+  return kRowHeight;
 }
 
 }  // namespace
 
 AutofillPopupViewGtk::AutofillPopupViewGtk(
     content::WebContents* web_contents,
     GtkThemeService* theme_service,
     AutofillExternalDelegate* external_delegate,
     GtkWidget* parent)
     : AutofillPopupView(web_contents, external_delegate),
@@ skipping 52 matching lines @@
 void AutofillPopupViewGtk::InvalidateRow(size_t row) {
   GdkRectangle row_rect = GetRectForRow(row, bounds_.width()).ToGdkRectangle();
   GdkWindow* gdk_window = gtk_widget_get_window(window_);
   gdk_window_invalidate_rect(gdk_window, &row_rect, FALSE);
 }
 
 void AutofillPopupViewGtk::ResizePopup() {
   gtk_widget_set_size_request(
       window_,
       GetPopupRequiredWidth(),
-      kRowHeight * autofill_values().size());
+      GetPopupRequiredHeight());
 }
 
 gboolean AutofillPopupViewGtk::HandleButtonRelease(GtkWidget* widget,
                                                    GdkEventButton* event) {
   // We only care about the left click.
   if (event->button != 1)
     return FALSE;
 
   DCHECK_EQ(selected_line(), LineFromY(event->y));
 
@@ skipping 26 matching lines @@
   pango_layout_get_size(layout_, &actual_content_width, &actual_content_height);
   actual_content_width /= PANGO_SCALE;
   actual_content_height /= PANGO_SCALE;
 
   for (size_t i = 0; i < autofill_values().size(); ++i) {
     gfx::Rect line_rect = GetRectForRow(i, window_rect.width());
     // Only repaint and layout damaged lines.
     if (!line_rect.Intersects(damage_rect))
       continue;
 
-    if (IsSeparatorIndex(i)) {
-      int line_y = i * kRowHeight;
-
-      cairo_save(cr);
-      cairo_move_to(cr, 0, line_y);
-      cairo_line_to(cr, window_rect.width(), line_y);
-      cairo_stroke(cr);
-      cairo_restore(cr);
-    }
-
-    if (selected_line() == static_cast<int>(i)) {
-      gdk_cairo_set_source_color(cr, &kHoveredBackgroundColor);
-      cairo_rectangle(cr, line_rect.x(), line_rect.y(),
-                      line_rect.width(), line_rect.height());
-      cairo_fill(cr);
-    }
-
-    // Center the text within the line.
-    int content_y = std::max(
-        line_rect.y(),
-        line_rect.y() + ((kRowHeight - actual_content_height) / 2));
-
-    // Draw the value.
-    gtk_util::SetLayoutText(layout_, autofill_values()[i]);
-
-    cairo_save(cr);
-    cairo_move_to(cr, 0, content_y);
-    pango_cairo_show_layout(cr, layout_);
-    cairo_restore(cr);
-
-    // Draw the label.
-    int x_align_left = window_rect.width() -
-        font_.GetStringWidth(autofill_labels()[i]);
-
-    if (!autofill_icons()[i].empty())
-      x_align_left -= 2 * kIconPadding + kIconWidth;
-
-    gtk_util::SetLayoutText(layout_, autofill_labels()[i]);
-
-    cairo_save(cr);
-    cairo_move_to(cr, x_align_left, content_y);
-    pango_cairo_show_layout(cr, layout_);
-    cairo_restore(cr);
-
-    // Draw the icon, if one exists
-    if (!autofill_icons()[i].empty()) {
-      int icon = GetIconResourceID(autofill_icons()[i]);
-      DCHECK_NE(-1, icon);
-      int icon_y = line_rect.y() + ((kRowHeight - kIconHeight) / 2);
-
-      cairo_save(cr);
-      gtk_util::DrawFullImage(cr,
-                              window_,
-                              theme_service_->GetImageNamed(icon),
-                              window_rect.width() -
-                                (kIconPadding + kIconWidth),
-                              icon_y);
-      cairo_restore(cr);
+    switch (autofill_unique_ids()[i]) {

[Ilya Sherman, 2012/05/23 18:58:08]

nit: Since there are only two cases, I think this might be clearer as an
if-else.

[csharp, 2012/05/24 13:40:10]

Done.

+      case WebAutofillClient::MenuItemIDSeparator:
+        DrawSeparator(cr, line_rect);
+        break;
+      default:
+        DrawAutofillEntry(cr, i, actual_content_height, line_rect);
     }
   }
 
   cairo_destroy(cr);
 
   return TRUE;
 }
 
 gboolean AutofillPopupViewGtk::HandleMotion(GtkWidget* widget,
                                             GdkEventMotion* event) {
@@ skipping 46 matching lines @@
   PangoAttribute* fg_attr = pango_attr_foreground_new(text_color.red,
                                                       text_color.green,
                                                       text_color.blue);
   pango_attr_list_insert(attrs, fg_attr);  // Ownership taken.
 
 
   pango_layout_set_attributes(layout_, attrs);  // Ref taken.
   pango_attr_list_unref(attrs);
 }
 
+
+void AutofillPopupViewGtk::DrawSeparator(cairo_t* cr,
+                                         const gfx::Rect& separator_rect) {
+  cairo_save(cr);
+  cairo_move_to(cr, 0, separator_rect.y());
+  cairo_line_to(cr,
+                separator_rect.width(),
+                separator_rect.y() + separator_rect.height());
+  cairo_stroke(cr);
+  cairo_restore(cr);
+}
+
+void AutofillPopupViewGtk::DrawAutofillEntry(cairo_t* cr,
+                                             size_t index,
+                                             int actual_content_height,
+                                             const gfx::Rect& entry_rect) {
+  if (selected_line() == static_cast<int>(index)) {
+    gdk_cairo_set_source_color(cr, &kHoveredBackgroundColor);
+    cairo_rectangle(cr, entry_rect.x(), entry_rect.y(),
+                    entry_rect.width(), entry_rect.height());
+    cairo_fill(cr);
+  }
+
+  // Center the text within the line.
+  int content_y = std::max(
+      entry_rect.y(),
+      entry_rect.y() + ((kRowHeight - actual_content_height) / 2));
+
+  // Draw the value.
+  gtk_util::SetLayoutText(layout_, autofill_values()[index]);
+
+  cairo_save(cr);
+  cairo_move_to(cr, 0, content_y);
+  pango_cairo_show_layout(cr, layout_);
+  cairo_restore(cr);
+
+  // Draw the label.
+  int x_align_left = entry_rect.width() -
+      font_.GetStringWidth(autofill_labels()[index]);
+
+  if (!autofill_icons()[index].empty())
+    x_align_left -= 2 * kIconPadding + kIconWidth;
+
+  gtk_util::SetLayoutText(layout_, autofill_labels()[index]);
+
+  cairo_save(cr);
+  cairo_move_to(cr, x_align_left, content_y);
+  pango_cairo_show_layout(cr, layout_);
+  cairo_restore(cr);
+
+  // Draw the icon, if one exists
+  if (!autofill_icons()[index].empty()) {
+    int icon = GetIconResourceID(autofill_icons()[index]);
+    DCHECK_NE(-1, icon);
+    int icon_y = entry_rect.y() + ((kRowHeight - kIconHeight) / 2);
+
+    cairo_save(cr);
+    gtk_util::DrawFullImage(cr,
+                            window_,
+                            theme_service_->GetImageNamed(icon),
+                            entry_rect.width() -
+                            (kIconPadding + kIconWidth),
+                            icon_y);
+    cairo_restore(cr);
+  }
+}
+
+
 void AutofillPopupViewGtk::SetBounds() {
   gint origin_x, origin_y;
   gdk_window_get_origin(gtk_widget_get_window(parent_), &origin_x, &origin_y);
 
   GdkScreen* screen = gtk_widget_get_screen(parent_);
   gint screen_height = gdk_screen_get_height(screen);
 
   int bottom_of_field = origin_y + element_bounds().y() +
       element_bounds().height();
-  int popup_size =  kRowHeight * autofill_values().size();
+  int popup_height =  GetPopupRequiredHeight();
 
   // Find the correct top position of the popup so that is doesn't go off
   // the screen.
   int top_of_popup = 0;
-  if (screen_height < bottom_of_field + popup_size) {
+  if (screen_height < bottom_of_field + popup_height) {
     // The popup must appear above the field.
-    top_of_popup = origin_y + element_bounds().y() - popup_size;
+    top_of_popup = origin_y + element_bounds().y() - popup_height;
   } else {
     // The popup can appear below the field.
     top_of_popup = bottom_of_field;
   }
 
   bounds_.SetRect(
       origin_x + element_bounds().x(),
       top_of_popup,
       GetPopupRequiredWidth(),
-      popup_size);
+      popup_height);
 }
 
 int AutofillPopupViewGtk::GetPopupRequiredWidth() {
   int popup_width = element_bounds().width();
   DCHECK_EQ(autofill_values().size(), autofill_labels().size());
   for (size_t i = 0; i < autofill_values().size(); ++i) {
     int row_size = font_.GetStringWidth(autofill_values()[i]) +
         kMiddlePadding +
         font_.GetStringWidth(autofill_labels()[i]);
 
     // Add the icon size if required.
     if (!autofill_icons()[i].empty())
       row_size += kIconWidth + 2 * kIconPadding;
 
     popup_width = std::max(popup_width, row_size);
   }
 
   return popup_width;
 }
 
+int AutofillPopupViewGtk::GetPopupRequiredHeight() {
+  int popup_height = 0;
+
+  for (size_t i = 0; i < autofill_unique_ids().size(); ++i) {
+    popup_height += GetRowHeight(autofill_unique_ids()[i]);
+  }
+
+  return popup_height;
+}
+
 int AutofillPopupViewGtk::LineFromY(int y) {
-  return y / kRowHeight;
+  int current_height = 0;
+
+  for (size_t i = 0; i < autofill_unique_ids().size(); ++i) {
+    current_height += GetRowHeight(autofill_unique_ids()[i]);
+
+    if (y <= current_height)
+      return i;
+  }
+
+  // The y value goes beyond the popup so stop the selection at the last line.
+  return autofill_unique_ids().size() - 1;

[Ilya Sherman, 2012/05/23 18:58:08]

Hmm, I think we might want to return "-1" in this case instead.  I know this
isn't how the WebKit popup currently works, but if your mouse is off the popup,
no row should be selected (this has been a long-standing bug with the WebKit
popup).  It's fine if you want to tackle this bug more generally in a separate
CL, since you'll also need to look at the x-coordinate of the mouse position for
this to work correctly.

[csharp, 2012/05/24 13:40:10]

I'm leaving this as is for now, but I've added a TODO and filed a bug to fix the
behaviour.

 }
+
+gfx::Rect AutofillPopupViewGtk::GetRectForRow(size_t row, int width) {
+  int top = 0;
+  for (size_t i = 0; i < row; ++i) {
+    top += GetRowHeight(autofill_unique_ids()[i]);
+  }
+
+  return gfx::Rect(0, top, width, GetRowHeight(autofill_unique_ids()[row]));
+}