Fix broken assumptions about Widgets.

Widgets, GetNativeWindow(), and GetNativeView() should always be in
1:1:1 correspondence. NativeWidgetPrivate::ReparentNativeView()
violates that correspondence on some code paths; remove those paths.

Also rename some variables in that function for clarity.

BUG=1035131, 957362

Change-Id: I761b86e6e53f27c562aa1444d13e5fc050d8faed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972851Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727036}

ui/views/widget/native_widget_mac.mm

@@ -921,74 +921,49 @@ void NativeWidgetPrivate::GetAllOwnedWidgets(gfx::NativeView native_view,
 }
 
 // static
-void NativeWidgetPrivate::ReparentNativeView(gfx::NativeView native_view,
+void NativeWidgetPrivate::ReparentNativeView(gfx::NativeView child,
                                              gfx::NativeView new_parent) {
-  DCHECK_NE(native_view, new_parent);
+  DCHECK_NE(child, new_parent);
   DCHECK([new_parent.GetNativeNSView() window]);
-  if (!new_parent || [native_view.GetNativeNSView() superview] ==
-                         new_parent.GetNativeNSView()) {
+  if (!new_parent ||
+      [child.GetNativeNSView() superview] == new_parent.GetNativeNSView()) {
     NOTREACHED();
     return;
   }
 
-  NativeWidgetMacNSWindowHost* window_host =
-      NativeWidgetMacNSWindowHost::GetFromNativeView(native_view);
-  DCHECK(window_host);
-  gfx::NativeView bridge_view =
-      window_host->native_widget_mac()->GetNativeView();
-  gfx::NativeWindow bridge_window =
-      window_host->native_widget_mac()->GetNativeWindow();
-  bool bridge_is_top_level =
-      window_host->native_widget_mac()->GetWidget()->is_top_level();
-  DCHECK([native_view.GetNativeNSView()
-      isDescendantOf:bridge_view.GetNativeNSView()]);
-  DCHECK(bridge_window && ![bridge_window.GetNativeNSWindow() isSheet]);
+  NativeWidgetMacNSWindowHost* child_window_host =
+      NativeWidgetMacNSWindowHost::GetFromNativeView(child);
+  DCHECK(child_window_host);
+  gfx::NativeView widget_view =
+      child_window_host->native_widget_mac()->GetNativeView();
+  DCHECK_EQ(child, widget_view);
+  gfx::NativeWindow widget_window =
+      child_window_host->native_widget_mac()->GetNativeWindow();
+  DCHECK(
+      [child.GetNativeNSView() isDescendantOf:widget_view.GetNativeNSView()]);
+  DCHECK(widget_window && ![widget_window.GetNativeNSWindow() isSheet]);
 
   NativeWidgetMacNSWindowHost* parent_window_host =
       NativeWidgetMacNSWindowHost::GetFromNativeView(new_parent);
 
   // Early out for no-op changes.
-  if (native_view == bridge_view && bridge_is_top_level &&
-      window_host->parent() == parent_window_host) {
+  if (child == widget_view &&
+      child_window_host->parent() == parent_window_host) {
     return;
   }
 
   // First notify all the widgets that they are being disassociated from their
   // previous parent.
   Widget::Widgets widgets;
-  GetAllChildWidgets(native_view, &widgets);
-  for (auto* child : widgets)
-    child->NotifyNativeViewHierarchyWillChange();
-
-  // Update |bridge_host|'s parent only if
-  // NativeWidgetNSWindowBridge::ReparentNativeView will.
-  if (native_view == bridge_view) {
-    window_host->SetParent(parent_window_host);
-    if (!bridge_is_top_level) {
-      // Make |window_host|'s NSView be a child of |new_parent| by adding it as
-      // a subview. Note that this will have the effect of removing
-      // |window_host|'s NSView from its NSWindow. The |NSWindow| must remain
-      // visible because it controls the bounds and visibility of the ui::Layer,
-      // so just hide it by setting alpha value to zero.
-      // TODO(ccameron): This path likely violates assumptions. Verify that this
-      // path is unused and remove it.
-      LOG(ERROR) << "Reparenting a non-top-level BridgedNativeWidget. This is "
-                    "likely unsupported.";
-      [new_parent.GetNativeNSView() addSubview:native_view.GetNativeNSView()];
-      [bridge_window.GetNativeNSWindow() setAlphaValue:0];
-      [bridge_window.GetNativeNSWindow() setIgnoresMouseEvents:YES];
-    }
-  } else {
-    // TODO(ccameron): This path likely violates assumptions. Verify that this
-    // path is unused and remove it.
-    LOG(ERROR) << "Reparenting with a non-root BridgedNativeWidget NSView. "
-                  "This is likely unsupported.";
-    [new_parent.GetNativeNSView() addSubview:native_view.GetNativeNSView()];
-  }
+  GetAllChildWidgets(child, &widgets);
+  for (auto* widget : widgets)
+    widget->NotifyNativeViewHierarchyWillChange();
+
+  child_window_host->SetParent(parent_window_host);
 
   // And now, notify them that they have a brand new parent.
-  for (auto* child : widgets)
-    child->NotifyNativeViewHierarchyChanged();
+  for (auto* widget : widgets)
+    widget->NotifyNativeViewHierarchyChanged();
 }
 
 // static

ui/views/widget/native_widget_mac_unittest.mm

@@ -2081,46 +2081,6 @@ TEST_F(NativeWidgetMacTest, ChangeFocusOnChangeFirstResponder) {
   widget->CloseNow();
 }
 
-// Ensure reparented native view has correct bounds.
-TEST_F(NativeWidgetMacTest, ReparentNativeViewBounds) {
-  Widget* parent = CreateTopLevelFramelessPlatformWidget();
-  gfx::Rect parent_rect(100, 100, 300, 200);
-  parent->SetBounds(parent_rect);
-
-  Widget::InitParams params(Widget::InitParams::TYPE_CONTROL);
-  params.parent = parent->GetNativeView();
-  Widget* widget = new Widget;
-  widget->Init(std::move(params));
-  widget->SetContentsView(new View);
-
-  NSView* child_view = widget->GetNativeView().GetNativeNSView();
-  Widget::ReparentNativeView(child_view, parent->GetNativeView());
-
-  // Reparented content view has the size of the Widget that created it.
-  gfx::Rect widget_rect(0, 0, 200, 100);
-  widget->SetBounds(widget_rect);
-  EXPECT_EQ(200, NSWidth([child_view frame]));
-  EXPECT_EQ(100, NSHeight([child_view frame]));
-
-  // Reparented widget has bounds relative to the native parent
-  NSRect native_parent_rect = NSMakeRect(50, 100, 200, 70);
-  base::scoped_nsobject<NSView> native_parent(
-      [[NSView alloc] initWithFrame:native_parent_rect]);
-  [parent->GetNativeView().GetNativeNSView() addSubview:native_parent];
-
-  gfx::Rect screen_rect = widget->GetWindowBoundsInScreen();
-  EXPECT_EQ(100, screen_rect.x());
-  EXPECT_EQ(100, screen_rect.y());
-
-  Widget::ReparentNativeView(child_view, native_parent.get());
-  widget->SetBounds(widget_rect);
-  screen_rect = widget->GetWindowBoundsInScreen();
-  EXPECT_EQ(150, screen_rect.x());
-  EXPECT_EQ(130, screen_rect.y());
-
-  parent->CloseNow();
-}
-
 // Test two kinds of widgets to re-parent.
 TEST_F(NativeWidgetMacTest, ReparentNativeViewTypes) {
   std::unique_ptr<Widget> toplevel1(new Widget);
@@ -2141,16 +2101,13 @@ TEST_F(NativeWidgetMacTest, ReparentNativeViewTypes) {
 
   Widget::ReparentNativeView(child->GetNativeView(),
                              toplevel1->GetNativeView());
-  EXPECT_EQ([child->GetNativeView().GetNativeNSView() window],
+  EXPECT_EQ([child->GetNativeWindow().GetNativeNSWindow() parentWindow],
             [toplevel1->GetNativeView().GetNativeNSView() window]);
-  EXPECT_EQ(0, [child->GetNativeWindow().GetNativeNSWindow() alphaValue]);
 
   Widget::ReparentNativeView(child->GetNativeView(),
                              toplevel2->GetNativeView());
-  EXPECT_EQ([child->GetNativeView().GetNativeNSView() window],
+  EXPECT_EQ([child->GetNativeWindow().GetNativeNSWindow() parentWindow],
             [toplevel2->GetNativeView().GetNativeNSView() window]);
-  EXPECT_EQ(0, [child->GetNativeWindow().GetNativeNSWindow() alphaValue]);
-  EXPECT_NE(0, [toplevel1->GetNativeWindow().GetNativeNSWindow() alphaValue]);
 
   Widget::ReparentNativeView(toplevel2->GetNativeView(),
                              toplevel1->GetNativeView());