When internal functions try to focus the location bar, check whether it's focusable, and clear the focus if it's not. This shouldn't kick in in any cases today (I don't think), but it will matter in fullscreen mode, where creating a new tab should not attempt to focus the (unfocusable) location bar, but should not do nothing either, since these leaves focus in a hosed state.

There are other choices here.  I could make View::RequestFocus() try and clear the focus if the view is not focusable.  That seems likely to cause side-effects unless I restore a lot of the calls to IsFocusable() that I previously removed.  I also don't know whether sticking this on BrowserWindow was the right approach; if there's a better spot architecturally, speak up.
Review URL: http://codereview.chromium.org/21368

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

5 files changed, 25 insertions, 5 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 222fab2..59f6267 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1848,10 +1848,14 @@ void Browser::ShowHtmlDialog(HtmlDialogContentsDelegate* delegate,
 }
 
 void Browser::SetFocusToLocationBar() {
-  // This is the same as FocusLocationBar above but doesn't record the user
-  // metrics. This TabContentsDelegate version is called internally, so
-  // shouldn't get recorded in user commands.
-  window_->GetLocationBar()->FocusLocation();
+  // Two differences between this and FocusLocationBar():
+  // (1) This doesn't get recorded in user metrics, since it's called
+  //     internally.
+  // (2) This checks whether the location bar can be focused, and if not, clears
+  //     the focus.  FocusLocationBar() is only reached when the location bar is
+  //     focusable, but this may be reached at other times, e.g. while in
+  //     fullscreen mode, where we need to leave focus in a consistent state.
+  window_->SetFocusToLocationBar();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index 5a43bb8..7491ee0 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -88,6 +88,10 @@ class BrowserWindow {
   // Returns the location bar.
   virtual LocationBar* GetLocationBar() const = 0;
 
+  // Tries to focus the location bar.  Clears the window focus (to avoid
+  // inconsistent state) if this fails.
+  virtual void SetFocusToLocationBar() = 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.
   virtual void UpdateStopGoState(bool is_loading) = 0;
diff --git a/chrome/browser/tab_contents/native_ui_contents.cc b/chrome/browser/tab_contents/native_ui_contents.cc
index 9ff65ce..fecc5e2 100644
--- a/chrome/browser/tab_contents/native_ui_contents.cc
+++ b/chrome/browser/tab_contents/native_ui_contents.cc
@@ -317,7 +317,7 @@ void NativeUIContents::SetInitialFocus() {
     Browser* browser = Browser::GetBrowserForController(
         this->controller(), &tab_index);
     if (browser)
-      browser->FocusLocationBar();
+      browser->SetFocusToLocationBar();
     else
       TabContents::SetInitialFocus();  // Will set focus to our HWND.
   }
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 144f0a1..9d0f706 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -546,6 +546,17 @@ LocationBar* BrowserView::GetLocationBar() const {
   return toolbar_->GetLocationBarView();
 }
 
+void BrowserView::SetFocusToLocationBar() {
+  LocationBarView* location_bar = toolbar_->GetLocationBarView();
+  if (location_bar->IsFocusable()) {
+    location_bar->FocusLocation();
+  } else {
+    views::FocusManager* focus_manager = GetFocusManager();
+    DCHECK(focus_manager);
+    focus_manager->ClearFocus();
+  }
+}
+
 void BrowserView::UpdateStopGoState(bool is_loading) {
   toolbar_->GetGoButton()->ChangeMode(
       is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO);
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index 6f945b8..0ece17e 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -170,6 +170,7 @@ class BrowserView : public BrowserWindow,
   virtual gfx::Rect GetNormalBounds() const;
   virtual bool IsMaximized() const;
   virtual LocationBar* GetLocationBar() const;
+  virtual void SetFocusToLocationBar();
   virtual void UpdateStopGoState(bool is_loading);
   virtual void UpdateToolbar(TabContents* contents, bool should_restore_state);
   virtual void FocusToolbar();