RemoteCocoa: Implement NativeWidgetMac::ReorderNativeViews on mojo

This fixes a bug where docked DevTools in PWAs doesn't work (and would
affect anything that attempts to put two WebContents in a PWA).

This function was never implemented because it involved passing a
map of NSView*s over mojo. Since various refactorings, we now have a
ScopedNSViewIdMapping which helps with this.

Starting at the bottom of the change's stack and working to the top:
* Change NativeWidgetNSWindowBridge::SortSubviews to take a
  std::vector of NSView ids, instead of a std::map ranking NSView*s.
  In the function, create the std::map ranking from the std::vector.
* Change the caller, NativeWidgetMacNSWindowHost::ReorderChildViews.
  to build a std::vector of NSView*s, and then from that create a
  std::vector of NSView ids.
  * If the NSView*s do not have an id (that is, if they are not a
    WebContentsView, which only happens in tests), then create a
    temporary remote_cocoa::ScopedNSViewIdMapping to create an id.
* Rename NativeWidgetMacNSWindowHost's SetAssociationForView,
  ClearAssociationForView, and associated_views_ to reflect that the
  "associated views" are a map from views::NativeViewHosts (a subclass
  of views::View) and NSView*s.

Then there is an iffy part of the change
* Allow looking up an NSView's id (the uint64 that is sent over mojo
  to refer to the NSView) from the NSView.
* Arguably, we should change gfx::NativeView to also carry an id
  with it. But that is a too-large-scope change for now.

Bug: 991598, 1017446
Change-Id: I2371d1716f98c2e6223a5db0272984c98c6fa4db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885950
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710365}

components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h

@@ -152,8 +152,6 @@ class REMOTE_COCOA_APP_SHIM_EXPORT NativeWidgetNSWindowBridge
   // Called by the window show animation when it completes and wants to destroy
   // itself.
   void OnShowAnimationComplete();
-  // Sort child NSViews according to their ranking in |rank|.
-  void SortSubviews(std::map<NSView*, int> rank);
 
   BridgedContentView* ns_view() { return bridged_view_; }
   NativeWidgetNSWindowHost* host() { return host_; }
@@ -237,6 +235,8 @@ class REMOTE_COCOA_APP_SHIM_EXPORT NativeWidgetNSWindowBridge
   void SetWindowTitle(const base::string16& title) override;
   void SetIgnoresMouseEvents(bool ignores_mouse_events) override;
   void MakeFirstResponder() override;
+  void SortSubviews(
+      const std::vector<uint64_t>& associated_subview_ids) override;
   void ClearTouchBar() override;
   void UpdateTooltip() override;
   void AcquireCapture() override;

components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm

@@ -1060,11 +1060,17 @@ void NativeWidgetNSWindowBridge::InitCompositorView() {
   UpdateWindowGeometry();
 }
 
