[Mac] Delete ViewsCompositorSuperview. Shouldn't change behavior.

Bug: 865901
Change-Id: I02010ee8fec71011353f1b0be23e54055af0adf4
Reviewed-on: https://chromium-review.googlesource.com/1236675
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594160}

ui/views/cocoa/bridged_native_widget.h

@@ -259,9 +259,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   // Notify descendants of a visibility change.
   void NotifyVisibilityChangeDown();
 
-  // Installs the NSView for hosting the composited layer.
-  void AddCompositorSuperview();
-
   // Query the display properties of the monitor that |window_| is on, and
   // forward them to |host_|.
   void UpdateWindowDisplay();
@@ -312,7 +309,6 @@ class VIEWS_EXPORT BridgedNativeWidgetImpl
   // |content_dip_size_|, which is the frame size most recently *sent to* the
   // compositor.
   gfx::Size compositor_frame_dip_size_;
-  base::scoped_nsobject<NSView> compositor_superview_;
   std::unique_ptr<ui::DisplayCALayerTree> display_ca_layer_tree_;
 
   // Tracks the bounds when the window last started entering fullscreen. Used to

ui/views/cocoa/bridged_native_widget.mm

@@ -79,18 +79,6 @@ constexpr auto kUIPaintTimeout = base::TimeDelta::FromSeconds(5);
 }
 @end
 
-// The NSView that hosts the composited CALayer drawing the UI. It fills the
-// window but is not hittable so that accessibility hit tests always go to the
-// BridgedContentView.
-@interface ViewsCompositorSuperview : NSView
-@end
-
-@implementation ViewsCompositorSuperview
-- (NSView*)hitTest:(NSPoint)aPoint {
-  return nil;
-}
-@end
-
 // This class overrides NSAnimation methods to invalidate the shadow for each
 // frame. It is required because the show animation uses CGSSetWindowWarp()
 // which is touchy about the consistency of the points it is given. The show
