views: add AnyWidgetObserver

A common problem when writing integration tests for Views UI is that
the involved UI is hidden behind an interface which hides the Widget,
since it is usually an implementation detail. Specifically, it is
common to have a function like so:

  static void MyDialog::Show(...);

which internally creates a Widget, initializes it, shows it, etc, and
invokes some callbacks or notifies some controller when the Widget is
done. However, this makes it difficult for tests that expect to touch
that UI to actually get ahold of it, which leads to various hacky
approaches, like adding globals to track the last Widget created by
MyDialog and similar. However, even this hack is considerably ugly
when the UI is instead shown via something like:

  MyController::WaitUntilReadyAndThenShow(...);

because the test can rarely tell when the UI is actually being
displayed, which results in ad-hoc things like spinning the RunLoop a
set number of times.

To achieve that, this change:
1) Adds a class AnyWidgetObserver, which can be used to observe events
   that happen on any Widget instance without needing to specifically
   register for that Widget instance;
2) Adds a class AnyWidgetObserverSingleton, which is what the
   AnyWidgetObservers actually register as observers of;
3) Has Widget notify AnyWidgetObserverSingleton of various interesting
   events;
4) Adds some tests for AnyWidgetObserver.

Bug: None
Change-Id: I36c9b3c59bcae3bbe889731720e797339e5675d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110812Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753332}

ui/views/BUILD.gn

