Refactor AutocompleteEditViewGtk so that AutocompleteEditView impl can be swapped.

chrome/browser/autocomplete/autocomplete_edit_view_gtk.h

 // Copyright (c) 2010 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.
 
 #ifndef CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_EDIT_VIEW_GTK_H_
 #define CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_EDIT_VIEW_GTK_H_
 #pragma once
 
 #include <gtk/gtk.h>
 
 #include <algorithm>
 #include <string>
 
 #include "app/animation_delegate.h"
 #include "app/gtk_signal.h"
 #include "app/gtk_signal_registrar.h"
 #include "base/basictypes.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "chrome/browser/autocomplete/autocomplete_edit_view.h"
 #include "chrome/browser/gtk/owned_widget_gtk.h"
 #include "chrome/browser/ui/toolbar/toolbar_model.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/page_transition_types.h"
 #include "gfx/rect.h"
 #include "webkit/glue/window_open_disposition.h"
 
+class AccessibleWidgetHelper;
 class AutocompleteEditController;
 class AutocompleteEditModel;
 class AutocompletePopupView;
 class MultiAnimation;
 class Profile;
 class TabContents;
 
 namespace gfx{
 class Font;
 }
@@ skipping 25 matching lines @@
     int cp_min;  // For a selection: Represents the start.
     int cp_max;  // For a selection: Represents the end (insert position).
   };
 
   AutocompleteEditViewGtk(AutocompleteEditController* controller,
                           ToolbarModel* toolbar_model,
                           Profile* profile,
                           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.
   int WidthOfTextAfterCursor();
 
   // Returns the font.
   gfx::Font GetFont();
 
   // Implement the AutocompleteEditView interface.
   virtual AutocompleteEditModel* model();
@@ skipping 39 matching lines @@
 
   virtual void OnTemporaryTextMaybeChanged(const std::wstring& display_text,
                                            bool save_original_selection);
   virtual bool OnInlineAutocompleteTextMaybeChanged(
       const std::wstring& display_text, size_t user_text_length);
   virtual void OnRevertTemporaryText();
   virtual void OnBeforePossibleChange();
   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,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
   // Overridden from AnimationDelegate.
   virtual void AnimationEnded(const Animation* animation);
   virtual void AnimationProgressed(const Animation* animation);
   virtual void AnimationCanceled(const Animation* animation);
@@ skipping 351 matching lines @@
   // track preedit_'s content, as it'll be treated as part of text content.
   size_t preedit_size_before_change_;
 #endif
 
   // The view that is going to be focused next. Only valid while handling
   // "focus-out" events.
   GtkWidget* going_to_focus_;
 
   GtkSignalRegistrar signals_;
 
+#if defined(TOOLKIT_VIEWS)
+  scoped_ptr<AccessibleWidgetHelper> accessible_widget_helper_;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(AutocompleteEditViewGtk);
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_EDIT_VIEW_GTK_H_