@@ -463,8 +451,6 @@ void BridgedNativeWidgetImpl::DestroyContentView() {
 
 void BridgedNativeWidgetImpl::CreateContentView(const gfx::Rect& bounds) {
   DCHECK(!bridged_view_);
-  // The compositor needs to point at the new content view created here.
-  DCHECK(!compositor_superview_);
 
   bridged_view_.reset(
       [[BridgedContentView alloc] initWithBridge:this bounds:bounds]);
@@ -473,8 +459,12 @@ void BridgedNativeWidgetImpl::CreateContentView(const gfx::Rect& bounds) {
   // this should be treated as an error and caught early.
   CHECK(bridged_view_);
 
-  // Layer backing the content view improves resize performance, reduces memory
-  // use (no backing store), and clips sublayers to rounded window corners.
+  // Set the layer first to create a layer-hosting view (not layer-backed), and
+  // set the compositor output to go to that layer.
+  base::scoped_nsobject<CALayer> background_layer([[CALayer alloc] init]);
+  display_ca_layer_tree_ =
+      std::make_unique<ui::DisplayCALayerTree>(background_layer.get());
+  [bridged_view_ setLayer:background_layer];
   [bridged_view_ setWantsLayer:YES];
 
   [window_ setContentView:bridged_view_];
@@ -886,8 +876,6 @@ void BridgedNativeWidgetImpl::OnShowAnimationComplete() {
 }
 
 void BridgedNativeWidgetImpl::InitCompositorView() {
-  AddCompositorSuperview();
-
   // Use the regular window background for window modal sheets. The layer will
   // still paint over most of it, but the native -[NSApp beginSheet:] animation
   // blocks the UI thread, so there's no way to invalidate the shadow to match
@@ -919,12 +907,6 @@ void BridgedNativeWidgetImpl::SortSubviews(RankMap rank) {
   // orderOut: in Close(), which notifies visibility changes to Views.
   if (!bridged_view_)
     return;
-
-  // 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.
-  if (compositor_superview_)
-    rank[compositor_superview_] = -1;
   [bridged_view_ sortSubviewsUsingFunction:&SubviewSorter context:&rank];
 }
 
@@ -1021,7 +1003,7 @@ bool BridgedNativeWidgetImpl::ShouldWaitInPreCommit() {
     return false;
   if (ca_transaction_sync_suppressed_)
     return false;
-  if (!compositor_superview_)
+  if (!bridged_view_)
     return false;
   return content_dip_size_ != compositor_frame_dip_size_;
 }
@@ -1227,35 +1209,6 @@ void BridgedNativeWidgetImpl::NotifyVisibilityChangeDown() {
             CountBridgedWindows([window_ childWindows]));
 }
 
-void BridgedNativeWidgetImpl::AddCompositorSuperview() {
-  DCHECK(!compositor_superview_);
-  compositor_superview_.reset(
-      [[ViewsCompositorSuperview alloc] initWithFrame:[bridged_view_ bounds]]);
-
-  // Size and resize automatically with |bridged_view_|.
-  [compositor_superview_
-      setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
-
-  // Enable HiDPI backing when supported (only on 10.7+).
-  if ([compositor_superview_ respondsToSelector:
-      @selector(setWantsBestResolutionOpenGLSurface:)]) {
-    [compositor_superview_ setWantsBestResolutionOpenGLSurface:YES];
-  }
-
-  // Set the layer first to create a layer-hosting view (not layer-backed), and
-  // set the compositor output to go to that layer.
-  base::scoped_nsobject<CALayer> background_layer([[CALayer alloc] init]);
-  display_ca_layer_tree_ =
-      std::make_unique<ui::DisplayCALayerTree>(background_layer.get());
-  [compositor_superview_ setLayer:background_layer];
-  [compositor_superview_ setWantsLayer:YES];
-
-  // The UI compositor should always be the first subview, to ensure webviews
-  // are drawn on top of it.
-  DCHECK_EQ(0u, [[bridged_view_ subviews] count]);
-  [bridged_view_ addSubview:compositor_superview_];
-}
-
 void BridgedNativeWidgetImpl::UpdateWindowGeometry() {
   gfx::Rect window_in_screen = gfx::ScreenRectFromNSRect([window_ frame]);
   gfx::Rect content_in_screen = gfx::ScreenRectFromNSRect(

ui/views/controls/menu/menu_runner_cocoa_unittest.mm

@@ -106,6 +106,8 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
         gfx::Rect(kWindowOffset, kWindowOffset, kWindowWidth, kWindowHeight));
     parent_->Show();
 
+    native_view_subview_count_ = [[parent_->GetNativeView() subviews] count];
+
     base::Closure on_close = base::Bind(&MenuRunnerCocoaTest::MenuCloseCallback,
                                         base::Unretained(this));
     if (GetParam() == MenuType::NATIVE)
@@ -116,6 +118,9 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
   }
 
   void TearDown() override {
+    EXPECT_EQ(native_view_subview_count_,
+              [[parent_->GetNativeView() subviews] count]);
+
     if (runner_) {
       runner_->Release();
       runner_ = NULL;
@@ -151,10 +156,6 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
   void RunMenuAt(const gfx::Rect& anchor) {
     last_anchor_frame_ = NSZeroRect;
 
-    // Should be one child (the compositor layer) before showing, and it should
-    // go up by one (the anchor view) while the menu is shown.
-    EXPECT_EQ(1u, [[parent_->GetNativeView() subviews] count]);
-
     base::OnceClosure callback =
         base::BindOnce(&MenuRunnerCocoaTest::ComboboxRunMenuAtCallback,
                        base::Unretained(this));
@@ -168,9 +169,6 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
     runner_->RunMenuAt(parent_, nullptr, anchor, MENU_ANCHOR_TOPLEFT,
                        MenuRunner::COMBOBOX);
     MaybeRunAsync();
-
-    // Ensure the anchor view is removed.
-    EXPECT_EQ(1u, [[parent_->GetNativeView() subviews] count]);
   }
 
   void MenuCancelCallback() {
@@ -244,6 +242,7 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
   internal::MenuRunnerImplInterface* runner_ = nullptr;
   views::Widget* parent_ = nullptr;
   NSRect last_anchor_frame_ = NSZeroRect;
+  NSUInteger native_view_subview_count_ = 0;
   int menu_close_count_ = 0;
 
  private:
@@ -256,10 +255,11 @@ class MenuRunnerCocoaTest : public ViewsTestBase,
     NSArray* subviews = [parent_->GetNativeView() subviews];
     // An anchor view should only be added for Native menus.
     if (GetParam() == MenuType::NATIVE) {
-      ASSERT_EQ(2u, [subviews count]);
-      last_anchor_frame_ = [[subviews objectAtIndex:1] frame];
+      ASSERT_EQ(native_view_subview_count_ + 1, [subviews count]);
+      last_anchor_frame_ =
+          [[subviews objectAtIndex:native_view_subview_count_] frame];
     } else {
-      EXPECT_EQ(1u, [subviews count]);
+      EXPECT_EQ(native_view_subview_count_, [subviews count]);
     }
     runner_->Cancel();
   }

ui/views/widget/native_widget_mac_unittest.mm

@@ -2180,8 +2180,10 @@ class NativeWidgetMacViewsOrderTest : public WidgetTest {
 
     widget_ = CreateTopLevelPlatformWidget();
 
-    ASSERT_EQ(1u, [[widget_->GetNativeView() subviews] count]);
-    compositor_view_ = [[widget_->GetNativeView() subviews] firstObject];
+    // It's not a bug if the native view has a different number of subviews,
+    // but the rest of this test assumes it. It may or may not be worth
+    // removing that assumption at some point.
+    ASSERT_EQ(0u, [[widget_->GetNativeView() subviews] count]);
 
     native_host_parent_ = new View();
     widget_->GetContentsView()->AddChildView(native_host_parent_);
@@ -2194,10 +2196,8 @@ class NativeWidgetMacViewsOrderTest : public WidgetTest {
       hosts_.push_back(std::move(holder));
     }
     EXPECT_EQ(kNativeViewCount, native_host_parent_->child_count());
-    EXPECT_NSEQ([widget_->GetNativeView() subviews], (@[
-                  compositor_view_, hosts_[0]->view(), hosts_[1]->view(),
-                  hosts_[2]->view()
-                ]));
+    EXPECT_NSEQ([widget_->GetNativeView() subviews],
+                (@[ hosts_[0]->view(), hosts_[1]->view(), hosts_[2]->view() ]));
   }
 
   void TearDown() override {
@@ -2209,7 +2209,6 @@ class NativeWidgetMacViewsOrderTest : public WidgetTest {
 
   Widget* widget_ = nullptr;
   View* native_host_parent_ = nullptr;
-  NSView* compositor_view_ = nil;
   std::vector<std::unique_ptr<NativeHostHolder>> hosts_;
 
  private:
@@ -2221,58 +2220,50 @@ class NativeWidgetMacViewsOrderTest : public WidgetTest {
 TEST_F(NativeWidgetMacViewsOrderTest, NativeViewAttached) {
   hosts_[1]->Detach();
   EXPECT_NSEQ([GetContentNativeView() subviews],
-              (@[ compositor_view_, hosts_[0]->view(), hosts_[2]->view() ]));
+              (@[ hosts_[0]->view(), hosts_[2]->view() ]));
 
   hosts_[1]->AttachNativeView();
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[0]->view(), hosts_[1]->view(),
-                hosts_[2]->view()
-              ]));
+  EXPECT_NSEQ([GetContentNativeView() subviews],
+              (@[ hosts_[0]->view(), hosts_[1]->view(), hosts_[2]->view() ]));
 }
 
 // Tests that NativeViews order changes according to views::View hierarchy.
 TEST_F(NativeWidgetMacViewsOrderTest, ReorderViews) {
   native_host_parent_->ReorderChildView(hosts_[2]->host(), 1);
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[0]->view(), hosts_[2]->view(),
-                hosts_[1]->view()
-              ]));
+  EXPECT_NSEQ([GetContentNativeView() subviews],
+              (@[ hosts_[0]->view(), hosts_[2]->view(), hosts_[1]->view() ]));
 
   native_host_parent_->RemoveChildView(hosts_[2]->host());
   EXPECT_NSEQ([GetContentNativeView() subviews],
-              (@[ compositor_view_, hosts_[0]->view(), hosts_[1]->view() ]));
+              (@[ hosts_[0]->view(), hosts_[1]->view() ]));
 
   View* new_parent = new View();
   native_host_parent_->RemoveChildView(hosts_[1]->host());
   native_host_parent_->AddChildView(new_parent);
   new_parent->AddChildView(hosts_[1]->host());
   new_parent->AddChildView(hosts_[2]->host());
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[0]->view(), hosts_[1]->view(),
-                hosts_[2]->view()
-              ]));
+  EXPECT_NSEQ([GetContentNativeView() subviews],
+              (@[ hosts_[0]->view(), hosts_[1]->view(), hosts_[2]->view() ]));
 
   native_host_parent_->ReorderChildView(new_parent, 0);
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[1]->view(), hosts_[2]->view(),
-                hosts_[0]->view()
-              ]));
+  EXPECT_NSEQ([GetContentNativeView() subviews],
+              (@[ hosts_[1]->view(), hosts_[2]->view(), hosts_[0]->view() ]));
 }
 
 // Test that unassociated native views stay on top after reordering.
 TEST_F(NativeWidgetMacViewsOrderTest, UnassociatedViewsIsAbove) {
   base::scoped_nsobject<NSView> child_view([[NSView alloc] init]);
   [GetContentNativeView() addSubview:child_view];
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[0]->view(), hosts_[1]->view(),
-                hosts_[2]->view(), child_view
-              ]));
+  EXPECT_NSEQ(
+      [GetContentNativeView() subviews], (@[
+        hosts_[0]->view(), hosts_[1]->view(), hosts_[2]->view(), child_view
+      ]));
 
   native_host_parent_->ReorderChildView(hosts_[2]->host(), 1);
-  EXPECT_NSEQ([GetContentNativeView() subviews], (@[
-                compositor_view_, hosts_[0]->view(), hosts_[2]->view(),
-                hosts_[1]->view(), child_view
-              ]));
+  EXPECT_NSEQ(
+      [GetContentNativeView() subviews], (@[
+        hosts_[0]->view(), hosts_[2]->view(), hosts_[1]->view(), child_view
+      ]));
 }
 
 // Test -[NSWindowDelegate windowShouldClose:].