@@ -247,6 +247,8 @@ jumbo_component("views") {
     "views_features.h",
     "views_switches.h",
     "views_touch_selection_controller_factory.h",
+    "widget/any_widget_observer.h",
+    "widget/any_widget_observer_singleton.h",
     "widget/drop_helper.h",
     "widget/native_widget.h",
     "widget/native_widget_delegate.h",
@@ -429,6 +431,8 @@ jumbo_component("views") {
     "views_delegate.cc",
     "views_features.cc",
     "views_switches.cc",
+    "widget/any_widget_observer.cc",
+    "widget/any_widget_observer_singleton.cc",
     "widget/drop_helper.cc",
     "widget/native_widget_private.cc",
     "widget/root_view.cc",
@@ -1111,6 +1115,7 @@ test("views_unittests") {
     "view_targeter_unittest.cc",
     "view_tracker_unittest.cc",
     "view_unittest.cc",
+    "widget/any_widget_observer_unittest.cc",
     "widget/native_widget_unittest.cc",
     "widget/root_view_unittest.cc",
     "widget/widget_unittest.cc",

ui/views/widget/any_widget_observer.cc

+// Copyright 2020 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.
+
+#include "ui/views/widget/any_widget_observer.h"
+#include "ui/views/widget/any_widget_observer_singleton.h"
+
+namespace views {
+
+AnyWidgetObserver::AnyWidgetObserver(AnyWidgetObserver::Passkey passkey)
+    : AnyWidgetObserver() {}
+AnyWidgetObserver::AnyWidgetObserver(test::AnyWidgetTestPasskey passkey)
+    : AnyWidgetObserver() {}
+
+AnyWidgetObserver::AnyWidgetObserver() {
+  internal::AnyWidgetObserverSingleton::GetInstance()->AddObserver(this);
+}
+
+AnyWidgetObserver::~AnyWidgetObserver() {
+  internal::AnyWidgetObserverSingleton::GetInstance()->RemoveObserver(this);
+}
+
+#define PROPAGATE_NOTIFICATION(method, callback)   \
+  void AnyWidgetObserver::method(Widget* widget) { \
+    if (callback)                                  \
+      callback.Run(widget);                        \
+  }
+
+PROPAGATE_NOTIFICATION(OnAnyWidgetInitialized, initialized_callback_)
+PROPAGATE_NOTIFICATION(OnAnyWidgetShown, shown_callback_)
+PROPAGATE_NOTIFICATION(OnAnyWidgetHidden, hidden_callback_)
+PROPAGATE_NOTIFICATION(OnAnyWidgetClosing, closing_callback_)
+
+#undef PROPAGATE_NOTIFICATION
+
+}  // namespace views

ui/views/widget/any_widget_observer.h

+// Copyright 2020 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.
+
+#ifndef UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_H_
+#define UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_H_
+
+#include "base/callback.h"
+#include "base/observer_list_types.h"
+#include "ui/views/views_export.h"
+
+namespace views {
+
+namespace internal {
+class AnyWidgetObserverSingleton;
+}
+
+namespace test {
+class AnyWidgetTestPasskey;
+}
+
+class Widget;
+
+// AnyWidgetObserver is used when you want to observe widget changes but don't
+// have an easy way to get a reference to the Widget in question, usually
+// because it is created behind a layer of abstraction that hides the Widget.
+//
+// This class should only be used as a last resort - if you find yourself
+// reaching for it in production code, it probably means some parts of your
+// system aren't coupled enough or your API boundaries are hiding too much. You
+// will need review from an owner of this class to add new uses of it, because
+// it requires a Passkey to construct it - see //docs/patterns/passkey.md for
+// details. Uses in tests can be done freely using
+// views::test::AnyWidgetTestPasskey.
+//
+// This class is useful when doing something like this:
+//
+//    RunLoop run_loop;
+//    AnyWidgetCallbackObserver observer(views::test::AnyWidgetTestPasskey{});
+//    Widget* widget;
+//    observer.set_shown_callback(
+//        base::BindLambdaForTesting([&](Widget* w) {
+//            if (w->GetName() == "MyWidget") {
+//                widget = w;
+//                run_loop.Quit();
+//            }
+//        }));
+//    ThingThatCreatesAndShowsWidget();
+//    run_loop.Run();
+//
+// with your widget getting the name "MyWidget" via InitParams::name.
+// TODO(ellyjones): Add Widget::SetDebugName and add a remark about that here.
+//
+// This allows you to create a test that is robust even if
+// ThingThatCreatesAndShowsWidget() later becomes asynchronous, takes a few
+// milliseconds, etc.
+class VIEWS_EXPORT AnyWidgetObserver : public base::CheckedObserver {
+ public:
+  using AnyWidgetCallback = base::RepeatingCallback<void(Widget*)>;
+
+  class Passkey {
+   private:
+    Passkey();
+
+    // Add friend classes here that are allowed to use AnyWidgetObserver in
+    // production code.
+  };
+
+  // Using this in production requires an AnyWidgetObserver::Passkey, which can
+  // only be constructed by a list of allowed friend classes...
+  explicit AnyWidgetObserver(Passkey passkey);
+
+  // ... but for tests or debugging, anyone can construct a
+  // views::test::AnyWidgetTestPasskey.
+  explicit AnyWidgetObserver(test::AnyWidgetTestPasskey passkey);
+
+  ~AnyWidgetObserver() override;
+
+  AnyWidgetObserver(const AnyWidgetObserver& other) = delete;
+  AnyWidgetObserver& operator=(const AnyWidgetObserver& other) = delete;
+
+  // This sets the callback for when the Widget has finished initialization and
+  // is ready to use.  This is the first point at which the widget is useable.
+  void set_initialized_callback(const AnyWidgetCallback& callback) {
+    initialized_callback_ = callback;
+  }
+
+  // These set the callbacks for when the backing native widget has just been
+  // asked to change visibility. Note that the native widget may or may not
+  // actually be drawn on the screen when these callbacks are run, because there
+  // are more layers of asynchronousness at the OS layer.
+  void set_shown_callback(const AnyWidgetCallback& callback) {
+    shown_callback_ = callback;
+  }
+  void set_hidden_callback(const AnyWidgetCallback& callback) {
+    hidden_callback_ = callback;
+  }
+
+  // This sets the callback for when the Widget has decided that it is about to
+  // close, but not yet begun to tear down the backing native Widget or the
+  // RootView. This is the last point at which the Widget is in a consistent,
+  // useable state.
+  void set_closing_callback(const AnyWidgetCallback& callback) {
+    closing_callback_ = callback;
+  }
+
+  // These two methods deliberately don't exist:
+  //   void set_created_callback(...)
+  //   void set_destroyed_callback(...)
+  // They would be called respectively too early and too late in the Widget's
+  // lifecycle for it to be usefully identified - at construction time the
+  // Widget has no properties or contents yet (and therefore can't be
+  // meaningfully identified as "your widget" for test purposes), and at
+  // destruction time the Widget's state is already partly destroyed by the
+  // closure process. Both methods are deliberately left out to avoid dangerous
+  // designs based on them.
+
+ private:
+  friend class internal::AnyWidgetObserverSingleton;
+
+  AnyWidgetObserver();
+
+  // Called from the singleton to propagate events to each AnyWidgetObserver.
+  void OnAnyWidgetInitialized(Widget* widget);
+  void OnAnyWidgetShown(Widget* widget);
+  void OnAnyWidgetHidden(Widget* widget);
+  void OnAnyWidgetClosing(Widget* widget);
+
+  AnyWidgetCallback initialized_callback_;
+  AnyWidgetCallback shown_callback_;
+  AnyWidgetCallback hidden_callback_;
+  AnyWidgetCallback closing_callback_;
+};
+
+namespace test {
+
+// A passkey class to allow tests to use AnyWidgetObserver without needing a
+// views owner signoff.
+class AnyWidgetTestPasskey {
+ public:
+  AnyWidgetTestPasskey() = default;
+};
+
+}  // namespace test
+
+}  // namespace views
+
+#endif  // UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_H_

ui/views/widget/any_widget_observer_singleton.cc

+// Copyright 2020 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.
+
+#include "ui/views/widget/any_widget_observer_singleton.h"
+#include "ui/views/widget/any_widget_observer.h"
+
+#include "base/no_destructor.h"
+
+namespace views {
+namespace internal {
+
+// static
+AnyWidgetObserverSingleton* AnyWidgetObserverSingleton::GetInstance() {
+  static base::NoDestructor<AnyWidgetObserverSingleton> observer;
+  return observer.get();
+}
+
+#define PROPAGATE_NOTIFICATION(method)                      \
+  void AnyWidgetObserverSingleton::method(Widget* widget) { \
+    for (AnyWidgetObserver & obs : observers_)              \
+      obs.method(widget);                                   \
+  }
+
+PROPAGATE_NOTIFICATION(OnAnyWidgetInitialized)
+PROPAGATE_NOTIFICATION(OnAnyWidgetShown)
+PROPAGATE_NOTIFICATION(OnAnyWidgetHidden)
+PROPAGATE_NOTIFICATION(OnAnyWidgetClosing)
+
+#undef PROPAGATE_NOTIFICATION
+
+void AnyWidgetObserverSingleton::AddObserver(AnyWidgetObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void AnyWidgetObserverSingleton::RemoveObserver(AnyWidgetObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+AnyWidgetObserverSingleton::AnyWidgetObserverSingleton() = default;
+AnyWidgetObserverSingleton::~AnyWidgetObserverSingleton() = default;
+
+}  // namespace internal
+}  // namespace views

ui/views/widget/any_widget_observer_singleton.h

+// Copyright 2020 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.
+
+#ifndef UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_SINGLETON_H_
+#define UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_SINGLETON_H_
+
+#include "base/no_destructor.h"
+#include "base/observer_list.h"
+
+namespace views {
+
+class AnyWidgetObserver;
+class Widget;
+
+namespace internal {
+
+// This is not the class you want - go look at AnyWidgetObserver.
+
+// This class serves as the "thing being observed" by AnyWidgetObservers,
+// since there is no relevant singleton for Widgets. Every Widget notifies the
+// singleton instance of this class of its creation, and it handles notifying
+// all AnyWidgetObservers of that.
+class AnyWidgetObserverSingleton {
+ public:
+  static AnyWidgetObserverSingleton* GetInstance();
+
+  void OnAnyWidgetInitialized(Widget* widget);
+  void OnAnyWidgetShown(Widget* widget);
+  void OnAnyWidgetHidden(Widget* widget);
+  void OnAnyWidgetClosing(Widget* widget);
+
+  void AddObserver(AnyWidgetObserver* observer);
+  void RemoveObserver(AnyWidgetObserver* observer);
+
+ private:
+  friend class base::NoDestructor<AnyWidgetObserverSingleton>;
+
+  AnyWidgetObserverSingleton();
+  ~AnyWidgetObserverSingleton();
+
+  base::ObserverList<AnyWidgetObserver> observers_;
+};
+
+}  // namespace internal
+}  // namespace views
+
+#endif  // UI_VIEWS_WIDGET_ANY_WIDGET_OBSERVER_SINGLETON_H_

ui/views/widget/any_widget_observer_unittest.cc

+// Copyright 2020 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.
+
+#include "ui/views/widget/any_widget_observer.h"
+
+#include "base/test/bind_test_util.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/views/test/native_widget_factory.h"
+#include "ui/views/test/widget_test.h"
+
+namespace {
+
+using views::Widget;
+
+using AnyWidgetObserverTest = views::test::WidgetTest;
+
+TEST_F(AnyWidgetObserverTest, ObservesInitialize) {
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+
+  bool initialized = false;
+
+  observer.set_initialized_callback(
+      base::BindLambdaForTesting([&](Widget*) { initialized = true; }));
+
+  EXPECT_FALSE(initialized);
+  WidgetAutoclosePtr w0(WidgetTest::CreateTopLevelPlatformWidget());
+  EXPECT_TRUE(initialized);
+}
+
+TEST_F(AnyWidgetObserverTest, ObservesClose) {
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+
+  bool closing = false;
+
+  observer.set_closing_callback(
+      base::BindLambdaForTesting([&](Widget*) { closing = true; }));
+
+  EXPECT_FALSE(closing);
+  { WidgetAutoclosePtr w0(WidgetTest::CreateTopLevelPlatformWidget()); }
+  EXPECT_TRUE(closing);
+}
+
+TEST_F(AnyWidgetObserverTest, ObservesShow) {
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+
+  bool shown = false;
+
+  observer.set_shown_callback(
+      base::BindLambdaForTesting([&](Widget*) { shown = true; }));
+
+  EXPECT_FALSE(shown);
+  WidgetAutoclosePtr w0(WidgetTest::CreateTopLevelPlatformWidget());
+  w0->Show();
+  EXPECT_TRUE(shown);
+}
+
+TEST_F(AnyWidgetObserverTest, ObservesHide) {
+  views::AnyWidgetObserver observer(views::test::AnyWidgetTestPasskey{});
+
+  bool hidden = false;
+
+  observer.set_hidden_callback(
+      base::BindLambdaForTesting([&](Widget*) { hidden = true; }));
+
+  EXPECT_FALSE(hidden);
+  WidgetAutoclosePtr w0(WidgetTest::CreateTopLevelPlatformWidget());
+  w0->Hide();
+  EXPECT_TRUE(hidden);
+}
+
+}  // namespace

ui/views/widget/widget.cc

@@ -31,6 +31,7 @@
 #include "ui/views/focus/focus_manager_factory.h"
 #include "ui/views/focus/widget_focus_manager.h"
 #include "ui/views/views_delegate.h"
+#include "ui/views/widget/any_widget_observer_singleton.h"
 #include "ui/views/widget/native_widget_private.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/tooltip_manager.h"
@@ -379,6 +380,9 @@ void Widget::Init(InitParams params) {
 
   if (delegate)
     delegate->OnWidgetInitialized();
+
+  internal::AnyWidgetObserverSingleton::GetInstance()->OnAnyWidgetInitialized(
+      this);
 }
 
 void Widget::ShowEmojiPanel() {
@@ -598,6 +602,8 @@ void Widget::CloseWithReason(ClosedReason closed_reason) {
   for (WidgetObserver& observer : observers_)
     observer.OnWidgetClosing(this);
 
+  internal::AnyWidgetObserverSingleton::GetInstance()->OnAnyWidgetClosing(this);
+
   if (widget_delegate_)
     widget_delegate_->WindowWillClose();
 
@@ -611,6 +617,7 @@ void Widget::Close() {
 void Widget::CloseNow() {
   for (WidgetObserver& observer : observers_)
     observer.OnWidgetClosing(this);
+  internal::AnyWidgetObserverSingleton::GetInstance()->OnAnyWidgetClosing(this);
   native_widget_->CloseNow();
 }
 
@@ -643,10 +650,12 @@ void Widget::Show() {
   } else {
     native_widget_->Show(preferred_show_state, gfx::Rect());
   }
+  internal::AnyWidgetObserverSingleton::GetInstance()->OnAnyWidgetShown(this);
 }
 
 void Widget::Hide() {
   native_widget_->Hide();
+  internal::AnyWidgetObserverSingleton::GetInstance()->OnAnyWidgetHidden(this);
 }
 
 void Widget::ShowInactive() {