The focus is not restored properly when a Windows modal dialog

(such as "open file', 'print'...) is closed.
We are running these dialogs from a different thread and they
cause the browser window to get activated before it has been
enabled. This causes the focus restoration to fail as the window
is not enabled.
In an earlier patch, I fixed it by storing/restoring the focus
explicitly before/after the dialog is shown.
But the fix did not apply to the print dialog which does not use
the code I added my fix in.

This CL reverts that previous fix and comes with a simpler solution:
if when we are about to restore focus the window is disabled, we
delay the focus restoration untill the window is enabled again.

BUG=3380
TEST=Set the focus on a page with scroll-bars, right-click to do a
    'save as'. Close the dialog. The arrow keys should let you scroll
    the page. Accelerators such as Ctrl-T should still work. Test
    'open a file', 'print' and the font selection dialog (in the options).
    When closing the dialog the focus should return to the view that last
    had focus. 

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

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

3 files changed, 22 insertions, 43 deletions

diff --git a/chrome/browser/views/shell_dialogs_win.cc b/chrome/browser/views/shell_dialogs_win.cc
index fa2b6f0..5c5b1f9 100644
--- a/chrome/browser/views/shell_dialogs_win.cc
+++ b/chrome/browser/views/shell_dialogs_win.cc
@@ -21,8 +21,6 @@
 #include "base/thread.h"
 #include "chrome/browser/browser_process.h"
 #include "grit/generated_resources.h"
-#include "views/focus/focus_manager.h"
-#include "views/focus/view_storage.h"
 
 // Helpers to show certain types of Windows shell dialogs in a way that doesn't
 // block the UI of the entire app.
@@ -118,11 +116,6 @@ class BaseShellDialogImpl {
   // This set only contains non-null HWNDs. NULL hwnds are not added to this
   // list.
   typedef std::set<HWND> Owners;
-
-  // Storage id used to store the last focused view so we can restore focus
-  // appropriately.
-  int view_storage_id_;
-
   static Owners owners_;
   static int instance_count_;
 
@@ -134,9 +127,7 @@ BaseShellDialogImpl::Owners BaseShellDialogImpl::owners_;
 int BaseShellDialogImpl::instance_count_ = 0;
 
 BaseShellDialogImpl::BaseShellDialogImpl()
-    : ui_loop_(MessageLoop::current()),
-      view_storage_id_(views::ViewStorage::GetSharedInstance()->
-          CreateStorageID()) {
+    : ui_loop_(MessageLoop::current()) {
   ++instance_count_;
 }
 
@@ -157,20 +148,6 @@ BaseShellDialogImpl::RunState BaseShellDialogImpl::BeginRun(HWND owner) {
   run_state.owner = owner;
   if (owner) {
     owners_.insert(owner);
-    // Disabling the owner causes the browser to be disabled when the dialog is
-    // closed and the browser gets the activation.  This messes up the normal
-    // focus restoration process.  To work-around this, we'll restore the focus
-    // ourselves after we have reenabled the owner.
-    views::FocusManager* focus_manager =
-        views::FocusManager::GetFocusManagerForNativeView(owner);
-    if (focus_manager) {
-      views::View* focused_view = focus_manager->GetFocusedView();
-      if (focused_view)
-        views::ViewStorage::GetSharedInstance()->StoreView(view_storage_id_,
-                                                           focused_view);
-    } else {
-      NOTREACHED();
-    }
     DisableOwner(owner);
   }
   return run_state;
@@ -182,24 +159,6 @@ void BaseShellDialogImpl::EndRun(RunState run_state) {
     EnableOwner(run_state.owner);
     DCHECK(owners_.find(run_state.owner) != owners_.end());
     owners_.erase(run_state.owner);
-    // Now that the owner is enabled, restore the focus if applicable.
-    views::View* view_to_focus =
-        views::ViewStorage::GetSharedInstance()->RetrieveView(view_storage_id_);
-    if (view_to_focus) {
-      views::ViewStorage::GetSharedInstance()->RemoveView(view_storage_id_);
-      views::FocusManager* focus_manager =
-          views::FocusManager::GetFocusManagerForNativeView(run_state.owner);
-      if (focus_manager) {
-        // We need to clear focus as when the focus is restored when the dialog
-        // is closed, only setting the native focus fails.  Meaning the focused
-        // manager still believes the right view has focus, and would ignore
-        // requesting focus to what it thinks is already focused.
-        focus_manager->ClearFocus();
-        view_to_focus->RequestFocus();
-      } else {
-        NOTREACHED();
-      }
-    }
   }
   DCHECK(run_state.dialog_thread);
   delete run_state.dialog_thread;
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 8fbf31e..13df3ad 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -52,7 +52,8 @@ WidgetWin::WidgetWin()
       can_update_layered_window_(true),
       last_mouse_event_was_move_(false),
       is_mouse_down_(false),
-      is_window_(false) {
+      is_window_(false),
+      restore_focus_when_enabled_(false) {
 }
 
 WidgetWin::~WidgetWin() {
@@ -926,6 +927,10 @@ LRESULT WidgetWin::OnWndProc(UINT message, WPARAM w_param, LPARAM l_param) {
     OnFinalMessage(window);
   if (message == WM_ACTIVATE)
     PostProcessActivateMessage(this, LOWORD(w_param));
+  if (message == WM_ENABLE && restore_focus_when_enabled_) {
+    restore_focus_when_enabled_ = false;
+    focus_manager_->RestoreFocusedView();
+  }
   return result;
 }
 
@@ -941,6 +946,17 @@ void WidgetWin::PostProcessActivateMessage(WidgetWin* widget,
   } else {
     // We must restore the focus after the message has been DefProc'ed as it
     // does set the focus to the last focused HWND.
+    // Note that if the window is not enabled, we cannot restore the focus as
+    // calling ::SetFocus on a child of the non-enabled top-window would fail.
+    // This is the case when showing a modal dialog (such as 'open file',
+    // 'print'...) from a different thread.
+    // In that case we delay the focus restoration to when the window is enabled
+    // again.
+    if (!IsWindowEnabled(widget->GetNativeView())) {
+      DCHECK(!widget->restore_focus_when_enabled_);
+      widget->restore_focus_when_enabled_ = true;
+      return;
+    }
     widget->focus_manager_->RestoreFocusedView();
   }
 }
diff --git a/views/widget/widget_win.h b/views/widget/widget_win.h
index 88a2a33..787281d 100644
--- a/views/widget/widget_win.h
+++ b/views/widget/widget_win.h
@@ -586,6 +586,10 @@ class WidgetWin : public base::WindowImpl,
   int last_mouse_move_x_;
   int last_mouse_move_y_;
 
+  // Whether the focus should be restored next time we get enabled.  Needed to
+  // restore focus correctly when Windows modal dialogs are displayed.
+  bool restore_focus_when_enabled_;
+
   // Instance of accessibility information and handling for MSAA root
   ScopedComPtr<IAccessible> accessibility_root_;