views/location_bar: Eliminate TouchableLocationBarView.

chrome/browser/ui/views/location_bar/location_bar_view.h

Index: chrome/browser/ui/views/location_bar/location_bar_view.h
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.h b/chrome/browser/ui/views/location_bar/location_bar_view.h
index 96151da987aec284a8c479261c1cb6365c6e1e91..864557ab1f5ecd657cb8e9952d95efc6ac299b69 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.h
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.h
@@ -42,7 +42,6 @@ class GURL;
 class InstantController;
 class KeywordHintView;
 class LocationIconView;
-class MicSearchView;
 class OpenPDFInReaderView;
 class PageActionWithBadgeView;
 class PageActionImageView;
@@ -55,6 +54,7 @@ class ZoomView;
 
 namespace views {
 class BubbleDelegateView;
+class ImageButton;
 class Label;
 class Widget;
 }
@@ -140,6 +140,10 @@ class LocationBarView : public LocationBar,
 
   virtual ~LocationBarView();
 
+  // This function should be called if you want to make LocationBarView
+  // touchable.

[Peter Kasting, 2013/08/09 20:57:28]

This is still confusing.  This has nothing to do with LocationBarView being
touchable, it's about making child views touchable.  And it should be called
unconditionally for child views, not only if touch is enabled (it makes that
determination internally).

How about:

Uses GetBuiltInHorizontalPaddingForChildViews() to optionally add additional
padding (via an empty border) to |view|.  This should be called during creation
on all child views which are potentially touchable so that when touch is enabled
they will have sufficient padding.

[tfarina, 2013/08/10 02:13:40]

Done.

+  static void InitTouchableLocationBarView(views::View* view);

[Peter Kasting, 2013/08/09 20:57:28]

Nit: Perhaps for clarity, this should end "...ChildView" instead of "View".

[tfarina, 2013/08/10 02:13:40]

Done.

+
   // Initializes the LocationBarView.
   void Init();
 
@@ -333,11 +337,10 @@ class LocationBarView : public LocationBar,
   // Returns the position and width that the popup should be, and also the left
   // edge that the results should align themselves to (which will leave some
   // border on the left of the popup).
-  void GetOmniboxPopupPositioningInfo(
-      gfx::Point* top_left_screen_coord,
-      int* popup_width,
-      int* left_margin,
-      int* right_margin);
+  void GetOmniboxPopupPositioningInfo(gfx::Point* top_left_screen_coord,
+                                      int* popup_width,
+                                      int* left_margin,
+                                      int* right_margin);
 
   // Space between items in the location bar, as well as between items and the
   // edges.
@@ -362,6 +365,11 @@ class LocationBarView : public LocationBar,
   friend class PageActionWithBadgeView;
   typedef std::vector<PageActionWithBadgeView*> PageActionViews;
 
+  // Returns the number of pixels of built-in padding to the left and
+  // right for child views. This is used to increase the area in Location Bar to
+  // make it more touchable.

[Peter Kasting, 2013/08/09 20:57:28]

Nit: This comment likewise has problems.  We're not increasing the area of the
location bar.  How about:

...This is nonzero when touch UI is enabled so as to space out child views for
easier targeting.  See InitTouchableLocationBarChildView().

[tfarina, 2013/08/10 02:13:40]

Done.

+  static int GetBuiltInHorizontalPaddingForChildViews();
+
   // Returns the thickness of any visible left and right edge, in pixels.
   int GetHorizontalEdgeThickness() const;
 
@@ -475,7 +483,7 @@ class LocationBarView : public LocationBar,
   KeywordHintView* keyword_hint_view_;
 
   // The voice search icon.
-  MicSearchView* mic_search_view_;
+  views::ImageButton* mic_search_view_;
 
   // The content setting views.
   ContentSettingViews content_setting_views_;