Remove set_owned_by_client() from ExtensionViewViews.

Instead, have the ExtensionDialog or ExtensionPopup create this and
ensure it's properly owned by the Views hierarchy.

Bug: 1044687
Change-Id: I988473f274a0b0f7434ba85e4874932e8cb45cc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207803
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770631}

chrome/browser/extensions/extension_view_host.cc

@@ -99,8 +99,6 @@ ExtensionViewHost::ExtensionViewHost(const Extension* extension,
         host_contents()->GetRenderViewHost()->GetRoutingID(),
         zoom_map->GetDefaultZoomLevel());
   }
-
-  view_ = CreateExtensionView(this);
 }
 
 ExtensionViewHost::~ExtensionViewHost() {

chrome/browser/extensions/extension_view_host.h

@@ -45,7 +45,9 @@ class ExtensionViewHost
   ~ExtensionViewHost() override;
 
   Browser* browser() { return browser_; }
-  ExtensionView* view() { return view_.get(); }
+
+  void set_view(ExtensionView* view) { view_ = view; }
+  ExtensionView* view() { return view_; }
 
   void SetAssociatedWebContents(content::WebContents* web_contents);
 
@@ -111,10 +113,6 @@ class ExtensionViewHost
                const content::NotificationDetails& details) override;
 
  private:
-  // Implemented per-platform. Create the platform-specific ExtensionView.
-  static std::unique_ptr<ExtensionView> CreateExtensionView(
-      ExtensionViewHost* host);
-
   // Returns whether the provided event is a raw escape keypress in a
   // VIEW_TYPE_EXTENSION_POPUP.
   bool IsEscapeInPopup(const content::NativeWebKeyboardEvent& event) const;
@@ -123,7 +121,7 @@ class ExtensionViewHost
   Browser* browser_;
 
   // View that shows the rendered content in the UI.
-  std::unique_ptr<ExtensionView> view_;
+  ExtensionView* view_;
 
   // The relevant WebContents associated with this ExtensionViewHost, if any.
   content::WebContents* associated_web_contents_ = nullptr;

chrome/browser/extensions/extension_view_host_factory_browsertest.cc

@@ -32,7 +32,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionViewHostFactoryTest, CreateExtensionHosts) {
     EXPECT_EQ(extension.get(), host->extension());
     EXPECT_EQ(browser_context, host->browser_context());
     EXPECT_EQ(VIEW_TYPE_EXTENSION_POPUP, host->extension_host_type());
-    EXPECT_TRUE(host->view());
   }
 
   {
@@ -43,7 +42,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionViewHostFactoryTest, CreateExtensionHosts) {
     EXPECT_EQ(extension.get(), host->extension());
     EXPECT_EQ(browser_context, host->browser_context());
     EXPECT_EQ(VIEW_TYPE_EXTENSION_DIALOG, host->extension_host_type());
-    EXPECT_TRUE(host->view());
   }
 }
 

chrome/browser/ui/views/extensions/extension_dialog.cc

@@ -123,7 +123,7 @@ const views::Widget* ExtensionDialog::GetWidget() const {
 
 views::View* ExtensionDialog::GetContentsView() {
   if (!extension_view_) {
-    extension_view_ = static_cast<ExtensionViewViews*>(host_->view());
+    extension_view_ = new ExtensionViewViews(host_.get());  // Owned by caller.
 
     // Show a white background while the extension loads.  This is prettier than
     // flashing a black unfilled window frame.

chrome/browser/ui/views/extensions/extension_popup.cc

@@ -129,6 +129,10 @@ void ExtensionPopup::OnExtensionUnloaded(
     const extensions::Extension* extension,
     extensions::UnloadedExtensionReason reason) {
   if (extension->id() == host_->extension_id()) {
+    // To ensure |extension_view_| cannot receive any messages that cause it to
+    // try to access the host during Widget closure, destroy it immediately.
+    RemoveChildViewT(extension_view_);
+
     host_.reset();
     GetWidget()->Close();
   }
@@ -195,7 +199,7 @@ ExtensionPopup::ExtensionPopup(
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
   extension_view_ =
-      AddChildView(static_cast<ExtensionViewViews*>(host_.get()->view()));
+      AddChildView(std::make_unique<ExtensionViewViews>(host_.get()));
   extension_view_->set_container(this);
 
   // See comments in OnWidgetActivationChanged().

chrome/browser/ui/views/extensions/extension_view_views.cc

@@ -28,6 +28,7 @@
 ExtensionViewViews::ExtensionViewViews(extensions::ExtensionViewHost* host)
     : views::WebView(host->browser() ? host->browser()->profile() : nullptr),
       host_(host) {
+  host_->set_view(this);
   SetWebContents(host_->web_contents());
   if (host->extension_host_type() == extensions::VIEW_TYPE_EXTENSION_POPUP) {
     EnableSizingFromWebContents(
@@ -119,17 +120,3 @@ void ExtensionViewViews::OnWebContentsAttached() {
   host_->CreateRenderViewSoon();
   SetVisible(false);
 }
-
-namespace extensions {
-
-// static
-std::unique_ptr<ExtensionView> ExtensionViewHost::CreateExtensionView(
-    ExtensionViewHost* host) {
-  auto view = std::make_unique<ExtensionViewViews>(host);
-  // We own |view_|, so don't auto delete when it's removed from the view
-  // hierarchy.
-  view->set_owned_by_client();
-  return std::move(view);
-}
-
-}  // namespace extensions

chrome/browser/ui/views/extensions/extension_view_views.h

@@ -63,7 +63,6 @@ class ExtensionViewViews : public views::WebView,
   void PreferredSizeChanged() override;
   void OnWebContentsAttached() override;
 
-  // Note that host_ owns view
   extensions::ExtensionViewHost* host_;
 
   // What we should set the preferred width to once the ExtensionViewViews has