Revert :

Changed by: jcampan@chromium.org
Changed at: Fri 19 Jun 2009 21:22:47
Branch: src
Revision: 18889
Comments:
Relanding focus manager refactoring with build fix, see:http://codereview.chromium.org/125148BUG=NoneTEST=NoneTBR=ben
Review URL: http://codereview.chromium.org/141013

Because it creates hundreds of new reliability crashes.

TBR:jcampan

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

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

5 files changed, 40 insertions, 51 deletions

diff --git a/views/widget/accelerator_handler.cc b/views/widget/accelerator_handler.cc
index c171b50..386357f 100644
--- a/views/widget/accelerator_handler.cc
+++ b/views/widget/accelerator_handler.cc
@@ -15,8 +15,7 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
   bool process_message = true;
 
   if (msg.message >= WM_KEYFIRST && msg.message <= WM_KEYLAST) {
-    FocusManager* focus_manager =
-        FocusManager::GetFocusManagerForNativeView(msg.hwnd);
+    FocusManager* focus_manager = FocusManager::GetFocusManager(msg.hwnd);
     if (focus_manager) {
       // FocusManager::OnKeyDown and OnKeyUp return false if this message has
       // been consumed and should not be propagated further.
diff --git a/views/widget/root_view.cc b/views/widget/root_view.cc
index 751786a..367c720 100644
--- a/views/widget/root_view.cc
+++ b/views/widget/root_view.cc
@@ -46,7 +46,7 @@ class PaintTask : public Task {
   // The target root view.
   RootView* root_view_;
 
-  DISALLOW_COPY_AND_ASSIGN(PaintTask);
+  DISALLOW_EVIL_CONSTRUCTORS(PaintTask);
 };
 
 const char RootView::kViewClassName[] = "views/RootView";
@@ -262,11 +262,23 @@ void RootView::ViewHierarchyChanged(bool is_add, View* parent, View* child) {
       default_keyboard_handler_ = NULL;
     }
 
-    FocusManager* focus_manager = widget_->GetFocusManager();
-    // An unparanted RootView does not have a FocusManager.
-    if (focus_manager)
+    // For a given widget hierarchy, focus is tracked by a FocusManager attached
+    // to our nearest enclosing Window. <-- Important Assumption!
+    // We may not have access to our window if this function is called as a
+    // result of teardown during the deletion of the RootView and its hierarchy,
+    // so we don't bother notifying the FocusManager in that case because it
+    // will have already been destroyed (the Widget that contains us is
+    // NCDESTROY'ed which in turn destroys the focus manager _before_ the
+    // RootView is deleted.)
+#if defined(OS_WIN)
+    Window* window = GetWindow();
+    if (window) {
+      FocusManager* focus_manager =
+          FocusManager::GetFocusManager(window->GetNativeWindow());
       focus_manager->ViewRemoved(parent, child);
+    }
     ViewStorage::GetSharedInstance()->ViewRemoved(parent, child);
+#endif
   }
 }
 
