Re-land this from earlier... unit tests no longer crash.

Extract a cross platform LocationBar interface.
Adds a TestLocationBar object that unit tests can use to mock the location bar (fixes NULL deref).

Review URL: http://codereview.chromium.org/18851

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8745 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 201 insertions, 88 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index f4bccc7..fe947c5 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -1948,7 +1948,9 @@ void AutomationProvider::GetAutocompleteEditForBrowser(
 
   if (browser_tracker_->ContainsHandle(browser_handle)) {
     Browser* browser = browser_tracker_->GetResource(browser_handle);
-    LocationBarView* loc_bar_view = browser->GetLocationBarView();
+    BrowserWindowTesting* testing_interface =
+        browser->window()->GetBrowserWindowTesting();
+    LocationBarView* loc_bar_view = testing_interface->GetLocationBarView();
     AutocompleteEditView* edit_view = loc_bar_view->location_entry();
     // Add() returns the existing handle for the resource if any.
     autocomplete_edit_handle = autocomplete_edit_tracker_->Add(edit_view);
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 7cb862c..d685068 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -8,6 +8,7 @@
 #include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/location_bar.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/tab_contents/tab_contents_type.h"
 #include "chrome/common/chrome_constants.h"
@@ -275,7 +276,7 @@ void Browser::CreateBrowserWindow() {
       local_state->GetBoolean(prefs::kShouldShowFirstRunBubble)) {
     // Reset the preference so we don't show the bubble for subsequent windows.
     local_state->ClearPref(prefs::kShouldShowFirstRunBubble);
-    GetLocationBarView()->ShowFirstRunBubble();
+    window_->GetLocationBar()->ShowFirstRunBubble();
   }
 }
 
@@ -625,18 +626,15 @@ void Browser::Home() {
 #if defined(OS_WIN)
 void Browser::OpenCurrentURL() {
   UserMetrics::RecordAction(L"LoadURL", profile_);
-  LocationBarView* lbv = GetLocationBarView();
-  if (lbv) {
-    OpenURL(GURL(lbv->location_input()), GURL(), lbv->disposition(),
-            lbv->transition());
-  }
+  LocationBar* location_bar = window_->GetLocationBar();
+  OpenURL(GURL(location_bar->GetInputString()), GURL(),
+               location_bar->GetWindowOpenDisposition(),
+               location_bar->GetPageTransition());
 }
 
 void Browser::Go() {
   UserMetrics::RecordAction(L"Go", profile_);
-  LocationBarView* lbv = GetLocationBarView();
-  if (lbv)
-    lbv->location_entry()->model()->AcceptInput(CURRENT_TAB, false);
+  window_->GetLocationBar()->AcceptInput();
 }
 #endif
 
@@ -899,23 +897,13 @@ void Browser::FocusToolbar() {
 
 void Browser::FocusLocationBar() {
   UserMetrics::RecordAction(L"FocusLocation", profile_);
-  LocationBarView* lbv = GetLocationBarView();
-  if (lbv) {
-    AutocompleteEditView* aev = lbv->location_entry();
-    aev->SetFocus();
-    aev->SelectAll(true);
-  }
+  window_->GetLocationBar()->FocusLocation();
 }
 
 void Browser::FocusSearch() {
   // TODO(beng): replace this with FocusLocationBar
   UserMetrics::RecordAction(L"FocusSearch", profile_);
-  LocationBarView* lbv = GetLocationBarView();
-  if (lbv) {
-    AutocompleteEditView* aev = lbv->location_entry();
-    aev->SetUserText(L"?");
-    aev->SetFocus();
-  }
+  window_->GetLocationBar()->FocusSearch();
 }
 
 void Browser::OpenFile() {
@@ -1476,12 +1464,10 @@ void Browser::TabSelectedAt(TabContents* old_contents,
   if (!chrome_updater_factory_.empty() && old_contents)
     ProcessPendingUIUpdates();
 
-  LocationBarView* location_bar = GetLocationBarView();
   if (old_contents) {
     // Save what the user's currently typing, so it can be restored when we
     // switch back to this tab.
-    if (location_bar)
-      location_bar->location_entry()->SaveStateToTab(old_contents);
+    window_->GetLocationBar()->SaveStateToContents(old_contents);
   }
 
   // Propagate the profile to the location bar.
@@ -2275,10 +2261,6 @@ void Browser::RemoveScheduledUpdatesFor(TabContents* contents) {
 ///////////////////////////////////////////////////////////////////////////////
 // Browser, Getters for UI (private):
 
-LocationBarView* Browser::GetLocationBarView() const {
-  return window_->GetLocationBarView();
-}
-
 StatusBubble* Browser::GetStatusBubble() {
   return window_->GetStatusBubble();
 }
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index db9f326..c6977d6 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -35,7 +35,7 @@ class BrowserIdleTimer;
 class BrowserWindow;
 class DebuggerWindow;
 class GoButton;
-class LocationBarView;
+class LocationBar;
 class PrefService;
 class Profile;
 class StatusBubble;
@@ -456,7 +456,6 @@ class Browser : public TabStripModelDelegate,
 #endif  // OS_WIN
 
   // Getters for the location bar and go button.
-  LocationBarView* GetLocationBarView() const;
   GoButton* GetGoButton();
 
   // Returns the StatusBubble from the current toolbar. It is possible for
diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj
index c1cbed3..32f9051 100644
--- a/chrome/browser/browser.vcproj
+++ b/chrome/browser/browser.vcproj
@@ -734,6 +734,10 @@
 				>
 			</File>
 			<File
+				RelativePath=".\location_bar.h"
+				>
+			</File>
+			<File
 				RelativePath=".\login_prompt.cc"
 				>
 			</File>
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index 38f32b4..408f4ff 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -5,13 +5,11 @@
 #ifndef CHROME_BROWSER_BROWSER_WINDOW_H_
 #define CHROME_BROWSER_BROWSER_WINDOW_H_
 
-class BookmarkBarView;
 class Browser;
 class BrowserList;
-class BrowserView;
 class BrowserWindowTesting;
 class GURL;
-class LocationBarView;
+class LocationBar;
 class HtmlDialogContentsDelegate;
 class Profile;
 class StatusBubble;
@@ -25,7 +23,7 @@ class Rect;
 // BrowserWindow interface
 //  An interface implemented by the "view" of the Browser window.
 //
-// NOTE: all getters, save GetTabStrip(), may return NULL.
+// NOTE: All getters except GetTabStrip() may return NULL.
 class BrowserWindow {
  public:
   // Initialize the frame.
@@ -89,7 +87,7 @@ class BrowserWindow {
   virtual bool IsMaximized() = 0;
 
   // Returns the location bar.
-  virtual LocationBarView* GetLocationBarView() const = 0;
+  virtual LocationBar* GetLocationBar() const = 0;
 
   // Informs the view whether or not a load is in progress for the current tab.
   // The view can use this notification to update the go/stop button.
@@ -157,13 +155,19 @@ class BrowserWindow {
   virtual void DestroyBrowser() = 0;
 };
 
+class BookmarkBarView;
+class LocationBarView;
+
 // A BrowserWindow utility interface used for accessing elements of the browser
 // UI used only by UI test automation.
 class BrowserWindowTesting {
-public:
+ public:
 #if defined(OS_WIN)
-  // Returns the Bookmark Bar view.
+  // Returns the BookmarkBarView.
   virtual BookmarkBarView* GetBookmarkBarView() = 0;
+
+  // Returns the LocationBarView.
+  virtual LocationBarView* GetLocationBarView() const = 0;
 #endif
 };
 
diff --git a/chrome/browser/browser_window_cocoa.h b/chrome/browser/browser_window_cocoa.h
index 88d9fed..fa7e707 100644
--- a/chrome/browser/browser_window_cocoa.h
+++ b/chrome/browser/browser_window_cocoa.h
@@ -35,8 +35,7 @@ class BrowserWindowCocoa : public BrowserWindow {
   virtual void SetStarredState(bool is_starred);
   virtual gfx::Rect GetNormalBounds() const;
   virtual bool IsMaximized();
-  virtual LocationBarView* GetLocationBarView() const;
-  virtual BookmarkBarView* GetBookmarkBarView();
+  virtual LocationBar* GetLocationBar() const;
   virtual void UpdateStopGoState(bool is_loading);
   virtual void UpdateToolbar(TabContents* contents,
                              bool should_restore_state);
diff --git a/chrome/browser/browser_window_cocoa.mm b/chrome/browser/browser_window_cocoa.mm
index 095600f..d178f02 100644
--- a/chrome/browser/browser_window_cocoa.mm
+++ b/chrome/browser/browser_window_cocoa.mm
@@ -81,11 +81,7 @@ bool BrowserWindowCocoa::IsMaximized() {
   return [window_ isZoomed];
 }
 
-LocationBarView* BrowserWindowCocoa::GetLocationBarView() const {
-  return NULL;
-}
-
-BookmarkBarView* BrowserWindowCocoa::GetBookmarkBarView() {
+LocationBar* BrowserWindowCocoa::GetLocationBar() const {
   return NULL;
 }
 
diff --git a/chrome/browser/location_bar.h b/chrome/browser/location_bar.h
new file mode 100644
index 0000000..0e6bc83
--- /dev/null
+++ b/chrome/browser/location_bar.h
@@ -0,0 +1,44 @@
+// Copyright (c) 2009 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_LOCATION_BAR_H_
+#define CHROME_BROWSER_LOCATION_BAR_H_
+
+////////////////////////////////////////////////////////////////////////////////
+// LocationBar
+//  An interface providing access to the Location Bar component of the browser
+//  window.
+//
+class LocationBar {
+ public:
+  // Shows the first run information bubble anchored to the location bar.
+  virtual void ShowFirstRunBubble() = 0;
+
+  // Returns the string of text entered in the location bar.
+  virtual std::wstring GetInputString() const = 0;
+
+  // Returns the WindowOpenDisposition that should be used to determine where to
+  // open a URL entered in the location bar.
+  virtual WindowOpenDisposition GetWindowOpenDisposition() const = 0;
+
+  // Returns the PageTransition that should be recorded in history when the URL
+  // entered in the location bar is loaded.
+  virtual PageTransition::Type GetPageTransition() const = 0;
+
+  // Accepts the current string of text entered in the location bar.
+  virtual void AcceptInput() = 0;
+
+  // Focuses and selects the contents of the location bar.
+  virtual void FocusLocation() = 0;
+
+  // Clears the location bar, inserts an annoying little "?" turd and sets focus
+  // to it.
+  virtual void FocusSearch() = 0;
+
+  // Saves the state of the location bar to the specified TabContents, so that
+  // it can be restored later. (Done when switching tabs).
+  virtual void SaveStateToContents(TabContents* contents) = 0;
+};
+
+#endif  //  #ifndef CHROME_BROWSER_LOCATION_BAR_H_
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 6caf2cd..1e5ad23 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -172,8 +172,8 @@ void BrowserView::WindowMoved() {
   status_bubble_->Reposition();
 
   // Close the omnibox popup, if any.
-  if (GetLocationBarView())
-    GetLocationBarView()->location_entry()->ClosePopup();
+  if (toolbar_->GetLocationBarView())
+    toolbar_->GetLocationBarView()->location_entry()->ClosePopup();
 }
 
 gfx::Rect BrowserView::GetToolbarBounds() const {
@@ -485,14 +485,10 @@ bool BrowserView::IsMaximized() {
   return frame_->GetWindow()->IsMaximized();
 }
 
-LocationBarView* BrowserView::GetLocationBarView() const {
+LocationBar* BrowserView::GetLocationBar() const {
   return toolbar_->GetLocationBarView();
 }
 
-BrowserView* BrowserView::GetBrowserView() const {
-  return NULL;
-}
-
 void BrowserView::UpdateStopGoState(bool is_loading) {
   toolbar_->GetGoButton()->ChangeMode(
       is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO);
@@ -648,6 +644,10 @@ BookmarkBarView* BrowserView::GetBookmarkBarView() {
   return bookmark_bar_view_.get();
 }
 
+LocationBarView* BrowserView::GetLocationBarView() const {
+  return toolbar_->GetLocationBarView();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, NotificationObserver implementation:
 
@@ -739,7 +739,7 @@ std::wstring BrowserView::GetWindowTitle() const {
 }
 
 views::View* BrowserView::GetInitiallyFocusedView() const {
-  return GetLocationBarView();
+  return toolbar_->GetLocationBarView();
 }
 
 bool BrowserView::ShouldShowWindowTitle() const {
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index 33960f0..1a6ff36 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -169,8 +169,7 @@ class BrowserView : public BrowserWindow,
   virtual void SetStarredState(bool is_starred);
   virtual gfx::Rect GetNormalBounds() const;
   virtual bool IsMaximized();
-  virtual LocationBarView* GetLocationBarView() const;
-  virtual BrowserView* GetBrowserView() const;
+  virtual LocationBar* GetLocationBar() const;
   virtual void UpdateStopGoState(bool is_loading);
   virtual void UpdateToolbar(TabContents* contents, bool should_restore_state);
   virtual void FocusToolbar();
@@ -193,6 +192,7 @@ class BrowserView : public BrowserWindow,
 
   // Overridden from BrowserWindowTesting:
   virtual BookmarkBarView* GetBookmarkBarView();
+  virtual LocationBarView* GetLocationBarView() const;
 
   // Overridden from NotificationObserver:
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index aa6fb1d..0ee51ea 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -876,13 +876,6 @@ void LocationBarView::ShowFirstRunBubbleInternal() {
       bounds);
 }
 
-void LocationBarView::ShowFirstRunBubble() {
-  // We wait 30 milliseconds to open. It allows less flicker.
-  Task* task = first_run_bubble_.NewRunnableMethod(
-      &LocationBarView::ShowFirstRunBubbleInternal);
-  MessageLoop::current()->PostDelayedTask(FROM_HERE, task, 30);
-}
-
 // SecurityImageView------------------------------------------------------------
 
 // static
@@ -1005,3 +998,42 @@ bool LocationBarView::OverrideAccelerator(
   return location_entry_->OverrideAccelerator(accelerator);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// LocationBarView, LocationBar implementation:
+
+void LocationBarView::ShowFirstRunBubble() {
+  // We wait 30 milliseconds to open. It allows less flicker.
+  Task* task = first_run_bubble_.NewRunnableMethod(
+      &LocationBarView::ShowFirstRunBubbleInternal);
+  MessageLoop::current()->PostDelayedTask(FROM_HERE, task, 30);
+}
+
+std::wstring LocationBarView::GetInputString() const {
+  return location_input_;
+}
+
+WindowOpenDisposition LocationBarView::GetWindowOpenDisposition() const {
+  return disposition_;
+}
+
+PageTransition::Type LocationBarView::GetPageTransition() const {
+  return transition_;
+}
+
+void LocationBarView::AcceptInput() {
+  location_entry_->model()->AcceptInput(CURRENT_TAB, false);
+}
+
+void LocationBarView::FocusLocation() {
+  location_entry_->SetFocus();
+  location_entry_->SelectAll(true);
+}
+
+void LocationBarView::FocusSearch() {
+  location_entry_->SetUserText(L"?");
+  location_entry_->SetFocus();
+}
+
+void LocationBarView::SaveStateToContents(TabContents* contents) {
+  location_entry_->SaveStateToTab(contents);
+}
diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h
index eb3b4ef..ec01c63 100644
--- a/chrome/browser/views/location_bar_view.h
+++ b/chrome/browser/views/location_bar_view.h
@@ -9,6 +9,7 @@
 
 #include "base/gfx/rect.h"
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
+#include "chrome/browser/location_bar.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/toolbar_model.h"
 #include "chrome/browser/views/info_bubble.h"
@@ -30,10 +31,10 @@ class Profile;
 //   of the URL bar strip and contains its content.
 //
 /////////////////////////////////////////////////////////////////////////////
-class LocationBarView : public views::View,
+class LocationBarView : public LocationBar,
+                        public views::View,
                         public AutocompleteEditController {
  public:
-
   class Delegate {
    public:
     // Should return the current tab contents.
@@ -107,25 +108,19 @@ class LocationBarView : public views::View,
     return location_entry_.get();
   }
 
-  std::wstring location_input() {
-    return location_input_;
-  }
-
-  WindowOpenDisposition disposition() {
-    return disposition_;
-  }
-
-  PageTransition::Type transition() {
-    return transition_;
-  }
-
-  // Shows a info bubble that tells the user what the omnibox is and allows
-  // to change the search providers.
-  void ShowFirstRunBubble();
-
-  // Overridden from View.
+  // Overridden from views::View:
   virtual bool OverrideAccelerator(const views::Accelerator& accelerator);
 
+  // Overridden from LocationBar:
+  virtual void ShowFirstRunBubble();
+  virtual std::wstring GetInputString() const;
+  virtual WindowOpenDisposition GetWindowOpenDisposition() const;
+  virtual PageTransition::Type GetPageTransition() const;
+  virtual void AcceptInput();
+  virtual void FocusLocation();
+  virtual void FocusSearch();
+  virtual void SaveStateToContents(TabContents* contents);
+
   static const int kTextVertMargin;
   static const COLORREF kBackgroundColorByLevel[];
 
diff --git a/chrome/common/temp_scaffolding_stubs.h b/chrome/common/temp_scaffolding_stubs.h
index ed710bf..034de96 100644
--- a/chrome/common/temp_scaffolding_stubs.h
+++ b/chrome/common/temp_scaffolding_stubs.h
@@ -294,11 +294,6 @@ class SavePackage {
   static bool IsSavableURL(const GURL& url) { return false; }
 };
 
-class LocationBarView {
- public:
-  void ShowFirstRunBubble() { }
-};
-
 class DebuggerWindow : public base::RefCountedThreadSafe<DebuggerWindow> {
  public:
 };
diff --git a/chrome/test/test_browser_window.h b/chrome/test/test_browser_window.h
index 29ab2bc..fef495a 100644
--- a/chrome/test/test_browser_window.h
+++ b/chrome/test/test_browser_window.h
@@ -8,6 +8,7 @@
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/views/tabs/tab_strip.h"
+#include "chrome/test/test_location_bar.h"
 
 // An implementation of BrowserWindow used for testing. TestBrowserWindow only
 // contains a valid TabStrip, all other getters return NULL.
@@ -34,8 +35,9 @@ class TestBrowserWindow : public BrowserWindow {
   virtual void SetStarredState(bool is_starred) {}
   virtual gfx::Rect GetNormalBounds() const { return gfx::Rect(); }
   virtual bool IsMaximized() { return false; }
-  virtual LocationBarView* GetLocationBarView() const { return NULL; }
-  virtual BookmarkBarView* GetBookmarkBarView() { return NULL; }
+  virtual LocationBar* GetLocationBar() const {
+    return const_cast<TestLocationBar*>(&location_bar_);
+  }
   virtual void UpdateStopGoState(bool is_loading) {}
   virtual void UpdateToolbar(TabContents* contents,
                              bool should_restore_state) {}
@@ -62,6 +64,8 @@ class TestBrowserWindow : public BrowserWindow {
  private:
   TabStrip tab_strip_;
 
+  TestLocationBar location_bar_;
+
   DISALLOW_COPY_AND_ASSIGN(TestBrowserWindow);
 };
 
diff --git a/chrome/test/test_location_bar.h b/chrome/test/test_location_bar.h
new file mode 100644
index 0000000..6f9b2ab
--- /dev/null
+++ b/chrome/test/test_location_bar.h
@@ -0,0 +1,53 @@
+// Copyright (c) 2009 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_TEST_TEST_LOCATION_BAR_H_
+#define CHROME_TEST_TEST_LOCATION_BAR_H_
+
+#include "chrome/browser/location_bar.h"
+#include "chrome/common/page_transition_types.h"
+#include "webkit/glue/window_open_disposition.h"
+
+class TestLocationBar : public LocationBar {
+ public:
+  TestLocationBar()
+      : disposition_(CURRENT_TAB),
+        transition_(PageTransition::LINK) {
+  }
+
+  void set_input_string(const std::wstring& input_string) {
+   input_string_ = input_string;
+  }
+  void set_disposition(WindowOpenDisposition disposition) {
+   disposition_ = disposition;
+  }
+  void set_transition(PageTransition::Type transition) {
+   transition_ = transition;
+  }
+
+  // Overridden from LocationBar:
+  virtual void ShowFirstRunBubble() {}
+  virtual std::wstring GetInputString() const { return input_string_; }
+  virtual WindowOpenDisposition GetWindowOpenDisposition() const {
+    return disposition_;
+  }
+  virtual PageTransition::Type GetPageTransition() const { return transition_; }
+  virtual void AcceptInput() {}
+  virtual void FocusLocation() {}
+  virtual void FocusSearch() {}
+  virtual void SaveStateToContents(TabContents* contents) {}
+
+ private:
+  
+  // Test-supplied values that will be returned through the LocationBar
+  // interface.
+  std::wstring input_string_;
+  WindowOpenDisposition disposition_;
+  PageTransition::Type transition_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestLocationBar);
+};
+
+
+#endif  // #ifndef CHROME_TEST_TEST_LOCATION_BAR_H_
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index aa5ee79..03f50f0 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -734,6 +734,10 @@
 					>
 				</File>
 				<File
+					RelativePath="..\test_location_bar.h"
+					>
+				</File>
+				<File
 					RelativePath="..\..\browser\renderer_host\test_render_view_host.cc"
 					>
 				</File>