WebView: Fixes accessibility bug when reparenting WebView

This CL fixes a bug whereby the WebView class was not updating
its accessible parent view when reparented to a new Widget
hierarchy.

Using VoiceOver on Mac while reparenting the WebView otherwise
results in a crash.

Bug: 1148710
Change-Id: If690f1bdcb669cddde411cbdeb64890ac774bd0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537594
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827895}

content/browser/web_contents/web_contents_view_mac.h

@@ -120,6 +120,7 @@ class WebContentsViewMac : public WebContentsView,
   void ViewsHostableMakeFirstResponder() override;
   void ViewsHostableSetParentAccessible(
       gfx::NativeViewAccessible parent_accessibility_element) override;
+  gfx::NativeViewAccessible ViewsHostableGetParentAccessible() override;
   gfx::NativeViewAccessible ViewsHostableGetAccessibilityElement() override;
 
   // A helper method for closing the tab in the

content/browser/web_contents/web_contents_view_mac.mm

@@ -701,6 +701,11 @@ void WebContentsViewMac::ViewsHostableSetParentAccessible(
     rwhv_mac->SetParentAccessibilityElement(views_host_accessibility_element_);
 }
 
+gfx::NativeViewAccessible
+WebContentsViewMac::ViewsHostableGetParentAccessible() {
+  return views_host_accessibility_element_;
+}
+
 gfx::NativeViewAccessible
 WebContentsViewMac::ViewsHostableGetAccessibilityElement() {
   RenderWidgetHostView* rwhv = web_contents_->GetRenderWidgetHostView();

ui/base/cocoa/views_hostable.h

@@ -74,6 +74,9 @@ class ViewsHostableView {
   virtual void ViewsHostableSetParentAccessible(
       gfx::NativeViewAccessible parent_accessibility_element) = 0;
 
+  // Get the WebContentsView's parent accessibility element.
+  virtual gfx::NativeViewAccessible ViewsHostableGetParentAccessible() = 0;
+
   // Retrieve the WebContentsView's accessibility element.
   virtual gfx::NativeViewAccessible ViewsHostableGetAccessibilityElement() = 0;
 };

ui/views/controls/native/native_view_host.cc

@@ -58,6 +58,10 @@ void NativeViewHost::SetParentAccessible(gfx::NativeViewAccessible accessible) {
   native_wrapper_->SetParentAccessible(accessible);
 }
 
+gfx::NativeViewAccessible NativeViewHost::GetParentAccessible() {
+  return native_wrapper_->GetParentAccessible();
+}
+
 bool NativeViewHost::SetCornerRadii(const gfx::RoundedCornersF& corner_radii) {
   return native_wrapper_->SetCornerRadii(corner_radii);
 }

ui/views/controls/native/native_view_host.h

@@ -83,6 +83,9 @@ class VIEWS_EXPORT NativeViewHost : public View {
   // it can return this value when querying its parent accessible.
   void SetParentAccessible(gfx::NativeViewAccessible);
 
+  // Returns the parent accessible object to this host's native view.
+  gfx::NativeViewAccessible GetParentAccessible();
+
   // Fast resizing will move the native view and clip its visible region, this
   // will result in white areas and will not resize the content (so scrollbars
   // will be all wrong and content will flow offscreen). Only use this

ui/views/controls/native/native_view_host_aura.cc

@@ -111,6 +111,11 @@ void NativeViewHostAura::SetParentAccessible(
       aura::client::kParentNativeViewAccessibleKey, accessible);
 }
 
+gfx::NativeViewAccessible NativeViewHostAura::GetParentAccessible() {
+  return host_->native_view()->GetProperty(
+      aura::client::kParentNativeViewAccessibleKey);
+}
+
 void NativeViewHostAura::NativeViewDetaching(bool destroyed) {
   // This method causes a succession of window tree changes. ScopedPause ensures
   // that occlusion is recomputed at the end of the method instead of after each

ui/views/controls/native/native_view_host_aura.h

@@ -52,6 +52,7 @@ class NativeViewHostAura : public NativeViewHostWrapper,
   gfx::NativeCursor GetCursor(int x, int y) override;
   void SetVisible(bool visible) override;
   void SetParentAccessible(gfx::NativeViewAccessible) override;
+  gfx::NativeViewAccessible GetParentAccessible() override;
 
  private:
   friend class NativeViewHostAuraTest;

ui/views/controls/native/native_view_host_mac.h

@@ -61,6 +61,7 @@ class NativeViewHostMac : public NativeViewHostWrapper,
   gfx::NativeCursor GetCursor(int x, int y) override;
   void SetVisible(bool visible) override;
   void SetParentAccessible(gfx::NativeViewAccessible) override;
+  gfx::NativeViewAccessible GetParentAccessible() override;
 
  private:
   // Return the NativeWidgetMacNSWindowHost for this hosted view.

ui/views/controls/native/native_view_host_mac.mm

@@ -274,6 +274,12 @@ void NativeViewHostMac::SetParentAccessible(
   }
 }
 
+gfx::NativeViewAccessible NativeViewHostMac::GetParentAccessible() {
+  return native_view_hostable_
+             ? native_view_hostable_->ViewsHostableGetParentAccessible()
+             : nullptr;
+}
+
 // static
 NativeViewHostWrapper* NativeViewHostWrapper::CreateWrapper(
     NativeViewHost* host) {

ui/views/controls/native/native_view_host_mac_unittest.mm

@@ -36,6 +36,9 @@ class TestViewsHostable : public ui::ViewsHostableView {
       gfx::NativeViewAccessible parent_accessibility_element) override {
     parent_accessibility_element_ = parent_accessibility_element;
   }
+  gfx::NativeViewAccessible ViewsHostableGetParentAccessible() override {
+    return parent_accessibility_element_;
+  }
   gfx::NativeViewAccessible ViewsHostableGetAccessibilityElement() override {
     return nil;
   }

ui/views/controls/native/native_view_host_wrapper.h

@@ -115,6 +115,9 @@ class NativeViewHostWrapper {
   // this value when querying its parent accessible.
   virtual void SetParentAccessible(gfx::NativeViewAccessible) = 0;
 
+  // Returns the parent accessible object to the native view.
+  virtual gfx::NativeViewAccessible GetParentAccessible() = 0;
+
   // Creates a platform-specific instance of an object implementing this
   // interface.
   static NativeViewHostWrapper* CreateWrapper(NativeViewHost* host);

ui/views/controls/webview/webview.cc

@@ -254,6 +254,14 @@ void WebView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   }
 }
 
+void WebView::AddedToWidget() {
+  // If added to a widget hierarchy and |holder_| already has a NativeView
+  // attached, update the accessible parent here to support reparenting the
+  // WebView.
+  if (web_contents() && holder_->native_view())
+    UpdateNativeViewHostAccessibleParent(holder_, parent());
+}
+
 gfx::NativeViewAccessible WebView::GetNativeViewAccessible() {
   if (web_contents() && !web_contents()->IsCrashed()) {
     content::RenderWidgetHostView* host_view =

ui/views/controls/webview/webview.h

@@ -135,6 +135,7 @@ class WEBVIEW_EXPORT WebView : public View,
   void AboutToRequestFocusFromTabTraversal(bool reverse) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   gfx::NativeViewAccessible GetNativeViewAccessible() override;
+  void AddedToWidget() override;
 
   // Overridden from content::WebContentsObserver:
   void RenderViewCreated(content::RenderViewHost* render_view_host) override;

ui/views/controls/webview/webview_unittest.cc

@@ -391,4 +391,34 @@ TEST_F(WebViewUnitTest, DefaultConstructability) {
   EXPECT_EQ(browser_context.get(), web_contents->GetBrowserContext());
 }
 
+// Tests that when a web view is reparented to a different widget hierarchy its
+// holder's parent NativeViewAccessible matches that of its parent view's
+// NativeViewAccessible.
+TEST_F(WebViewUnitTest, ReparentingUpdatesParentAccessible) {
+  const std::unique_ptr<content::WebContents> web_contents(CreateWebContents());
+  auto web_view = std::make_unique<WebView>(web_contents->GetBrowserContext());
+  web_view->SetWebContents(web_contents.get());
+
+  WidgetAutoclosePtr widget_1(CreateTopLevelPlatformWidget());
+  View* contents_view_1 = widget_1->GetContentsView();
+  WebView* added_web_view = contents_view_1->AddChildView(std::move(web_view));
+
+  // After being added to the widget hierarchy the holder's NativeViewAccessible
+  // should match that of the web view's parent view.
+  EXPECT_EQ(added_web_view->parent()->GetNativeViewAccessible(),
+            added_web_view->holder()->GetParentAccessible());
+
+  WidgetAutoclosePtr widget_2(CreateTopLevelPlatformWidget());
+  View* contents_view_2 = widget_2->GetContentsView();
+
+  // Reparent the web view.
+  added_web_view = contents_view_2->AddChildView(
+      contents_view_1->RemoveChildViewT(added_web_view));
+
+  // After reparenting the holder's NativeViewAccessible should match that of
+  // the web view's new parent view.
+  EXPECT_EQ(added_web_view->parent()->GetNativeViewAccessible(),
+            added_web_view->holder()->GetParentAccessible());
+}
+
 }  // namespace views