@@ -509,6 +521,7 @@ void RootView::OnWidgetDestroyed() {
   // TODO(port): Port RootViewDropTarget and this goes away.
   NOTIMPLEMENTED();
 #endif
+  widget_ = NULL;
 }
 
 void RootView::ProcessMouseDragCanceled() {
diff --git a/views/widget/widget.h b/views/widget/widget.h
index 9c4c5ef..1dabf08 100644
--- a/views/widget/widget.h
+++ b/views/widget/widget.h
@@ -17,7 +17,6 @@ class Rect;
 namespace views {
 
 class Accelerator;
-class FocusManager;
 class RootView;
 class TooltipManager;
 class Window;
@@ -106,17 +105,13 @@ class Widget {
   virtual Window* GetWindow() { return NULL; }
   virtual const Window* GetWindow() const { return NULL; }
 
-  // Gets the theme provider.
+  // Get the theme provider.
   virtual ThemeProvider* GetThemeProvider() const { return NULL; }
 
-  // Gets the default theme provider; this is necessary for when a widget has
+  // Get the default theme provider; this is necessary for when a widget has
   // no profile (and ThemeProvider) associated with it. The default theme
   // provider provides a default set of bitmaps that such widgets can use.
   virtual ThemeProvider* GetDefaultThemeProvider() { return NULL; }
-
-  // Returns the FocusManager for this widget.
-  // Note that all widgets in a widget hierarchy share the same focus manager.
-  virtual FocusManager* GetFocusManager() { return NULL; }
 };
 
 }  // namespace views
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 8f280d1..1fe3416 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -154,7 +154,8 @@ WidgetWin::~WidgetWin() {
   MessageLoopForUI::current()->RemoveObserver(this);
 }
 
-void WidgetWin::Init(HWND parent, const gfx::Rect& bounds) {
+void WidgetWin::Init(HWND parent, const gfx::Rect& bounds,
+                     bool has_own_focus_manager) {
   if (window_style_ == 0)
     window_style_ = parent ? kWindowDefaultChildStyle : kWindowDefaultStyle;
 
@@ -186,9 +187,8 @@ void WidgetWin::Init(HWND parent, const gfx::Rect& bounds) {
 
   root_view_->OnWidgetCreated();
 
-  if ((window_style_ & WS_CHILD) == 0) {
-    // Top-level widgets get a FocusManager.
-    focus_manager_.reset(new FocusManager(this));
+  if (has_own_focus_manager) {
+    FocusManager::CreateFocusManager(hwnd_, GetRootView());
   }
 
   // Sets the RootView as a property, so the automation can introspect windows.
@@ -419,18 +419,6 @@ const Window* WidgetWin::GetWindow() const {
   return GetWindowImpl(hwnd_);
 }
 
-FocusManager* WidgetWin::GetFocusManager() {
-  if (focus_manager_.get())
-    return focus_manager_.get();
-
-  HWND root = ::GetAncestor(hwnd_, GA_ROOT);
-  if (!root)
-    return NULL;
-
-  WidgetWin* widget = GetWidget(root);
-  return widget ?  widget->focus_manager_.get() : NULL;
-}
-
 void WidgetWin::SetUseLayeredBuffer(bool use_layered_buffer) {
   if (use_layered_buffer_ == use_layered_buffer)
     return;
@@ -472,12 +460,7 @@ RootView* WidgetWin::FindRootView(HWND hwnd) {
   return root_view;
 }
 
-// static
-WidgetWin* WidgetWin::GetWidget(HWND hwnd) {
-  return reinterpret_cast<WidgetWin*>(win_util::GetWindowUserData(hwnd));
-}
-
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////
 // MessageLoop::Observer
 
 void WidgetWin::WillProcessMessage(const MSG& msg) {
@@ -489,7 +472,7 @@ void WidgetWin::DidProcessMessage(const MSG& msg) {
   }
 }
 
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////
 // FocusTraversable
 
 View* WidgetWin::FindNextFocusableView(
@@ -1056,8 +1039,10 @@ LRESULT CALLBACK WidgetWin::WndProc(HWND window, UINT message,
   // Otherwise we handle everything else.
   if (!widget->ProcessWindowMessage(window, message, w_param, l_param, result))
     result = DefWindowProc(window, message, w_param, l_param);
-  if (message == WM_NCDESTROY)
+  if (message == WM_NCDESTROY) {
+    widget->hwnd_ = NULL;
     widget->OnFinalMessage(window);
+  }
   if (message == WM_ACTIVATE)
     PostProcessActivateMessage(widget, LOWORD(w_param));
   return result;
@@ -1066,16 +1051,18 @@ LRESULT CALLBACK WidgetWin::WndProc(HWND window, UINT message,
 // static
 void WidgetWin::PostProcessActivateMessage(WidgetWin* widget,
                                            int activation_state) {
-  if (!widget->focus_manager_.get()) {
+  FocusManager* focus_manager =
+      FocusManager::GetFocusManager(widget->GetNativeView());
+  if (!focus_manager) {
     NOTREACHED();
     return;
   }
   if (WA_INACTIVE == activation_state) {
-    widget->focus_manager_->StoreFocusedView();
+    focus_manager->StoreFocusedView();
   } else {
     // We must restore the focus after the message has been DefProc'ed as it
     // does set the focus to the last focused HWND.
-    widget->focus_manager_->RestoreFocusedView();
+    focus_manager->RestoreFocusedView();
   }
 }
 }  // namespace views
diff --git a/views/widget/widget_win.h b/views/widget/widget_win.h
index 6ae1141..b8500c7 100644
--- a/views/widget/widget_win.h
+++ b/views/widget/widget_win.h
@@ -78,7 +78,11 @@ class WidgetWin : public Widget,
   // this view, you are responsible for its destruction. If this value is NULL,
   // the caller is responsible for populating the RootView, and sizing its
   // contents as the window is sized.
-  void Init(HWND parent, const gfx::Rect& bounds);
+  // If |has_own_focus_manager| is true, the focus traversal stay confined to
+  // the window.
+  void Init(HWND parent,
+            const gfx::Rect& bounds,
+            bool has_own_focus_manager);
 
   // Sets the specified view as the contents of this Widget. There can only
   // be one contnets view child of this Widget's RootView. This view is sized to
@@ -118,9 +122,6 @@ class WidgetWin : public Widget,
   // Returns the RootView associated with the specified HWND (if any).
   static RootView* FindRootView(HWND hwnd);
 
-  // Returns the Widget associated with the specified HWND (if any).
-  static WidgetWin* GetWidget(HWND hwnd);
-
   // All classes registered by WidgetWin start with this name.
   static const wchar_t* const kBaseClassName;
 
@@ -229,7 +230,6 @@ class WidgetWin : public Widget,
   virtual ThemeProvider* GetThemeProvider() const;
   virtual Window* GetWindow();
   virtual const Window* GetWindow() const;
-  virtual FocusManager* GetFocusManager();
 
   // Overridden from MessageLoop::Observer:
   void WillProcessMessage(const MSG& msg);
@@ -327,6 +327,7 @@ class WidgetWin : public Widget,
   }
 
  protected:
+
   // Call close instead of this to Destroy the window.
   BOOL DestroyWindow() {
     DCHECK(::IsWindow(GetNativeView()));
@@ -522,12 +523,6 @@ class WidgetWin : public Widget,
   // the TooltipManager.
   scoped_ptr<TooltipManagerWin> tooltip_manager_;
 
-  // The focus manager keeping track of focus for this Widget and any of its
-  // children.  NULL for non top-level widgets.
-  // WARNING: RootView's destructor calls into the FocusManager. As such, this
-  // must be destroyed AFTER root_view_.
-  scoped_ptr<FocusManager> focus_manager_;
-
   // The root of the View hierarchy attached to this window.
   // WARNING: see warning in tooltip_manager_ for ordering dependencies with
   // this and tooltip_manager_.