Refactor AutocompleteEditViewGtk so that AutocompleteEditView impl can be swapped.

chrome/browser/autocomplete/autocomplete_edit_view_gtk.h

Index: chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
index dc09d9143779d37b8728d7b5f9fc01744ecdea97..c8ccbc9674a96e3c46aa0824dbd943ee8491ebe1 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
@@ -26,6 +26,7 @@
 #include "gfx/rect.h"
 #include "webkit/glue/window_open_disposition.h"
 
+class AccessibleWidgetHelper;
 class AutocompleteEditController;
 class AutocompleteEditModel;
 class AutocompletePopupView;
@@ -71,19 +72,15 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
                           CommandUpdater* command_updater,
                           bool popup_window_mode,
 #if defined(TOOLKIT_VIEWS)
-                          const views::View* location_bar);
+                          const views::View* location_bar
 #else
-                          GtkWidget* location_bar);
+                          GtkWidget* location_bar
 #endif
-  ~AutocompleteEditViewGtk();
+                          );

[Peter Kasting, 2011/01/06 02:06:48]

Nit: I slightly prefer leaving the ); on the individual lines

[oshima, 2011/01/06 19:43:35]

unbalanced parenthesis and brackets confused emacs indentation, so I'd like to
keep this if you don't mind.
I can change it back if you feel strongly about it, so please let me know.

[Peter Kasting, 2011/01/06 20:51:38]

I don't feel strongly, but I do think the old way was better.  MSVC indentation
is constantly confused by style quirks we have and I don't worry much about
that; I'm not sure emacs is more important.

[oshima, 2011/01/06 21:39:26]

This code is gtk (thus linux) code and I assume there will be more emacs users
needs to edit this file often than MSVC users. This part will become obsolete
once we migrate, so I'd like to keep this way for now.

+  virtual ~AutocompleteEditViewGtk();
 
   // Initialize, create the underlying widgets, etc.
   void Init();
-
-  // Returns the width in pixels needed to display the current text. The
-  // returned value includes margins.
-  int TextWidth();
-
   // Returns the width in pixels needed to display the text from one character
   // before the caret to the end of the string. See comments in
   // LocationBarView::Layout as to why this uses -1.
@@ -143,6 +140,25 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
   virtual bool OnAfterPossibleChange();
   virtual gfx::NativeView GetNativeView() const;
   virtual CommandUpdater* GetCommandUpdater();
+#if defined(TOOLKIT_VIEWS)
+  virtual int TextWidth() const;
+  virtual views::View* AddToView(views::View* parent);
+  virtual bool CommitInstantSuggestion(const std::wstring& typed_text,
+                                       const std::wstring& suggested_text);
+  virtual void SetInstantSuggestion(const string16& suggestion);
+
+  void CreateAccessibleWidgetHelper(int res);

[Peter Kasting, 2011/01/06 02:06:48]

Nit: |res| is a meaningless name; pick something more descriptive (e.g. xxx_id)
and add a comment about what this function does

[dmazzoni, 2011/01/06 15:03:03]

Maybe EnableAccessibility would be a more clear name? I'd just have it take no
arguments unless you expect it'd ever be called with anything other than
IDS_ACCNAME_LOCATION (like if you expect the AutocompleteEditView to be used for
something other than the omnibox?)

[oshima, 2011/01/06 19:43:35]

Done.

+
+  static AutocompleteEditView* Create(AutocompleteEditController* controller,

[Peter Kasting, 2011/01/06 02:06:48]

Nit: A comment about what this function's responsibilities are would also be
nice.

[dmazzoni, 2011/01/06 15:03:03]

What's the reason for adding a static Create method when the constructor
sufficed before? Would it make sense to do the same to AutocompleteEditViewWin
for consistency?

[oshima, 2011/01/06 19:43:35]

Done.

[oshima, 2011/01/06 19:43:35]

It will return either AutocompleteEditViewGtk or AutocompleteEditViewViews. I
may move this to more common place though, but i'd rather keep it here for now.

+                                      ToolbarModel* toolbar_model,
+                                      Profile* profile,
+                                      CommandUpdater* command_updater,
+                                      bool popup_window_mode,
+                                      const views::View* location_bar);
+#else
+  int TextWidth() const;

[Peter Kasting, 2011/01/06 02:06:48]

Nit: Since there's only one definition of this function, it seems better to have
one declaration outside the #ifdefs than one in each branch.  Adding "virtual"
in the non-views case isn't going to make any real difference.

[oshima, 2011/01/06 19:43:35]

Done.

+#endif
+
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
@@ -514,6 +530,10 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
 
   GtkSignalRegistrar signals_;
 
+#if defined(TOOLKIT_VIEWS)
+  scoped_ptr<AccessibleWidgetHelper> accessible_widget_helper_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(AutocompleteEditViewGtk);
 };