-void NativeWidgetNSWindowBridge::SortSubviews(RankMap rank) {
+void NativeWidgetNSWindowBridge::SortSubviews(
+    const std::vector<uint64_t>& attached_subview_ids) {
   // 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;
+  for (uint64_t subview_id : attached_subview_ids) {
+    if (NSView* subview = remote_cocoa::GetNSViewFromId(subview_id))
+      rank[subview] = rank.size() + 1;
+  }
   [bridged_view_ sortSubviewsUsingFunction:&SubviewSorter context:&rank];
 }
 

components/remote_cocoa/app_shim/ns_view_ids.h

@@ -17,6 +17,10 @@ namespace remote_cocoa {
 // Return an NSView given its id. This is to be called in an app shim process.
 NSView* REMOTE_COCOA_APP_SHIM_EXPORT GetNSViewFromId(uint64_t ns_view_id);
 
+// Return an id given an NSView. Returns 0 if |ns_view| does not have an
+// associated id.
+uint64_t REMOTE_COCOA_APP_SHIM_EXPORT GetIdFromNSView(NSView* ns_view);
+
 // A scoped mapping from |ns_view_id| to |view|. While this object exists,
 // GetNSViewFromId will return |view| when queried with |ns_view_id|. This
 // is to be instantiated in the app shim process.
@@ -26,6 +30,7 @@ class REMOTE_COCOA_APP_SHIM_EXPORT ScopedNSViewIdMapping {
   ~ScopedNSViewIdMapping();
 
  private:
+  NSView* const ns_view_;
   const uint64_t ns_view_id_;
   DISALLOW_COPY_AND_ASSIGN(ScopedNSViewIdMapping);
 };

components/remote_cocoa/app_shim/ns_view_ids.mm

@@ -13,30 +13,56 @@
 
 namespace remote_cocoa {
 
-std::map<uint64_t, NSView*>& GetNSViewIdMap() {
+std::map<uint64_t, NSView*>& GetIdToNSViewMap() {
   static base::NoDestructor<std::map<uint64_t, NSView*>> instance;
   return *instance;
 }
 
+std::map<NSView*, uint64_t>& GetNSViewToIdMap() {
+  static base::NoDestructor<std::map<NSView*, uint64_t>> instance;
+  return *instance;
+}
+
 NSView* GetNSViewFromId(uint64_t ns_view_id) {
-  auto& view_map = GetNSViewIdMap();
-  auto found = view_map.find(ns_view_id);
-  if (found == view_map.end())
+  auto& id_to_view_map = GetIdToNSViewMap();
+  auto found = id_to_view_map.find(ns_view_id);
+  if (found == id_to_view_map.end())
     return nil;
   return found->second;
 }
 
+uint64_t GetIdFromNSView(NSView* ns_view) {
+  auto& view_to_id_map = GetNSViewToIdMap();
+  auto found = view_to_id_map.find(ns_view);
+  if (found == view_to_id_map.end())
+    return 0;
+  return found->second;
+}
+
 ScopedNSViewIdMapping::ScopedNSViewIdMapping(uint64_t ns_view_id, NSView* view)
-    : ns_view_id_(ns_view_id) {
-  auto result = GetNSViewIdMap().insert(std::make_pair(ns_view_id, view));
-  DCHECK(result.second);
+    : ns_view_(view), ns_view_id_(ns_view_id) {
+  DCHECK(ns_view_id_);
+  {
+    auto result = GetIdToNSViewMap().insert(std::make_pair(ns_view_id, view));
+    DCHECK(result.second);
+  }
+  {
+    auto result = GetNSViewToIdMap().insert(std::make_pair(view, ns_view_id));
+    DCHECK(result.second);
+  }
 }
 
 ScopedNSViewIdMapping::~ScopedNSViewIdMapping() {
-  auto& view_map = GetNSViewIdMap();
-  auto found = view_map.find(ns_view_id_);
-  DCHECK(found != view_map.end());
-  view_map.erase(found);
+  {
+    auto found = GetIdToNSViewMap().find(ns_view_id_);
+    DCHECK(found != GetIdToNSViewMap().end());
+    GetIdToNSViewMap().erase(found);
+  }
+  {
+    auto found = GetNSViewToIdMap().find(ns_view_);
+    DCHECK(found != GetNSViewToIdMap().end());
+    GetNSViewToIdMap().erase(found);
+  }
 }
 
 }  // namespace remote_cocoa

components/remote_cocoa/common/native_widget_ns_window.mojom

@@ -202,6 +202,11 @@ interface NativeWidgetNSWindow {
   // Make the content view be the first responder for the NSWindow.
   MakeFirstResponder();
 
+  // Sort the subviews of the content view of this window. The sorted ids of
+  // the NSViews attached by NativeViewHost::Attach are listed in
+  // |attached_subview_ids|.
+  SortSubviews(array<uint64> attached_subview_ids);
+
   // Clear the touchbar.
   ClearTouchBar();
 

ui/views/cocoa/native_widget_mac_ns_window_host.h

@@ -210,9 +210,10 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
   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);
+  // Updates |attached_native_view_host_views_| on
+  // NativeViewHost::Attach()/Detach().
+  void OnNativeViewHostAttach(const views::View* view, NSView* native_view);
+  void OnNativeViewHostDetach(const views::View* view);
 
   // Sorts child NSViews according to NativeViewHosts order in views hierarchy.
   void ReorderChildViews();
@@ -230,7 +231,13 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
 
   void UpdateCompositorProperties();
   void DestroyCompositor();
-  void RankNSViewsRecursive(View* view, std::map<NSView*, int>* rank) const;
+
+  // Sort |attached_native_view_host_views_| by the order in which their
+  // NSViews should appear as subviews. This does a recursive pre-order
+  // traversal of the views::View tree starting at |view|.
+  void GetAttachedNativeViewHostViewsRecursive(
+      View* view,
+      std::vector<NSView*>* attached_native_view_host_views_ordered) const;
 
   // If we are accessing the BridgedNativeWidget through mojo, then
   // |in_process_ns_window_| is not the true window that is resized. This
@@ -470,8 +477,9 @@ class VIEWS_EXPORT NativeWidgetMacNSWindowHost
   // 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_;
+  // Contains NativeViewHost->gfx::NativeView associations for NativeViewHosts
+  // attached to |this|.
+  std::map<const views::View*, NSView*> attached_native_view_host_views_;
 
   mojo::AssociatedBinding<remote_cocoa::mojom::NativeWidgetNSWindowHost>
       remote_ns_window_host_binding_;

ui/views/cocoa/native_widget_mac_ns_window_host.mm

@@ -681,37 +681,55 @@ void NativeWidgetMacNSWindowHost::SetParent(
   }
 }
 
-void NativeWidgetMacNSWindowHost::SetAssociationForView(const View* view,
-                                                        NSView* native_view) {
-  DCHECK_EQ(0u, associated_views_.count(view));
-  associated_views_[view] = native_view;
+void NativeWidgetMacNSWindowHost::OnNativeViewHostAttach(const View* view,
+                                                         NSView* native_view) {
+  DCHECK_EQ(0u, attached_native_view_host_views_.count(view));
+  attached_native_view_host_views_[view] = native_view;
   native_widget_mac_->GetWidget()->ReorderNativeViews();
 }
 
-void NativeWidgetMacNSWindowHost::ClearAssociationForView(const View* view) {
-  auto it = associated_views_.find(view);
-  DCHECK(it != associated_views_.end());
-  associated_views_.erase(it);
+void NativeWidgetMacNSWindowHost::OnNativeViewHostDetach(const View* view) {
+  auto it = attached_native_view_host_views_.find(view);
+  DCHECK(it != attached_native_view_host_views_.end());
+  attached_native_view_host_views_.erase(it);
 }
 
 void NativeWidgetMacNSWindowHost::ReorderChildViews() {
   Widget* widget = native_widget_mac_->GetWidget();
   if (!widget->GetRootView())
     return;
-  std::map<NSView*, int> rank;
-  RankNSViewsRecursive(widget->GetRootView(), &rank);
-  if (in_process_ns_window_bridge_)
-    in_process_ns_window_bridge_->SortSubviews(std::move(rank));
+
+  // Get the ordering for the NSViews in |attached_native_view_host_views_|.
+  std::vector<NSView*> attached_subviews;
+  GetAttachedNativeViewHostViewsRecursive(widget->GetRootView(),
+                                          &attached_subviews);
+
+  // Convert to NSView ids that can go over mojo. If need be, create temporary
+  // NSView ids.
+  std::vector<uint64_t> attached_subview_ids;
+  std::list<remote_cocoa::ScopedNSViewIdMapping> temp_ids;
+  for (NSView* subview : attached_subviews) {
+    uint64_t ns_view_id = remote_cocoa::GetIdFromNSView(subview);
+    if (!ns_view_id) {
+      // Subviews that do not already have an id will not work if the target is
+      // in a different process.
+      DCHECK(in_process_ns_window_bridge_);
+      ns_view_id = remote_cocoa::GetNewNSViewId();
+      temp_ids.emplace_back(ns_view_id, subview);
+    }
+    attached_subview_ids.push_back(ns_view_id);
+  }
+  GetNSWindowMojo()->SortSubviews(attached_subview_ids);
 }
 
-void NativeWidgetMacNSWindowHost::RankNSViewsRecursive(
+void NativeWidgetMacNSWindowHost::GetAttachedNativeViewHostViewsRecursive(
     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());
+    std::vector<NSView*>* order) const {
+  auto found = attached_native_view_host_views_.find(view);
+  if (found != attached_native_view_host_views_.end())
+    order->push_back(found->second);
   for (View* child : view->children())
-    RankNSViewsRecursive(child, rank);
+    GetAttachedNativeViewHostViewsRecursive(child, order);
 }
 
 void NativeWidgetMacNSWindowHost::UpdateLocalWindowFrame(

ui/views/controls/native/native_view_host_mac.mm

@@ -104,7 +104,7 @@ void NativeViewHostMac::AttachNativeView() {
         host_->parent()->GetNativeViewAccessible());
   }
 
-  window_host->SetAssociationForView(host_, native_view_);
+  window_host->OnNativeViewHostAttach(host_, native_view_);
 }
 
 void NativeViewHostMac::NativeViewDetaching(bool destroyed) {
@@ -133,7 +133,7 @@ void NativeViewHostMac::NativeViewDetaching(bool destroyed) {
   auto* window_host = GetNSWindowHost();
   // NativeWidgetNSWindowBridge can be null when Widget is closing.
   if (window_host)
-    window_host->ClearAssociationForView(host_);
+    window_host->OnNativeViewHostDetach(host_);
 
   native_view_.reset();
 }