The code was assuming that the BalloonViewHost's lifetime would be managed by the views hierarchy, but it was only taking ownership of the BalloonViewHost's native_view.

This was manifesting as a crasher when the chrome://extensions page attempted to access an EFD whose RVH was dead.

BUG=40967

Review URL: http://codereview.chromium.org/1619012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44392 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 24 insertions, 9 deletions

diff --git a/chrome/browser/views/notifications/balloon_view.cc b/chrome/browser/views/notifications/balloon_view.cc
index 3919d5e..64d1544 100644
--- a/chrome/browser/views/notifications/balloon_view.cc
+++ b/chrome/browser/views/notifications/balloon_view.cc
@@ -295,7 +295,7 @@ void BalloonViewImpl::Show(Balloon* balloon) {
   // We carefully keep these two windows in sync to present the illusion of
   // one window to the user.
   gfx::Rect contents_rect = GetContentsRectangle();
-  html_contents_ = new BalloonViewHost(balloon);
+  html_contents_.reset(new BalloonViewHost(balloon));
   html_contents_->SetPreferredSize(gfx::Size(10000, 10000));
 
   html_container_ = Widget::CreatePopupWidget(Widget::NotTransparent,
diff --git a/chrome/browser/views/notifications/balloon_view.h b/chrome/browser/views/notifications/balloon_view.h
index 775b474..6478922 100644
--- a/chrome/browser/views/notifications/balloon_view.h
+++ b/chrome/browser/views/notifications/balloon_view.h
@@ -59,7 +59,7 @@ class BalloonViewImpl : public BalloonView,
   virtual void RepositionToBalloon();
   virtual void Close(bool by_user);
   virtual gfx::Size GetSize() const;
-  virtual BalloonHost* GetHost() const { return html_contents_; }
+  virtual BalloonHost* GetHost() const { return html_contents_.get(); }
 
  private:
   // views::View interface.
@@ -151,8 +151,8 @@ class BalloonViewImpl : public BalloonView,
   // Pointer owned by the View subclass.
   views::Widget* html_container_;
 
-  // The renderer of the HTML contents. Pointer owned by the views hierarchy.
-  BalloonViewHost* html_contents_;
+  // The renderer of the HTML contents.
+  scoped_ptr<BalloonViewHost> html_contents_;
 
   // The following factory is used to call methods at a later time.
   ScopedRunnableMethodFactory<BalloonViewImpl> method_factory_;
diff --git a/chrome/browser/views/notifications/balloon_view_host.cc b/chrome/browser/views/notifications/balloon_view_host.cc
index bfb02fa..3ca4656 100644
--- a/chrome/browser/views/notifications/balloon_view_host.cc
+++ b/chrome/browser/views/notifications/balloon_view_host.cc
@@ -47,7 +47,7 @@ class BalloonViewHostView : public views::NativeViewHost {
 
 BalloonViewHost::BalloonViewHost(Balloon* balloon)
     : BalloonHost(balloon) {
-  native_host_.reset(new BalloonViewHostView(this));
+  native_host_ = new BalloonViewHostView(this);
 }
 
 void BalloonViewHost::Init(gfx::NativeView parent_native_view) {
diff --git a/chrome/browser/views/notifications/balloon_view_host.h b/chrome/browser/views/notifications/balloon_view_host.h
index 0ed90f9..19fdffc 100644
--- a/chrome/browser/views/notifications/balloon_view_host.h
+++ b/chrome/browser/views/notifications/balloon_view_host.h
@@ -26,7 +26,7 @@ class BalloonViewHost : public BalloonHost {
 
   // Accessors.
   views::View* view() {
-    return native_host_.get();
+    return native_host_;
   }
 
   gfx::NativeView native_view() const {
@@ -46,8 +46,8 @@ class BalloonViewHost : public BalloonHost {
   // The platform-specific widget host view.  Pointer is owned by the RVH.
   RenderWidgetHostView* render_widget_host_view_;
 
-  // The views-specific host view.
-  scoped_ptr<views::NativeViewHost> native_host_;
+  // The views-specific host view. Pointer owned by the views hierarchy.
+  views::NativeViewHost* native_host_;
 
   // The handle to the parent view.
   gfx::NativeView parent_native_view_;
diff --git a/chrome/test/data/extensions/api_test/notifications/has_permission_manifest/background.html b/chrome/test/data/extensions/api_test/notifications/has_permission_manifest/background.html
index 4b43004..546460c 100644
--- a/chrome/test/data/extensions/api_test/notifications/has_permission_manifest/background.html
+++ b/chrome/test/data/extensions/api_test/notifications/has_permission_manifest/background.html
@@ -1,5 +1,6 @@
 <script>
 var notification = null;
+var chromeExtensionsUrl = "chrome://extensions/";
 
 // Shows the notification window using the specified URL.
 // Control continues at onNotificationDone().
@@ -16,7 +17,21 @@ function onNotificationDone() {
   var views = chrome.extension.getViews();
   chrome.test.assertEq(2, views.length);
   notification.cancel();
-  chrome.test.succeed();
+  
+  // This last step tests that crbug.com/40967 stays fixed.
+  var listener = function(tabId, changeInfo, tab) {
+    if (changeInfo.status != 'complete')
+      return;
+    // web_page1 loaded, open extension page to inject script
+    if (tab.url == chromeExtensionsUrl) {
+      console.log(chromeExtensionsUrl + ' finished loading.');
+      chrome.tabs.onUpdated.removeListener(listener);
+      chrome.test.succeed();
+    }
+  };
+  
+  chrome.tabs.onUpdated.addListener(listener);
+  chrome.tabs.create({ url: chromeExtensionsUrl });
 }
 
 chrome.test.runTests([