Addded notification when ancestor gets changed. So windows that lack focus manager, because of being

created on inactive tab, could do the necessary work when focus manager is actually attached.
This is relevant for Windows only, but some support functions (FindAllRootViews) could be useful for
other architectures as well.
BUG=22481
TEST=in the bug
Review URL: http://codereview.chromium.org/492025

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

11 files changed, 380 insertions, 31 deletions

diff --git a/views/controls/native/native_view_host_win.cc b/views/controls/native/native_view_host_win.cc
index 4a8f357..75045ab 100644
--- a/views/controls/native/native_view_host_win.cc
+++ b/views/controls/native/native_view_host_win.cc
@@ -8,6 +8,7 @@
 #include "base/logging.h"
 #include "views/controls/native/native_view_host.h"
 #include "views/focus/focus_manager.h"
+#include "views/widget/root_view.h"
 #include "views/widget/widget.h"
 
 namespace views {
@@ -25,7 +26,6 @@ NativeViewHostWin::~NativeViewHostWin() {
 
 ////////////////////////////////////////////////////////////////////////////////
 // NativeViewHostWin, NativeViewHostWrapper implementation:
-
 void NativeViewHostWin::NativeViewAttached() {
   DCHECK(host_->native_view())
       << "Impossible detatched tab case; See crbug.com/6316";
@@ -37,10 +37,28 @@ void NativeViewHostWin::NativeViewAttached() {
   // Need to set the HWND's parent before changing its size to avoid flashing.
   SetParent(host_->native_view(), host_->GetWidget()->GetNativeView());
   host_->Layout();
+  // Notify children that parent changed, so they could adjust the focus
+  std::vector<RootView*> root_views;
+  Widget::FindAllRootViews(host_->native_view(), &root_views);
+  for (std::vector<RootView*>::iterator it = root_views.begin();
+       it < root_views.end();
+       ++it) {
+    (*it)->NotifyNativeViewHierarchyChanged(true,
+        host_->GetWidget()->GetNativeView());
+  }
 }
 
 void NativeViewHostWin::NativeViewDetaching(bool destroyed) {
   installed_clip_ = false;
+  // Notify children that parent is removed
+  std::vector<RootView*> root_views;
+  Widget::FindAllRootViews(host_->native_view(), &root_views);
+  for (std::vector<RootView*>::iterator it = root_views.begin();
+       it < root_views.end();
+       ++it) {
+    (*it)->NotifyNativeViewHierarchyChanged(false,
+        host_->GetWidget()->GetNativeView());
+  }
 }
 
 void NativeViewHostWin::AddedToWidget() {
diff --git a/views/view.cc b/views/view.cc
index 248622b..566de01 100644
--- a/views/view.cc
+++ b/views/view.cc
@@ -58,8 +58,10 @@ View::View()
       is_parent_owned_(true),
       notify_when_visible_bounds_in_root_changes_(false),
       registered_for_visible_bounds_notification_(false),
+      accelerator_registration_delayed_(false),
       next_focusable_view_(NULL),
       previous_focusable_view_(NULL),
+      accelerator_focus_manager_(NULL),
       registered_accelerator_count_(0),
       context_menu_controller_(NULL),
 #if defined(OS_WIN)
@@ -686,10 +688,16 @@ void View::ViewHierarchyChangedImpl(bool register_accelerators,
     if (is_add) {
       // If you get this registration, you are part of a subtree that has been
       // added to the view hierarchy.
-      RegisterPendingAccelerators();
+      if (GetFocusManager()) {
+        RegisterPendingAccelerators();
+      } else {
+        // Delay accelerator registration until visible as we do not have
+        // focus manager until then.
+        accelerator_registration_delayed_ = true;
+      }
     } else {
       if (child == this)
-        UnregisterAccelerators();
+        UnregisterAccelerators(false);
     }
   }
 
@@ -705,6 +713,34 @@ void View::PropagateVisibilityNotifications(View* start, bool is_visible) {
 void View::VisibilityChanged(View* starting_from, bool is_visible) {
 }
 
+void View::PropagateNativeViewHierarchyChanged(bool attached,
+                                               gfx::NativeView native_view,
+                                               RootView* root_view) {
+  for (int i = 0, count = GetChildViewCount(); i < count; ++i)
+    GetChildViewAt(i)->PropagateNativeViewHierarchyChanged(attached,
+                                                           native_view,
+                                                           root_view);
+  NativeViewHierarchyChanged(attached, native_view, root_view);
+}
+
+void View::NativeViewHierarchyChanged(bool attached,
+                                      gfx::NativeView native_view,
+                                      RootView* root_view) {
+  FocusManager* focus_manager = GetFocusManager();
+  if (!accelerator_registration_delayed_ &&
+      accelerator_focus_manager_ &&
+      accelerator_focus_manager_ != focus_manager) {
+    UnregisterAccelerators(true);
+    accelerator_registration_delayed_ = true;
+  }
+  if (accelerator_registration_delayed_ && attached) {
+    if (focus_manager) {
+      RegisterPendingAccelerators();
+      accelerator_registration_delayed_ = false;
+    }
+  }
+}
+
 void View::SetNotifyWhenVisibleBoundsInRootChanges(bool value) {
   if (notify_when_visible_bounds_in_root_changes_ == value)
     return;
@@ -988,18 +1024,16 @@ void View::RemoveAccelerator(const Accelerator& accelerator) {
     return;
   }
 
-  FocusManager* focus_manager = GetFocusManager();
-  if (focus_manager) {
-    // We may not have a FocusManager if the window containing us is being
-    // closed, in which case the FocusManager is being deleted so there is
-    // nothing to unregister.
-    focus_manager->UnregisterAccelerator(accelerator, this);
+  // If accelerator_focus_manager_ is NULL then we did not registered
+  // accelerators so there is nothing to unregister.
+  if (accelerator_focus_manager_) {
+    accelerator_focus_manager_->UnregisterAccelerator(accelerator, this);
   }
 }
 
 void View::ResetAccelerators() {
   if (accelerators_.get())
-    UnregisterAccelerators();
+    UnregisterAccelerators(false);
 }
 
 void View::RegisterPendingAccelerators() {
@@ -1016,8 +1050,8 @@ void View::RegisterPendingAccelerators() {
     return;
   }
 
-  FocusManager* focus_manager = GetFocusManager();
-  if (!focus_manager) {
+  accelerator_focus_manager_ = GetFocusManager();
+  if (!accelerator_focus_manager_) {
     // Some crash reports seem to show that we may get cases where we have no
     // focus manager (see bug #1291225).  This should never be the case, just
     // making sure we don't crash.
@@ -1033,26 +1067,28 @@ void View::RegisterPendingAccelerators() {
   std::vector<Accelerator>::const_iterator iter;
   for (iter = accelerators_->begin() + registered_accelerator_count_;
        iter != accelerators_->end(); ++iter) {
-    focus_manager->RegisterAccelerator(*iter, this);
+    accelerator_focus_manager_->RegisterAccelerator(*iter, this);
   }
   registered_accelerator_count_ = accelerators_->size();
 }
 
-void View::UnregisterAccelerators() {
+void View::UnregisterAccelerators(bool leave_data_intact) {
   if (!accelerators_.get())
     return;
 
   RootView* root_view = GetRootView();
   if (root_view) {
-    FocusManager* focus_manager = GetFocusManager();
-    if (focus_manager) {
+    if (accelerator_focus_manager_) {
       // We may not have a FocusManager if the window containing us is being
       // closed, in which case the FocusManager is being deleted so there is
       // nothing to unregister.
-      focus_manager->UnregisterAccelerators(this);
+      accelerator_focus_manager_->UnregisterAccelerators(this);
+      accelerator_focus_manager_ = NULL;
+    }
+    if (!leave_data_intact) {
+      accelerators_->clear();
+      accelerators_.reset();
     }
-    accelerators_->clear();
-    accelerators_.reset();
     registered_accelerator_count_ = 0;
   }
 }
diff --git a/views/view.h b/views/view.h
index 13a85ee..a9865d2 100644
--- a/views/view.h
+++ b/views/view.h
@@ -1019,6 +1019,18 @@ class View : public AcceleratorTarget {
   // invoked for that view as well as all the children recursively.
   virtual void VisibilityChanged(View* starting_from, bool is_visible);
 
+  // Called when the native view hierarchy changed.
+  // |attached| is true if that view has been attached to a new NativeView
+  // hierarchy, false if it has been detached.
+  // |native_view| is the NativeView this view was attached/detached from, and
+  // |root_view| is the root view associated with the NativeView.
+  // Views created without a native view parent don't have a focus manager.
+  // When this function is called they could do the processing that requires
+  // it - like registering accelerators, for example.
+  virtual void NativeViewHierarchyChanged(bool attached,
+                                          gfx::NativeView native_view,
+                                          RootView* root_view);
+
   // Called when the preferred size of a child view changed.  This gives the
   // parent an opportunity to do a fresh layout if that makes sense.
   virtual void ChildPreferredSizeChanged(View* child) {}
@@ -1136,6 +1148,12 @@ class View : public AcceleratorTarget {
   // Call VisibilityChanged() recursively for all children.
   void PropagateVisibilityNotifications(View* from, bool is_visible);
 
+  // Propagates NativeViewHierarchyChanged() notification through all the
+  // children.
+  void PropagateNativeViewHierarchyChanged(bool attached,
+                                           gfx::NativeView native_view,
+                                           RootView* root_view);
+
   // Takes care of registering/unregistering accelerators if
   // |register_accelerators| true and calls ViewHierarchyChanged().
   void ViewHierarchyChangedImpl(bool register_accelerators,
@@ -1184,7 +1202,9 @@ class View : public AcceleratorTarget {
   void RegisterPendingAccelerators();
 
   // Unregisters all the keyboard accelerators associated with this view.
-  void UnregisterAccelerators();
+  // |leave_data_intact| if true does not remove data from accelerators_ array,
+  // so it could be re-registered with other focus manager
+  void UnregisterAccelerators(bool leave_data_intact);
 
   // This View's bounds in the parent coordinate system.
   gfx::Rect bounds_;
@@ -1219,6 +1239,10 @@ class View : public AcceleratorTarget {
   // has been invoked.
   bool registered_for_visible_bounds_notification_;
 
+  // true if when we were added to hierarchy we were without focus manager
+  // attempt addition when ancestor chain changed.
+  bool accelerator_registration_delayed_;
+
   // List of descendants wanting notification when their visible bounds change.
   scoped_ptr<ViewList> descendants_to_notify_;
 
@@ -1228,6 +1252,9 @@ class View : public AcceleratorTarget {
   // Next view to be focused when the Shift-Tab key combination is pressed.
   View* previous_focusable_view_;
 
+  // Focus manager accelerators registered on.
+  FocusManager* accelerator_focus_manager_;
+
   // The list of accelerators. List elements in the range
   // [0, registered_accelerator_count_) are already registered to FocusManager,
   // and the rest are not yet.
diff --git a/views/view_unittest.cc b/views/view_unittest.cc
index 605d4838..1f4cfc1 100644
--- a/views/view_unittest.cc
+++ b/views/view_unittest.cc
@@ -16,6 +16,7 @@
 #if defined(OS_WIN)
 #include "views/controls/button/native_button_win.h"
 #endif
+#include "views/controls/native/native_view_host.h"
 #include "views/controls/scroll_view.h"
 #include "views/controls/textfield/textfield.h"
 #include "views/event.h"
@@ -59,6 +60,9 @@ class ViewTest : public testing::Test {
 #endif
   }
 
+  void RunPendingMessages() {
+    message_loop_.RunAllPending();
+  }
  private:
   MessageLoopForUI message_loop_;
 };
@@ -1207,3 +1211,138 @@ TEST_F(ViewTest, ChangeVisibility) {
   native->SetVisible(true);
 }
 */
+
+////////////////////////////////////////////////////////////////////////////////
+// Native view hierachy
+////////////////////////////////////////////////////////////////////////////////
+class TestNativeViewHierarchy : public views::View {
+ public:
+  TestNativeViewHierarchy() {
+  }
+
+  virtual void NativeViewHierarchyChanged(bool attached,
+                                          gfx::NativeView native_view,
+                                          RootView* root_view) {
+    NotificationInfo info;
+    info.attached = attached;
+    info.native_view = native_view;
+    info.root_view = root_view;
+    notifications_.push_back(info);
+  };
+  struct NotificationInfo {
+    bool attached;
+    gfx::NativeView native_view;
+    RootView* root_view;
+  };
+  static const size_t kTotalViews = 2;
+  std::vector<NotificationInfo> notifications_;
+};
+
+class TestChangeNativeViewHierarchy {
+ public:
+  explicit TestChangeNativeViewHierarchy(ViewTest *view_test) {
+    view_test_ = view_test;
+    native_host_ = new views::NativeViewHost();
+    host_ = view_test->CreateWidget();
+    host_->Init(NULL, gfx::Rect(0, 0, 500, 300));
+    host_->GetRootView()->AddChildView(native_host_);
+    for (size_t i = 0; i < TestNativeViewHierarchy::kTotalViews; ++i) {
+      windows_[i] = view_test->CreateWidget();
+      windows_[i]->Init(host_->GetNativeView(), gfx::Rect(0, 0, 500, 300));
+      root_views_[i] = windows_[i]->GetRootView();
+      test_views_[i] = new TestNativeViewHierarchy;
+      root_views_[i]->AddChildView(test_views_[i]);
+    }
+  }
+
+  ~TestChangeNativeViewHierarchy() {
+    for (size_t i = 0; i < TestNativeViewHierarchy::kTotalViews; ++i) {
+      windows_[i]->Close();
+    }
+    host_->Close();
+    // Will close and self-delete widgets - no need to manually delete them.
+    view_test_->RunPendingMessages();
+  }
+
+  void CheckEnumeratingRootViews() {
+    std::vector<RootView*> enumerated_root_views;
+#if defined(OS_WIN)
+    views::Widget::FindAllRootViews(host_->GetNativeView(),
+                                    &enumerated_root_views);
+#else
+    // host_->GetNativeView() returns gfx::NativeView which is GtkWidget on
+    // systems other than Windows and views::Widget::FindAllRootViews()
+    // requires GtkWindow.
+    if (host_->GetWindow()) {
+      views::Widget::FindAllRootViews(host_->GetWindow()->GetNativeWindow(),
+                                      &enumerated_root_views);
+    } else {
+      return;
+    }
+#endif
+    EXPECT_EQ(TestNativeViewHierarchy::kTotalViews + 1,
+              enumerated_root_views.size());
+    // Unfortunately there is no guarantee the sequence of views here so always
+    // go through all of them.
+    for (std::vector<RootView*>::iterator i = enumerated_root_views.begin();
+         i != enumerated_root_views.end(); ++i) {
+      if (host_->GetRootView() == *i)
+        continue;
+      size_t j;
+      for (j = 0; j < TestNativeViewHierarchy::kTotalViews; ++j)
+        if (root_views_[j] == *i)
+          break;
+      // EXPECT_LT/GT/GE() fails to compile with class-defined constants
+      // with gcc, with error
+      // "error: undefined reference to 'TestNativeViewHierarchy::kTotalViews'"
+      // so I forced to use EXPECT_TRUE() instead.
+      EXPECT_TRUE(TestNativeViewHierarchy::kTotalViews > j);
+    }
+  }
+
+  void CheckChangingHierarhy() {
+    size_t i;
+    for (i = 0; i < TestNativeViewHierarchy::kTotalViews; ++i) {
+      // TODO(georgey): use actual hierarchy changes to send notifications.
+      root_views_[i]->NotifyNativeViewHierarchyChanged(false,
+          host_->GetNativeView());
+      root_views_[i]->NotifyNativeViewHierarchyChanged(true,
+          host_->GetNativeView());
+    }
+    for (i = 0; i < TestNativeViewHierarchy::kTotalViews; ++i) {
+      ASSERT_EQ(static_cast<size_t>(2), test_views_[i]->notifications_.size());
+      EXPECT_FALSE(test_views_[i]->notifications_[0].attached);
+      EXPECT_EQ(host_->GetNativeView(),
+          test_views_[i]->notifications_[0].native_view);
+      EXPECT_EQ(root_views_[i], test_views_[i]->notifications_[0].root_view);
+      EXPECT_TRUE(test_views_[i]->notifications_[1].attached);
+      EXPECT_EQ(host_->GetNativeView(),
+          test_views_[i]->notifications_[1].native_view);
+      EXPECT_EQ(root_views_[i], test_views_[i]->notifications_[1].root_view);
+    }
+  }
+
+  views::NativeViewHost* native_host_;
+  views::Widget* host_;
+  views::Widget* windows_[TestNativeViewHierarchy::kTotalViews];
+  views::RootView* root_views_[TestNativeViewHierarchy::kTotalViews];
+  TestNativeViewHierarchy* test_views_[TestNativeViewHierarchy::kTotalViews];
+  ViewTest* view_test_;
+};
+
+TEST_F(ViewTest, ChangeNativeViewHierarchyFindRoots) {
+  // TODO(georgey): Fix the test for Linux
+#if defined(OS_WIN)
+  TestChangeNativeViewHierarchy test(this);
+  test.CheckEnumeratingRootViews();
+#endif
+}
+
+TEST_F(ViewTest, ChangeNativeViewHierarchyChangeHierarchy) {
+  // TODO(georgey): Fix the test for Linux
+#if defined(OS_WIN)
+  TestChangeNativeViewHierarchy test(this);
+  test.CheckChangingHierarhy();
+#endif
+}
+
diff --git a/views/widget/root_view.cc b/views/widget/root_view.cc
index 5d2b6e4..862d000 100644
--- a/views/widget/root_view.cc
+++ b/views/widget/root_view.cc
@@ -764,6 +764,11 @@ void RootView::SetFocusTraversableParentView(View* view) {
   focus_traversable_parent_view_ = view;
 }
 
+void RootView::NotifyNativeViewHierarchyChanged(bool attached,
+                                                gfx::NativeView native_view) {
+  PropagateNativeViewHierarchyChanged(attached, native_view, this);
+}
+
 // static
 View* RootView::FindSelectedViewForGroup(View* view) {
   if (view->IsGroupFocusTraversable() ||
diff --git a/views/widget/root_view.h b/views/widget/root_view.h
index dbb480f..d175c63 100644
--- a/views/widget/root_view.h
+++ b/views/widget/root_view.h
@@ -152,6 +152,10 @@ class RootView : public View,
   // Used to set the View parent after the view has been created.
   virtual void SetFocusTraversableParentView(View* view);
 
+  // Called when parent of the host changed.
+  void NotifyNativeViewHierarchyChanged(bool attached,
+                                        gfx::NativeView native_view);
+
   // Returns the name of this class: views/RootView
   virtual std::string GetClassName() const;
 
diff --git a/views/widget/widget.h b/views/widget/widget.h
index 8de4b6c..fa8111e 100644
--- a/views/widget/widget.h
+++ b/views/widget/widget.h
@@ -5,6 +5,7 @@
 #ifndef VIEWS_WIDGET_WIDGET_H_
 #define VIEWS_WIDGET_WIDGET_H_
 
+#include <vector>
 #include "app/gfx/native_widget_types.h"
 
 class ThemeProvider;
@@ -71,6 +72,11 @@ class Widget {
   // one is found. If a root view isn't found, null is returned.
   static RootView* FindRootView(gfx::NativeWindow native_window);
 
+  // Returns list of all root views for the native window and its
+  // children.
+  static void FindAllRootViews(gfx::NativeWindow native_window,
+                               std::vector<RootView*>* root_views);
+
   // Retrieve the Widget corresponding to the specified native_view, or NULL
   // if there is no such Widget.
   static Widget* GetWidgetFromNativeView(gfx::NativeView native_view);
diff --git a/views/widget/widget_gtk.cc b/views/widget/widget_gtk.cc
index a2d699f..e10bf60 100644
--- a/views/widget/widget_gtk.cc
+++ b/views/widget/widget_gtk.cc
@@ -1302,6 +1302,42 @@ RootView* Widget::FindRootView(GtkWindow* window) {
   return root_view;
 }
 
+static void AllRootViewsLocatorCallback(GtkWidget* widget,
+                                        gpointer root_view_p) {
+  std::set<RootView*>* root_views_set =
+      reinterpret_cast<std::set<RootView*>*>(root_view_p);
+  RootView *root_view = WidgetGtk::GetRootViewForWidget(widget);
+  if (!root_view && GTK_IS_CONTAINER(widget)) {
+    // gtk_container_foreach only iterates over children, not all descendants,
+    // so we have to recurse here to get all descendants.
+    gtk_container_foreach(GTK_CONTAINER(widget), AllRootViewsLocatorCallback,
+        root_view_p);
+  } else {
+    if (root_view)
+      root_views_set->insert(root_view);
+  }
+}
+
+// static
+void Widget::FindAllRootViews(GtkWindow* window,
+                              std::vector<RootView*>* root_views) {
+  RootView* root_view = WidgetGtk::GetRootViewForWidget(GTK_WIDGET(window));
+  if (root_view)
+    root_views->push_back(root_view);
+
+  std::set<RootView*> root_views_set;
+
+  // Enumerate all children and check if they have a RootView.
+  gtk_container_foreach(GTK_CONTAINER(window), AllRootViewsLocatorCallback,
+      reinterpret_cast<gpointer>(&root_views_set));
+  root_views->clear();
+  root_views->reserve(root_views_set.size());
+  for (std::set<RootView*>::iterator it = root_views_set.begin();
+      it != root_views_set.end();
+      ++it)
+    root_views->push_back(*it);
+}
+
 // static
 Widget* Widget::GetWidgetFromNativeView(gfx::NativeView native_view) {
   gpointer raw_widget = g_object_get_data(G_OBJECT(native_view), kWidgetKey);
diff --git a/views/widget/widget_win.cc b/views/widget/widget_win.cc
index 76f58e4..6f75506 100644
--- a/views/widget/widget_win.cc
+++ b/views/widget/widget_win.cc
@@ -1205,6 +1205,38 @@ RootView* Widget::FindRootView(HWND hwnd) {
   return root_view;
 }
 
+// Enumerate child windows as they could have RootView distinct from
+// the HWND's root view.
+BOOL CALLBACK EnumAllRootViewsChildProc(HWND hwnd, LPARAM l_param) {
+  RootView* root_view =
+      reinterpret_cast<RootView*>(GetProp(hwnd, kRootViewWindowProperty));
+  if (root_view) {
+    std::set<RootView*>* root_views_set =
+        reinterpret_cast<std::set<RootView*>*>(l_param);
+    root_views_set->insert(root_view);
+  }
+  return TRUE;  // Keep enumerating.
+}
+
+void Widget::FindAllRootViews(HWND window,
+                              std::vector<RootView*>* root_views) {
+  RootView* root_view =
+      reinterpret_cast<RootView*>(GetProp(window, kRootViewWindowProperty));
+  std::set<RootView*> root_views_set;
+  if (root_view)
+    root_views_set.insert(root_view);
+  // Enumerate all children and check if they have a RootView.
+  EnumChildWindows(window, EnumAllRootViewsChildProc,
+      reinterpret_cast<LPARAM>(&root_views_set));
+  root_views->clear();
+  root_views->reserve(root_views_set.size());
+  for (std::set<RootView*>::iterator it = root_views_set.begin();
+      it != root_views_set.end();
+      ++it)
+    root_views->push_back(*it);
+}
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // Widget, public:
 
diff --git a/views/window/dialog_client_view.cc b/views/window/dialog_client_view.cc
index 64c4677..b25c6bc 100644
--- a/views/window/dialog_client_view.cc
+++ b/views/window/dialog_client_view.cc
@@ -110,7 +110,9 @@ DialogClientView::DialogClientView(Window* owner, View* contents_view)
       cancel_button_(NULL),
       default_button_(NULL),
       extra_view_(NULL),
-      accepted_(false) {
+      accepted_(false),
+      listening_to_focus_(false),
+      saved_focus_manager_(NULL) {
   InitClass();
 }
 
@@ -236,6 +238,17 @@ void DialogClientView::CancelWindow() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+// DialogClientView, View overrides:
+
+void DialogClientView::NativeViewHierarchyChanged(bool attached,
+                                                  gfx::NativeView native_view,
+                                                  RootView* root_view) {
+  if (attached) {
+    UpdateFocusListener();
+  }
+}
+
+///////////////////////////////////////////////////////////////////////////////
 // DialogClientView, ClientView overrides:
 
 bool DialogClientView::CanClose() const {
@@ -251,10 +264,11 @@ bool DialogClientView::CanClose() const {
 }
 
 void DialogClientView::WindowClosing() {
-  FocusManager* focus_manager = GetFocusManager();
-  DCHECK(focus_manager);
-  if (focus_manager)
-     focus_manager->RemoveFocusChangeListener(this);
+  if (listening_to_focus_) {
+    DCHECK(saved_focus_manager_);
+    if (saved_focus_manager_)
+       saved_focus_manager_->RemoveFocusChangeListener(this);
+  }
   ClientView::WindowClosing();
 }
 
@@ -302,12 +316,7 @@ void DialogClientView::ViewHierarchyChanged(bool is_add, View* parent,
     ShowDialogButtons();
     ClientView::ViewHierarchyChanged(is_add, parent, child);
 
-    FocusManager* focus_manager = GetFocusManager();
-    // Listen for focus change events so we can update the default button.
-    DCHECK(focus_manager);  // bug #1291225: crash reports seem to indicate it
-                            // can be NULL.
-    if (focus_manager)
-      focus_manager->AddFocusChangeListener(this);
+    UpdateFocusListener();
 
     // The "extra view" must be created and installed after the contents view
     // has been inserted into the view hierarchy.
@@ -503,4 +512,27 @@ void DialogClientView::Close() {
   GetDialogDelegate()->OnClose();
 }
 
+void DialogClientView::UpdateFocusListener() {
+  FocusManager* focus_manager = GetFocusManager();
+  // Listen for focus change events so we can update the default button.
+  // focus_manager can be NULL when the dialog is created on un-shown view.
+  // We start listening for focus changes when the page is visible.
+  // Focus manager could also change if window host changes a parent.
+  if (listening_to_focus_) {
+    if (saved_focus_manager_ == focus_manager)
+      return;
+    DCHECK(saved_focus_manager_);
+    if (saved_focus_manager_)
+      saved_focus_manager_->RemoveFocusChangeListener(this);
+    listening_to_focus_ = false;
+  }
+  saved_focus_manager_ = focus_manager;
+  // Listen for focus change events so we can update the default button.
+  if (focus_manager) {
+    focus_manager->AddFocusChangeListener(this);
+    listening_to_focus_ = true;
+  }
+}
+
+
 }  // namespace views
diff --git a/views/window/dialog_client_view.h b/views/window/dialog_client_view.h
index 1c3d19f..d7a5fb2 100644
--- a/views/window/dialog_client_view.h
+++ b/views/window/dialog_client_view.h
@@ -50,6 +50,11 @@ class DialogClientView : public ClientView,
   NativeButton* ok_button() const { return ok_button_; }
   NativeButton* cancel_button() const { return cancel_button_; }
 
+  // Overridden from View:
+  virtual void NativeViewHierarchyChanged(bool attached,
+                                          gfx::NativeView native_view,
+                                          RootView* root_view);
+
   // Overridden from ClientView:
   virtual bool CanClose() const;
   virtual void WindowClosing();
@@ -98,6 +103,9 @@ class DialogClientView : public ClientView,
   // Closes the window.
   void Close();
 
+  // Updates focus listener.
+  void UpdateFocusListener();
+
   // The dialog buttons.
   NativeButton* ok_button_;
   NativeButton* cancel_button_;
@@ -114,6 +122,12 @@ class DialogClientView : public ClientView,
   // True if the window was Accepted by the user using the OK button.
   bool accepted_;
 
+  // true if focus listener is added.
+  bool listening_to_focus_;
+
+  // When ancestor gets changed focus manager gets changed as well.
+  FocusManager* saved_focus_manager_;
+
   // Static resource initialization
   static void InitClass();
   static gfx::Font* dialog_button_font_;