Revert "RemoteMacViews: Move DialogObserver to browser side"

This reverts commit 8e335edf.

Reason for revert:
Speculative revert for test failures on mac bots:
https://chromium-review.googlesource.com/c/chromium/src/+/1175051

I'll reland this if this is not relevant.

Original change's description:
> RemoteMacViews: Move DialogObserver to browser side
> 
> Move these from BridgedNativeWidget to BridgedNativeWidgetHostImpl.
> 
> Also move window BridgedNativeWidget::OnWindowWillClose callbacks
> from directly referencing NativeWidgetHost to going through the
> BridgedNativeWidgetHost interface (since they are responsible for
> removing BridgedNativeWidgetHostImpl as an observer now).
> 
> Separate out the interface that to BridgedNativeWidget that will be
> turned into a mojo interface and name it BridgedNativeWidgetPublic.
> These will be renamed to BridgedNativeWidgetImpl and
> BridgedNativeWidget in a search-and-replace patch soon.
> 
> Change-Id: I523aa8b3670b9432bde434af77ffd0e18a50f00e
> Reviewed-on: https://chromium-review.googlesource.com/1175238
> Commit-Queue: ccameron <ccameron@chromium.org>
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583383}

TBR=ellyjones@chromium.org,ccameron@chromium.org

Change-Id: Ib4156fee617d8b80238a575e5d5524285a2a856e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/1177021Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583502}

ui/views/cocoa/bridged_native_widget.h

@@ -22,6 +22,7 @@
 #import "ui/views/focus/focus_manager.h"
 #include "ui/views/views_export.h"
 #include "ui/views/widget/widget.h"
+#include "ui/views/window/dialog_observer.h"
 
 @class BridgedContentView;
 @class ModalShowAnimationWithLayer;
@@ -39,32 +40,14 @@ class DragDropClientMac;
 class NativeWidgetMac;
 class View;
 
-// The interface through which a NativeWidgetMac may interact with an NSWindow
-// in another process.
-// TODO(ccameron): Rename this to BridgedNativeWidget, and rename
-// BridgedNativeWidget to BridgedNativeWidgetImpl.
-class VIEWS_EXPORT BridgedNativeWidgetPublic {
- public:
-  // Initialize the view to display compositor output. This will send the
-  // current visibility and dimensions (and any future updates) to the
-  // BridgedNativeWidgetHost.
-  virtual void InitCompositorView() = 0;
-
-  // Specify the content to draw in the NSView.
-  virtual void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) = 0;
-
-  // Clear the touchbar.
-  virtual void ClearTouchBar() = 0;
-};
-
 // A bridge to an NSWindow managed by an instance of NativeWidgetMac or
 // DesktopNativeWidgetMac. Serves as a helper class to bridge requests from the
 // NativeWidgetMac to the Cocoa window. Behaves a bit like an aura::Window.
 class VIEWS_EXPORT BridgedNativeWidget
-    : public BridgedNativeWidgetPublic,
-      public ui::CATransactionCoordinator::PreCommitObserver,
+    : public ui::CATransactionCoordinator::PreCommitObserver,
       public CocoaMouseCaptureDelegate,
-      public BridgedNativeWidgetOwner {
+      public BridgedNativeWidgetOwner,
+      public DialogObserver {
  public:
   // Contains NativeViewHost->gfx::NativeView associations.
   using AssociatedViews = std::map<const views::View*, NSView*>;
@@ -98,6 +81,9 @@ class VIEWS_EXPORT BridgedNativeWidget
   // Initialize the bridge (after the NSWindow has been created).
   void Init(const Widget::InitParams& params);
 
+  // Invoked at the end of Widget::Init().
+  void OnWidgetInitDone();
+
   // Changes the bounds of the window and the hosted layer if present. The
   // origin is a location in screen coordinates except for "child" windows,
   // which are positioned relative to their parent(). SetBounds() considers a
@@ -234,10 +220,16 @@ class VIEWS_EXPORT BridgedNativeWidget
   bool ShouldWaitInPreCommit() override;
   base::TimeDelta PreCommitTimeout() override;
 
-  // views::BridgedNativeWidgetPublic:
-  void InitCompositorView() override;
-  void SetCALayerParams(const gfx::CALayerParams& ca_layer_params) override;
-  void ClearTouchBar() override;
+  // views::BridgedNativeWidget:
+  // TODO(ccameron): Rename BridgedNativeWidget to BridgedNativeWidgetImpl, and
+  // make these methods be exposed via the BridgedNativeWidget interface.
+  // Initialize the view to display compositor output. This will send the
+  // current visibility and dimensions (and any future updates) to the
+  // BridgedNativeWidgetHost.
+  void InitCompositorView();
+
+  // Specify the content to draw in the NSView.
+  void SetCALayerParams(const gfx::CALayerParams& ca_layer_params);
 
   // TODO(ccameron): This method exists temporarily as we move all direct access
   // of TextInputClient out of BridgedContentView.
@@ -281,6 +273,9 @@ class VIEWS_EXPORT BridgedNativeWidget
   bool IsVisibleParent() const override;
   void RemoveChildWindow(BridgedNativeWidget* child) override;
 
+  // DialogObserver:
+  void OnDialogModelChanged() override;
+
   // Set |layer()| to be visible or not visible based on |window_visible_|. If
   // the layer is not visible, then lock the compositor, so we don't draw any
   // new frames.

ui/views/cocoa/bridged_native_widget.mm

@@ -40,6 +40,7 @@
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_aura_utils.h"
 #include "ui/views/widget/widget_delegate.h"
+#include "ui/views/window/dialog_delegate.h"
 
 namespace {
 constexpr auto kUIPaintTimeout = base::TimeDelta::FromSeconds(5);
@@ -375,6 +376,13 @@ void BridgedNativeWidget::Init(const Widget::InitParams& params) {
   tooltip_manager_.reset(new TooltipManagerMac(this));
 }
 
+void BridgedNativeWidget::OnWidgetInitDone() {
+  DialogDelegate* dialog =
+      native_widget_mac_->GetWidget()->widget_delegate()->AsDialogDelegate();
+  if (dialog)
+    dialog->AddObserver(this);
+}
+
 void BridgedNativeWidget::SetBounds(const gfx::Rect& new_bounds) {
   Widget* widget = native_widget_mac_->GetWidget();
   // -[NSWindow contentMinSize] is only checked by Cocoa for user-initiated
@@ -614,7 +622,10 @@ void BridgedNativeWidget::SetCursor(NSCursor* cursor) {
 }
 
 void BridgedNativeWidget::OnWindowWillClose() {
-  host_->OnWindowWillClose();
+  Widget* widget = native_widget_mac_->GetWidget();
+  if (DialogDelegate* dialog = widget->widget_delegate()->AsDialogDelegate())
+    dialog->RemoveObserver(this);
+  native_widget_mac_->WindowDestroying();
 
   // Ensure BridgedNativeWidget does not have capture, otherwise
   // OnMouseCaptureLost() may reference a deleted |native_widget_mac_| when
@@ -634,7 +645,7 @@ void BridgedNativeWidget::OnWindowWillClose() {
   DCHECK(!show_animation_);
 
   [window_ setDelegate:nil];
-  host_->OnWindowHasClosed();
+  native_widget_mac_->WindowDestroyed();
   // Note: |this| is deleted here.
 }
 
@@ -1023,13 +1034,6 @@ void BridgedNativeWidget::SetCALayerParams(
   }
 }
 
-void BridgedNativeWidget::ClearTouchBar() {
-  if (@available(macOS 10.12.2, *)) {
-    if ([bridged_view_ respondsToSelector:@selector(setTouchBar:)])
-      [bridged_view_ setTouchBar:nil];
-  }
-}
-
 void BridgedNativeWidget::SetTextInputClient(
     ui::TextInputClient* text_input_client) {
   [bridged_view_ setTextInputClient:text_input_client];
@@ -1063,6 +1067,18 @@ void BridgedNativeWidget::RemoveChildWindow(BridgedNativeWidget* child) {
   [window_ removeChildWindow:child->window_];
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// BridgedNativeWidget, DialogObserver:
+
+void BridgedNativeWidget::OnDialogModelChanged() {
+  // Note it's only necessary to clear the TouchBar. If the OS needs it again,
+  // a new one will be created.
+  if (@available(macOS 10.12.2, *)) {
+    if ([bridged_view_ respondsToSelector:@selector(setTouchBar:)])
+      [bridged_view_ setTouchBar:nil];
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidget, private:
 

ui/views/cocoa/bridged_native_widget_host.h

@@ -56,12 +56,6 @@ class VIEWS_EXPORT BridgedNativeWidgetHost {
                          bool* found_word,
                          gfx::DecoratedText* decorated_word,
                          gfx::Point* baseline_point) = 0;
-
-  // Called before the NSWindow is closed and destroyed.
-  virtual void OnWindowWillClose() = 0;
-
-  // Called after the NSWindow has been closed and destroyed.
-  virtual void OnWindowHasClosed() = 0;
 };
 
 }  // namespace views

ui/views/cocoa/bridged_native_widget_host_impl.h

@@ -15,7 +15,6 @@
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/views_export.h"
 #include "ui/views/widget/widget.h"
-#include "ui/views/window/dialog_observer.h"
 
 namespace ui {
 class RecyclableCompositorMac;
@@ -24,7 +23,6 @@ class RecyclableCompositorMac;
 namespace views {
 
 class BridgedNativeWidget;
-class BridgedNativeWidgetPublic;
 class NativeWidgetMac;
 
 // The portion of NativeWidgetMac that lives in the browser process. This
@@ -32,7 +30,6 @@ class NativeWidgetMac;
 // APIs, and which may live in an app shim process.
 class VIEWS_EXPORT BridgedNativeWidgetHostImpl
     : public BridgedNativeWidgetHost,
-      public DialogObserver,
       public FocusChangeListener,
       public ui::internal::InputMethodDelegate,
       public ui::LayerDelegate,
@@ -46,8 +43,7 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // Provide direct access to the BridgedNativeWidget that this is hosting.
   // TODO(ccameron): Remove all accesses to this member, and replace them
   // with methods that may be sent across processes.
-  BridgedNativeWidget* bridge_impl() const { return bridge_impl_.get(); }
-  BridgedNativeWidgetPublic* bridge() const;
+  BridgedNativeWidget* bridge() const { return bridge_.get(); }
 
   // Set the root view (set during initialization and un-set during teardown).
   void SetRootView(views::View* root_view);
@@ -59,9 +55,6 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // This does NOT take ownership of |focus_manager|.
   void SetFocusManager(FocusManager* focus_manager);
 
-  // Called when the owning Widget's Init method has completed.
-  void OnWidgetInitDone();
-
   // See widget.h for documentation.
   ui::InputMethod* GetInputMethod();
 
@@ -86,11 +79,6 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
                  bool* found_word,
                  gfx::DecoratedText* decorated_word,
                  gfx::Point* baseline_point) override;
-  void OnWindowWillClose() override;
-  void OnWindowHasClosed() override;
-
-  // DialogObserver:
-  void OnDialogModelChanged() override;
 
   // FocusChangeListener:
   void OnWillChangeFocus(View* focused_before, View* focused_now) override;
@@ -114,7 +102,7 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // TODO(ccameron): Rather than instantiate a BridgedNativeWidget here,
   // we will instantiate a mojo BridgedNativeWidget interface to a Cocoa
   // instance that may be in another process.
-  std::unique_ptr<BridgedNativeWidget> bridge_impl_;
+  std::unique_ptr<BridgedNativeWidget> bridge_;
 
   std::unique_ptr<ui::InputMethod> input_method_;
   FocusManager* focus_manager_ = nullptr;  // Weak. Owned by our Widget.

ui/views/cocoa/bridged_native_widget_host_impl.mm

@@ -12,8 +12,6 @@
 #include "ui/views/cocoa/bridged_native_widget.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/native_widget_mac.h"
-#include "ui/views/widget/widget_delegate.h"
-#include "ui/views/window/dialog_delegate.h"
 #include "ui/views/word_lookup_client.h"
 
 namespace views {
@@ -21,27 +19,23 @@ namespace views {
 BridgedNativeWidgetHostImpl::BridgedNativeWidgetHostImpl(
     NativeWidgetMac* parent)
     : native_widget_mac_(parent),
-      bridge_impl_(new BridgedNativeWidget(this, parent)) {}
+      bridge_(new BridgedNativeWidget(this, parent)) {}
 
 BridgedNativeWidgetHostImpl::~BridgedNativeWidgetHostImpl() {
   // Destroy the bridge first to prevent any calls back into this during
   // destruction.
   // TODO(ccameron): When all communication from |bridge_| to this goes through
   // the BridgedNativeWidgetHost, this can be replaced with closing that pipe.
-  bridge_impl_.reset();
+  bridge_.reset();
   SetFocusManager(nullptr);
   DestroyCompositor();
 }
 
-BridgedNativeWidgetPublic* BridgedNativeWidgetHostImpl::bridge() const {
-  return bridge_impl_.get();
-}
-
 void BridgedNativeWidgetHostImpl::SetRootView(views::View* root_view) {
   root_view_ = root_view;
   // TODO(ccameron): The BridgedNativeWidget should not need to know its root
   // view.
-  bridge_impl_->SetRootView(root_view);
+  bridge_->SetRootView(root_view);
 }
 
 void BridgedNativeWidgetHostImpl::CreateCompositor(
@@ -75,7 +69,7 @@ void BridgedNativeWidgetHostImpl::CreateCompositor(
   // The compositor is initially locked (prevented from producing frames), and
   // is only unlocked when the BridgedNativeWidget calls back via
   // SetCompositorVisibility.
-  bridge()->InitCompositorView();
+  bridge_->InitCompositorView();
 }
 
 void BridgedNativeWidgetHostImpl::DestroyCompositor() {
@@ -117,12 +111,6 @@ void BridgedNativeWidgetHostImpl::SetFocusManager(FocusManager* focus_manager) {
     OnDidChangeFocus(nullptr, new_focus);
 }
 
-void BridgedNativeWidgetHostImpl::OnWidgetInitDone() {
-  Widget* widget = native_widget_mac_->GetWidget();
-  if (DialogDelegate* dialog = widget->widget_delegate()->AsDialogDelegate())
-    dialog->AddObserver(this);
-}
-
 ui::InputMethod* BridgedNativeWidgetHostImpl::GetInputMethod() {
   if (!input_method_) {
     input_method_ = ui::CreateInputMethod(this, gfx::kNullAcceleratedWidget);
@@ -240,28 +228,8 @@ void BridgedNativeWidgetHostImpl::GetWordAt(
   *found_word = true;
 }
 
-void BridgedNativeWidgetHostImpl::OnWindowWillClose() {
-  Widget* widget = native_widget_mac_->GetWidget();
-  if (DialogDelegate* dialog = widget->widget_delegate()->AsDialogDelegate())
-    dialog->RemoveObserver(this);
-  native_widget_mac_->WindowDestroying();
-}
-
-void BridgedNativeWidgetHostImpl::OnWindowHasClosed() {
-  native_widget_mac_->WindowDestroyed();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// BridgedNativeWidgetHostImpl, DialogObserver:
-
-void BridgedNativeWidgetHostImpl::OnDialogModelChanged() {
-  // Note it's only necessary to clear the TouchBar. If the OS needs it again,
-  // a new one will be created.
-  bridge()->ClearTouchBar();
-}
-
 ////////////////////////////////////////////////////////////////////////////////
-// BridgedNativeWidgetHostImpl, FocusChangeListener:
+// BridgedNativeWidget, FocusChangeListener:
 
 void BridgedNativeWidgetHostImpl::OnWillChangeFocus(View* focused_before,
                                                     View* focused_now) {}
@@ -275,7 +243,7 @@ void BridgedNativeWidgetHostImpl::OnDidChangeFocus(View* focused_before,
     // Sanity check: When focus moves away from the widget (i.e. |focused_now|
     // is nil), then the textInputClient will be cleared.
     DCHECK(!!focused_now || !input_client);
-    bridge_impl_->SetTextInputClient(input_client);
+    bridge_->SetTextInputClient(input_client);
   }
 }
 
@@ -314,7 +282,7 @@ void BridgedNativeWidgetHostImpl::AcceleratedWidgetCALayerParamsUpdated() {
   const gfx::CALayerParams* ca_layer_params =
       compositor_->widget()->GetCALayerParams();
   if (ca_layer_params)
-    bridge()->SetCALayerParams(*ca_layer_params);
+    bridge_->SetCALayerParams(*ca_layer_params);
 }
 
 }  // namespace views

ui/views/widget/native_widget_mac.mm

@@ -152,7 +152,7 @@ void NativeWidgetMac::InitNativeWidget(const Widget::InitParams& params) {
 
 void NativeWidgetMac::OnWidgetInitDone() {
   OnSizeConstraintsChanged();
-  bridge_host_->OnWidgetInitDone();
+  bridge()->OnWidgetInitDone();
 }
 
 NonClientFrameView* NativeWidgetMac::CreateNonClientFrameView() {
@@ -664,7 +664,7 @@ NativeWidgetMacNSWindow* NativeWidgetMac::CreateNSWindow(
 }
 
 BridgedNativeWidget* NativeWidgetMac::bridge() const {
-  return bridge_host_ ? bridge_host_->bridge_impl() : nullptr;
+  return bridge_host_ ? bridge_host_->bridge() : nullptr;
 }
 
 ////////////////////////////////////////////////////////////////////////////////