Fixes bug where showing an auth dialog during instant would trigger

the page to be prematurely committed.

BUG=56462
TEST=see bug

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

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

9 files changed, 69 insertions, 20 deletions

diff --git a/chrome/browser/login_prompt_win.cc b/chrome/browser/login_prompt_win.cc
index 39f1497..e82899d 100644
--- a/chrome/browser/login_prompt_win.cc
+++ b/chrome/browser/login_prompt_win.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/views/login_view.h"
 #include "chrome/common/notification_service.h"
@@ -104,7 +105,11 @@ class LoginHandlerWin : public LoginHandler,
                                            std::wstring explanation) {
     DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
-    LoginView* view = new LoginView(explanation);
+    TabContents* tab_contents = GetTabContentsForLogin();
+    bool should_focus_view = !tab_contents->delegate() ||
+        tab_contents->delegate()->ShouldFocusConstrainedWindow(tab_contents);
+
+    LoginView* view = new LoginView(explanation, should_focus_view);
 
     // Set the model for the login view. The model (password manager) is owned
     // by the view's parent TabContents, so natural destruction order means we
diff --git a/chrome/browser/tab_contents/match_preview.cc b/chrome/browser/tab_contents/match_preview.cc
index deb9a9c..2b25bb2f 100644
--- a/chrome/browser/tab_contents/match_preview.cc
+++ b/chrome/browser/tab_contents/match_preview.cc
@@ -215,7 +215,7 @@ class MatchPreview::PaintObserverImpl : public RenderWidgetHost::PaintObserver {
   }
 
   virtual void RenderWidgetHostDidPaint(RenderWidgetHost* rwh) {
-    match_preview_->PreviewDidPaint();
+    match_preview_->ShowPreview();
     rwh->set_paint_observer(NULL);
     // WARNING: we've been deleted.
   }
@@ -305,6 +305,24 @@ class MatchPreview::TabContentsDelegateImpl : public TabContentsDelegate {
   virtual bool IsPopup(const TabContents* source) const {
     return false;
   }
+  virtual bool ShouldFocusConstrainedWindow(TabContents* source) {
+    // Return false so that constrained windows are not initially focused. If
+    // we did otherwise the preview would prematurely get committed when focus
+    // goes to the constrained window.
+    return false;
+  }
+  virtual void WillShowConstrainedWindow(TabContents* source) {
+    if (!match_preview_->is_active()) {
+      // A constrained window shown for an auth may not paint. Show the preview
+      // contents.
+      if (installed_paint_observer_) {
+        source->GetRenderWidgetHostView()->GetRenderWidgetHost()->
+            set_paint_observer(NULL);
+      }
+      installed_paint_observer_ = true;
+      match_preview_->ShowPreview();
+    }
+  }
   virtual TabContents* GetConstrainingContents(TabContents* source) {
     return NULL;
   }
@@ -608,7 +626,7 @@ void MatchPreview::SetCompleteSuggestedText(
       complete_suggested_text_.substr(user_text_.size()));
 }
 
-void MatchPreview::PreviewDidPaint() {
+void MatchPreview::ShowPreview() {
   DCHECK(!is_active_);
   is_active_ = true;
   delegate_->ShowMatchPreview();
diff --git a/chrome/browser/tab_contents/match_preview.h b/chrome/browser/tab_contents/match_preview.h
index d871abb..0adbe57f 100644
--- a/chrome/browser/tab_contents/match_preview.h
+++ b/chrome/browser/tab_contents/match_preview.h
@@ -96,9 +96,10 @@ class MatchPreview {
   // which results in updating the omnibox.
   void SetCompleteSuggestedText(const string16& suggested_text);
 
-  // Invoked when the preview paints. This notifies the delegate the preview is
-  // ready to be shown.
-  void PreviewDidPaint();
+  // Invoked to show the preview. This is invoked in two possible cases: when
+  // the renderer paints, or when an auth dialog is shown. This notifies the
+  // delegate the preview is ready to be shown.
+  void ShowPreview();
 
   // Invoked once the page has finished loading and the script has been sent.
   void PageFinishedLoading();
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.cc b/chrome/browser/tab_contents/tab_contents_delegate.cc
index 7e07ed6..3e8d50c 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.cc
+++ b/chrome/browser/tab_contents/tab_contents_delegate.cc
@@ -17,6 +17,13 @@ TabContents* TabContentsDelegate::GetConstrainingContents(TabContents* source) {
   return source;
 }
 
+bool TabContentsDelegate::ShouldFocusConstrainedWindow(TabContents* source) {
+  return true;
+}
+
+void TabContentsDelegate::WillShowConstrainedWindow(TabContents* source) {
+}
+
 void TabContentsDelegate::ContentsMouseEvent(
     TabContents* source, const gfx::Point& location, bool motion) {
 }
diff --git a/chrome/browser/tab_contents/tab_contents_delegate.h b/chrome/browser/tab_contents/tab_contents_delegate.h
index 6baa57c..e8a1684 100644
--- a/chrome/browser/tab_contents/tab_contents_delegate.h
+++ b/chrome/browser/tab_contents/tab_contents_delegate.h
@@ -102,6 +102,12 @@ class TabContentsDelegate : public AutomationResourceRoutingDelegate {
   // returns |source|.
   virtual TabContents* GetConstrainingContents(TabContents* source);
 
+  // Returns true if constrained windows should be focused. Default is true.
+  virtual bool ShouldFocusConstrainedWindow(TabContents* source);
+
+  // Invoked prior to the TabContents showing a constrained window.
+  virtual void WillShowConstrainedWindow(TabContents* source);
+
   // Notification that some of our content has changed size as
   // part of an animation.
   virtual void ToolbarSizeChanged(TabContents* source, bool is_animating) = 0;
diff --git a/chrome/browser/views/constrained_window_win.cc b/chrome/browser/views/constrained_window_win.cc
index 4230606..f997c3d 100644
--- a/chrome/browser/views/constrained_window_win.cc
+++ b/chrome/browser/views/constrained_window_win.cc
@@ -563,16 +563,20 @@ views::NonClientFrameView* ConstrainedWindowWin::CreateFrameViewForWindow() {
 }
 
 void ConstrainedWindowWin::FocusConstrainedWindow() {
-  if (GetDelegate() && GetDelegate()->GetInitiallyFocusedView())
+  if ((!owner_->delegate() ||
+       owner_->delegate()->ShouldFocusConstrainedWindow(owner_)) &&
+      GetDelegate() && GetDelegate()->GetInitiallyFocusedView()) {
     GetDelegate()->GetInitiallyFocusedView()->RequestFocus();
+  }
 }
 
 void ConstrainedWindowWin::ShowConstrainedWindow() {
+  if (owner_->delegate())
+    owner_->delegate()->WillShowConstrainedWindow(owner_);
   ActivateConstrainedWindow();
   FocusConstrainedWindow();
 }
 
-
 void ConstrainedWindowWin::CloseConstrainedWindow() {
   // Broadcast to all observers of NOTIFY_CWINDOW_CLOSED.
   // One example of such an observer is AutomationCWindowTracker in the
diff --git a/chrome/browser/views/location_bar/location_bar_view.cc b/chrome/browser/views/location_bar/location_bar_view.cc
index a55281a..98327a8 100644
--- a/chrome/browser/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/views/location_bar/location_bar_view.cc
@@ -1199,19 +1199,17 @@ void LocationBarView::OnTemplateURLModelChanged() {
 bool LocationBarView::ShouldCommitMatchPreviewOnFocusLoss(
     gfx::NativeView view_gaining_focus) {
   // The MatchPreview is active. Destroy it if the user didn't click on the
-  // RenderWidgetHostView (or one of its children).
+  // TabContents (or one of its children).
 #if defined(OS_WIN)
   MatchPreview* match_preview = delegate_->GetMatchPreview();
   if (!view_gaining_focus ||
       !match_preview->preview_contents()->GetRenderWidgetHostView()) {
     return false;
   }
-  RenderWidgetHostView* rwhv =
-      match_preview->preview_contents()->GetRenderWidgetHostView();
-  gfx::NativeView rwhv_native_view = rwhv->GetNativeView();
+  gfx::NativeView tab_view = match_preview->preview_contents()->GetNativeView();
   gfx::NativeView view_gaining_focus_ancestor = view_gaining_focus;
   while (view_gaining_focus_ancestor &&
-         view_gaining_focus_ancestor != rwhv_native_view) {
+         view_gaining_focus_ancestor != tab_view) {
     view_gaining_focus_ancestor = ::GetParent(view_gaining_focus_ancestor);
   }
   return view_gaining_focus_ancestor != NULL;
diff --git a/chrome/browser/views/login_view.cc b/chrome/browser/views/login_view.cc
index 819de41..3bf2403 100644
--- a/chrome/browser/views/login_view.cc
+++ b/chrome/browser/views/login_view.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -24,7 +24,8 @@ using views::GridLayout;
 ///////////////////////////////////////////////////////////////////////////////
 // LoginView, public:
 
-LoginView::LoginView(const std::wstring& explanation)
+LoginView::LoginView(const std::wstring& explanation,
+                     bool focus_view)
     : username_field_(new views::Textfield),
       password_field_(new views::Textfield(views::Textfield::STYLE_PASSWORD)),
       username_label_(new views::Label(
@@ -34,7 +35,8 @@ LoginView::LoginView(const std::wstring& explanation)
       message_label_(new views::Label(explanation)),
       ALLOW_THIS_IN_INITIALIZER_LIST(focus_grabber_factory_(this)),
       login_model_(NULL),
-      focus_delayed_(false) {
+      focus_delayed_(false),
+      focus_view_(focus_view) {
   message_label_->SetMultiLine(true);
   message_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   message_label_->SetAllowCharacterBreak(true);
@@ -100,6 +102,9 @@ void LoginView::SetModel(LoginModel* model) {
 }
 
 void LoginView::RequestFocus() {
+  if (!focus_view_)
+    return;
+
   MessageLoop::current()->PostTask(FROM_HERE,
     focus_grabber_factory_.NewRunnableMethod(&LoginView::FocusFirstField));
 }
@@ -108,7 +113,7 @@ void LoginView::RequestFocus() {
 // LoginView, views::View, views::LoginModelObserver overrides:
 
 void LoginView::ViewHierarchyChanged(bool is_add, View *parent, View *child) {
-  if (is_add && child == this) {
+  if (is_add && child == this && focus_view_) {
     MessageLoop::current()->PostTask(FROM_HERE,
         focus_grabber_factory_.NewRunnableMethod(&LoginView::FocusFirstField));
   }
@@ -117,7 +122,7 @@ void LoginView::ViewHierarchyChanged(bool is_add, View *parent, View *child) {
 void LoginView::NativeViewHierarchyChanged(bool attached,
                                            gfx::NativeView native_view,
                                            views::RootView* root_view) {
-  if (focus_delayed_ && attached) {
+  if (focus_delayed_ && attached && focus_view_) {
     focus_delayed_ = false;
     MessageLoop::current()->PostTask(FROM_HERE,
         focus_grabber_factory_.NewRunnableMethod(&LoginView::FocusFirstField));
@@ -137,6 +142,7 @@ void LoginView::OnAutofillDataAvailable(const std::wstring& username,
 // LoginView, private:
 
 void LoginView::FocusFirstField() {
+  DCHECK(focus_view_);
   if (GetFocusManager()) {
     username_field_->RequestFocus();
   } else {
diff --git a/chrome/browser/views/login_view.h b/chrome/browser/views/login_view.h
index 8618bc2..b4c133a 100644
--- a/chrome/browser/views/login_view.h
+++ b/chrome/browser/views/login_view.h
@@ -20,7 +20,8 @@ class LoginModel;
 // for HTTP/FTP authentication.
 class LoginView : public views::View, public LoginModelObserver {
  public:
-  explicit LoginView(const std::wstring& explanation);
+  // |focus_view| indicates if the view can be focused.
+  LoginView(const std::wstring& explanation, bool focus_view);
   virtual ~LoginView();
 
   // Access the data in the username/password text fields.
@@ -68,8 +69,11 @@ class LoginView : public views::View, public LoginModelObserver {
 
   ScopedRunnableMethodFactory<LoginView> focus_grabber_factory_;
 
+  // See description above constructor.
+  const bool focus_view_;
+
   // Indicates that this view was created when focus manager was unavailable
-  // (on the hidden tab, for example).
+  // (on the hidden tab, for example). This is only used if focus_view_ is true.
   bool focus_delayed_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginView);