RemoteMacViews: Move NativeWindowProperty and AssociatedViews

Move Set/GetNativeWindowProperty to the browser-side
BridgedNativeWidgetHostImpl. Also, change from using a NSDictionary
attached to the NSWindow to just using a std::map -- these properties
do not actually need to be attached to the NSWindow.

Move Set/RemoveAssociationForView and ReorderChildViews to the
browser side, because that's where the views::View hierarchy is
stored. Compute the appropriate NSView->rank map and then send that
to the BridgedNativeWidgetImpl (in the future, this will be a
handle->rank map, when crossing process boundaries).

Bug: 859152
Change-Id: I142ec9eb180a81a9e456957bc9f42f32ab7de792
Reviewed-on: https://chromium-review.googlesource.com/1217450Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590343}

ui/views/cocoa/bridged_native_widget.h

@@ -59,9 +59,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
       public CocoaMouseCaptureDelegate,
       public BridgedNativeWidgetOwner {
  public:
-  // Contains NativeViewHost->gfx::NativeView associations.
-  using AssociatedViews = std::map<const views::View*, NSView*>;
-
   // Return the size that |window| will take for the given client area |size|,
   // based on its current style mask.
   static gfx::Size GetWindowSizeForClientSize(NSWindow* window,
@@ -108,10 +105,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   Widget::MoveLoopResult RunMoveLoop(const gfx::Vector2d& drag_offset);
   void EndMoveLoop();
 
-  // See views::Widget.
-  void SetNativeWindowProperty(const char* key, void* value);
-  void* GetNativeWindowProperty(const char* key) const;
-
   // Sets the cursor associated with the NSWindow. Retains |cursor|.
   void SetCursor(NSCursor* cursor);
 
@@ -155,12 +148,8 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   // Called by the window show animation when it completes and wants to destroy
   // itself.
   void OnShowAnimationComplete();
-
-  // Updates |associated_views_| on NativeViewHost::Attach()/Detach().
-  void SetAssociationForView(const views::View* view, NSView* native_view);
-  void ClearAssociationForView(const views::View* view);
-  // Sorts child NSViews according to NativeViewHosts order in views hierarchy.
-  void ReorderChildViews();
+  // Sort child NSViews according to their ranking in |rank|.
+  void SortSubviews(std::map<NSView*, int> rank);
 
   NativeWidgetMac* native_widget_mac() { return native_widget_mac_; }
   BridgedContentView* ns_view() { return bridged_view_; }
@@ -285,10 +274,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   void OnMouseCaptureLost() override;
   NSWindow* GetWindow() const override;
 
-  // Returns a properties dictionary associated with the NSWindow.
-  // Creates and attaches a new instance if not found.
-  NSMutableDictionary* GetWindowProperties() const;
-
   // BridgedNativeWidgetOwner:
   NSWindow* GetNSWindow() override;
   gfx::Vector2d GetChildWindowOffset() const override;
@@ -357,8 +342,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   // shadow needs to be invalidated when a frame is received for the new shape.
   bool invalidate_shadow_on_frame_swap_ = false;
 
-  AssociatedViews associated_views_;
-
   DISALLOW_COPY_AND_ASSIGN(BridgedNativeWidgetImpl);
 };
 

ui/views/cocoa/bridged_native_widget.mm

@@ -154,8 +154,6 @@ using NSViewComparatorValue = id;
 using NSViewComparatorValue = __kindof NSView*;
 #endif
 
-int kWindowPropertiesKey;
-
 // Returns true if the content_view is reparented.
 bool PositionWindowInNativeViewParent(NSView* content_view) {
   return [[content_view window] contentView] != content_view;
@@ -181,16 +179,6 @@ gfx::Size GetClientSizeForWindowSize(NSWindow* window,
   return gfx::Size([window contentRectForFrameRect:frame_rect].size);
 }
 
-void RankNSViews(View* view,
-                 const BridgedNativeWidgetImpl::AssociatedViews& hosts,
-                 RankMap* rank) {
-  auto it = hosts.find(view);
-  if (it != hosts.end())
-    rank->emplace(it->second, rank->size());
-  for (int i = 0; i < view->child_count(); ++i)
-    RankNSViews(view->child_at(i), hosts, rank);
-}
-
 NSComparisonResult SubviewSorter(NSViewComparatorValue lhs,
                                  NSViewComparatorValue rhs,
                                  void* rank_as_void) {
@@ -668,22 +656,6 @@ void BridgedNativeWidgetImpl::EndMoveLoop() {
   window_move_loop_.reset();
 }
 
-void BridgedNativeWidgetImpl::SetNativeWindowProperty(const char* name,
-                                                      void* value) {
-  NSString* key = [NSString stringWithUTF8String:name];
-  if (value) {
-    [GetWindowProperties() setObject:[NSValue valueWithPointer:value]
-                              forKey:key];
-  } else {
-    [GetWindowProperties() removeObjectForKey:key];
-  }
-}
-
-void* BridgedNativeWidgetImpl::GetNativeWindowProperty(const char* name) const {
-  NSString* key = [NSString stringWithUTF8String:name];
-  return [[GetWindowProperties() objectForKey:key] pointerValue];
-}
-
 void BridgedNativeWidgetImpl::SetCursor(NSCursor* cursor) {
   [window_delegate_ setCursor:cursor];
 }
@@ -909,28 +881,12 @@ void BridgedNativeWidgetImpl::InitCompositorView() {
   UpdateWindowGeometry();
 }
 
-void BridgedNativeWidgetImpl::SetAssociationForView(const View* view,
-                                                    NSView* native_view) {
-  DCHECK_EQ(0u, associated_views_.count(view));
-  associated_views_[view] = native_view;
-  native_widget_mac_->GetWidget()->ReorderNativeViews();
-}
-
-void BridgedNativeWidgetImpl::ClearAssociationForView(const View* view) {
-  auto it = associated_views_.find(view);
-  DCHECK(it != associated_views_.end());
-  associated_views_.erase(it);
-}
-
-void BridgedNativeWidgetImpl::ReorderChildViews() {
+void BridgedNativeWidgetImpl::SortSubviews(RankMap rank) {
   // Ignore layer manipulation during a Close(). This can be reached during the
   // orderOut: in Close(), which notifies visibility changes to Views.
   if (!bridged_view_)
     return;
 
-  RankMap rank;
-  Widget* widget = native_widget_mac_->GetWidget();
-  RankNSViews(widget->GetRootView(), associated_views_, &rank);
   // Unassociated NSViews should be ordered above associated ones. The exception
   // is the UI compositor's superview, which should always be on the very
   // bottom, so give it an explicit negative rank.
@@ -1312,15 +1268,4 @@ void BridgedNativeWidgetImpl::ShowAsModalSheet() {
          contextInfo:nullptr];
 }
 
-NSMutableDictionary* BridgedNativeWidgetImpl::GetWindowProperties() const {
-  NSMutableDictionary* properties = objc_getAssociatedObject(
-      window_, &kWindowPropertiesKey);
-  if (!properties) {
-    properties = [NSMutableDictionary dictionary];
-    objc_setAssociatedObject(window_, &kWindowPropertiesKey,
-                             properties, OBJC_ASSOCIATION_RETAIN);
-  }
-  return properties;
-}
-
 }  // namespace views

ui/views/cocoa/bridged_native_widget_host_impl.h

@@ -129,6 +129,17 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   // fullscreen or transitioning between fullscreen states.
   gfx::Rect GetRestoredBounds() const;
 
+  // Properties set and queried by views. Not actually native.
+  void SetNativeWindowProperty(const char* name, void* value);
+  void* GetNativeWindowProperty(const char* name) const;
+
+  // Updates |associated_views_| on NativeViewHost::Attach()/Detach().
+  void SetAssociationForView(const views::View* view, NSView* native_view);
+  void ClearAssociationForView(const views::View* view);
+
+  // Sorts child NSViews according to NativeViewHosts order in views hierarchy.
+  void ReorderChildViews();
+
   bool IsVisible() const { return is_visible_; }
   bool IsMiniaturized() const { return is_miniaturized_; }
   bool IsWindowKey() const { return is_window_key_; }
@@ -138,6 +149,7 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
   gfx::Vector2d GetBoundsOffsetForParent() const;
   void UpdateCompositorProperties();
   void DestroyCompositor();
+  void RankNSViewsRecursive(View* view, std::map<NSView*, int>* rank) const;
 
   // BridgedNativeWidgetHostHelper:
   NSView* GetNativeViewAccessible() override;
@@ -274,6 +286,12 @@ class VIEWS_EXPORT BridgedNativeWidgetHostImpl
 
   std::unique_ptr<ui::RecyclableCompositorMac> compositor_;
 
+  // Properties used by Set/GetNativeWindowProperty.
+  std::map<std::string, void*> native_window_properties_;
+
+  // Contains NativeViewHost->gfx::NativeView associations.
+  std::map<const views::View*, NSView*> associated_views_;
+
   DISALLOW_COPY_AND_ASSIGN(BridgedNativeWidgetHostImpl);
 };
 

ui/views/cocoa/bridged_native_widget_host_impl.mm

@@ -298,6 +298,60 @@ gfx::Rect BridgedNativeWidgetHostImpl::GetRestoredBounds() const {
   return window_bounds_in_screen_;
 }
 
+void BridgedNativeWidgetHostImpl::SetNativeWindowProperty(const char* name,
+                                                          void* value) {
+  if (value)
+    native_window_properties_[name] = value;
+  else
+    native_window_properties_.erase(name);
+}
+
+void* BridgedNativeWidgetHostImpl::GetNativeWindowProperty(
+    const char* name) const {
+  auto found = native_window_properties_.find(name);
+  if (found == native_window_properties_.end())
+    return nullptr;
+  return found->second;
+}
+
+void BridgedNativeWidgetHostImpl::SetAssociationForView(const View* view,
+                                                        NSView* native_view) {
+  // TODO(ccameron): For the case of out-of-process NSWindows, it will be
+  // necessary to:
+  // - migrate |native_view| to the new process (for web contents).
+  // - use a dummy parent window if |native_view| is perforce in this process.
+  if (bridge_impl_)
+    [bridge_impl_->ns_view() addSubview:native_view];
+
+  DCHECK_EQ(0u, associated_views_.count(view));
+  associated_views_[view] = native_view;
+  native_widget_mac_->GetWidget()->ReorderNativeViews();
+}
+
+void BridgedNativeWidgetHostImpl::ClearAssociationForView(const View* view) {
+  auto it = associated_views_.find(view);
+  DCHECK(it != associated_views_.end());
+  associated_views_.erase(it);
+}
+
+void BridgedNativeWidgetHostImpl::ReorderChildViews() {
+  std::map<NSView*, int> rank;
+  Widget* widget = native_widget_mac_->GetWidget();
+  RankNSViewsRecursive(widget->GetRootView(), &rank);
+  if (bridge_impl_)
+    bridge_impl_->SortSubviews(std::move(rank));
+}
+
+void BridgedNativeWidgetHostImpl::RankNSViewsRecursive(
+    View* view,
+    std::map<NSView*, int>* rank) const {
+  auto it = associated_views_.find(view);
+  if (it != associated_views_.end())
+    rank->emplace(it->second, rank->size());
+  for (int i = 0; i < view->child_count(); ++i)
+    RankNSViewsRecursive(view->child_at(i), rank);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BridgedNativeWidgetHostImpl, views::BridgedNativeWidgetHostHelper:
 

ui/views/controls/native/native_view_host_mac.mm

@@ -9,7 +9,7 @@
 #include "base/mac/foundation_util.h"
 #import "ui/accessibility/platform/ax_platform_node_mac.h"
 #import "ui/base/cocoa/accessibility_hostable.h"
-#import "ui/views/cocoa/bridged_native_widget.h"
+#import "ui/views/cocoa/bridged_native_widget_host_impl.h"
 #include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/widget/native_widget_mac.h"
 #include "ui/views/widget/widget.h"
@@ -93,12 +93,11 @@ void NativeViewHostMac::AttachNativeView() {
   }
 
   EnsureNativeViewHasNoChildWidgets(native_view_);
-  BridgedNativeWidgetImpl* bridge =
-      NativeWidgetMac::GetBridgeImplForNativeWindow(
+  BridgedNativeWidgetHostImpl* bridge_host =
+      NativeWidgetMac::GetBridgeHostImplForNativeWindow(
           host_->GetWidget()->GetNativeWindow());
-  DCHECK(bridge);
-  [bridge->ns_view() addSubview:native_view_];
-  bridge->SetAssociationForView(host_, native_view_);
+  DCHECK(bridge_host);
+  bridge_host->SetAssociationForView(host_, native_view_);
 }
 
 void NativeViewHostMac::NativeViewDetaching(bool destroyed) {
@@ -127,12 +126,12 @@ void NativeViewHostMac::NativeViewDetaching(bool destroyed) {
   }
 
   EnsureNativeViewHasNoChildWidgets(host_->native_view());
-  BridgedNativeWidgetImpl* bridge =
-      NativeWidgetMac::GetBridgeImplForNativeWindow(
+  BridgedNativeWidgetHostImpl* bridge_host =
+      NativeWidgetMac::GetBridgeHostImplForNativeWindow(
           host_->GetWidget()->GetNativeWindow());
   // BridgedNativeWidgetImpl can be null when Widget is closing.
-  if (bridge)
-    bridge->ClearAssociationForView(host_);
+  if (bridge_host)
+    bridge_host->ClearAssociationForView(host_);
 
   native_view_.reset();
 }

ui/views/widget/native_widget_mac.mm

@@ -216,8 +216,8 @@ const ui::Layer* NativeWidgetMac::GetLayer() const {
 }
 
 void NativeWidgetMac::ReorderNativeViews() {
-  if (bridge_impl())
-    bridge_impl()->ReorderChildViews();
+  if (bridge_host_)
+    bridge_host_->ReorderChildViews();
 }
 
 void NativeWidgetMac::ViewRemoved(View* view) {
@@ -228,13 +228,13 @@ void NativeWidgetMac::ViewRemoved(View* view) {
 }
 
 void NativeWidgetMac::SetNativeWindowProperty(const char* name, void* value) {
-  if (bridge_impl())
-    bridge_impl()->SetNativeWindowProperty(name, value);
+  if (bridge_host_)
+    bridge_host_->SetNativeWindowProperty(name, value);
 }
 
 void* NativeWidgetMac::GetNativeWindowProperty(const char* name) const {
-  if (bridge_impl())
-    return bridge_impl()->GetNativeWindowProperty(name);
+  if (bridge_host_)
+    return bridge_host_->GetNativeWindowProperty(name);
 
   return nullptr;
 }
@@ -703,25 +703,26 @@ NativeWidgetPrivate* NativeWidgetPrivate::GetNativeWidgetForNativeWindow(
 // static
 NativeWidgetPrivate* NativeWidgetPrivate::GetTopLevelNativeWidget(
     gfx::NativeView native_view) {
-  BridgedNativeWidgetImpl* bridge_impl =
-      NativeWidgetMac::GetBridgeImplForNativeWindow([native_view window]);
-  if (!bridge_impl)
+  BridgedNativeWidgetHostImpl* bridge_host =
+      NativeWidgetMac::GetBridgeHostImplForNativeWindow([native_view window]);
+  if (!bridge_host)
     return nullptr;
 
   NativeWidgetPrivate* ancestor =
-      bridge_impl->parent()
+      bridge_host->bridge_impl()->parent()
           ? GetTopLevelNativeWidget(
-                [bridge_impl->parent()->GetNSWindow() contentView])
+                [bridge_host->bridge_impl()->parent()->GetNSWindow()
+                        contentView])
           : nullptr;
-  return ancestor ? ancestor : bridge_impl->native_widget_mac();
+  return ancestor ? ancestor : bridge_host->native_widget_mac();
 }
 
 // static
 void NativeWidgetPrivate::GetAllChildWidgets(gfx::NativeView native_view,
                                              Widget::Widgets* children) {
-  BridgedNativeWidgetImpl* bridge_impl =
-      NativeWidgetMac::GetBridgeImplForNativeWindow([native_view window]);
-  if (!bridge_impl) {
+  BridgedNativeWidgetHostImpl* bridge_host =
+      NativeWidgetMac::GetBridgeHostImplForNativeWindow([native_view window]);
+  if (!bridge_host) {
     // The NSWindow is not itself a views::Widget, but it may have children that
     // are. Support returning Widgets that are parented to the NSWindow, except:
     // - Ignore requests for children of an NSView that is not a contentView.
@@ -744,23 +745,25 @@ void NativeWidgetPrivate::GetAllChildWidgets(gfx::NativeView native_view,
 
   // If |native_view| is a subview of the contentView, it will share an
   // NSWindow, but will itself be a native child of the Widget. That is, adding
-  // bridge_impl->..->GetWidget() to |children| would be adding the _parent_ of
+  // bridge_host->..->GetWidget() to |children| would be adding the _parent_ of
   // |native_view|, not the Widget for |native_view|. |native_view| doesn't have
   // a corresponding Widget of its own in this case (and so can't have Widget
   // children of its own on Mac).
-  if (bridge_impl->ns_view() != native_view)
+  if (bridge_host->bridge_impl()->ns_view() != native_view)
     return;
 
   // Code expects widget for |native_view| to be added to |children|.
-  if (bridge_impl->native_widget_mac()->GetWidget())
-    children->insert(bridge_impl->native_widget_mac()->GetWidget());
+  if (bridge_host->native_widget_mac()->GetWidget())
+    children->insert(bridge_host->native_widget_mac()->GetWidget());
 
   // When the NSWindow *is* a Widget, only consider child_windows(). I.e. do not
-  // look through -[NSWindow childWindows] as done for the (!bridge_impl) case
+  // look through -[NSWindow childWindows] as done for the (!bridge_host) case
   // above. -childWindows does not support hidden windows, and anything in there
   // which is not in child_windows() would have been added by AppKit.
-  for (BridgedNativeWidgetImpl* child : bridge_impl->child_windows())
+  for (BridgedNativeWidgetImpl* child :
+       bridge_host->bridge_impl()->child_windows()) {
     GetAllChildWidgets(child->ns_view(), children);
+  }
 }
 
 // static