The test exposed similar problems on Linux and Mac as I fixed on Windows.

The test was failing because we get a call to UpdatePageActions while we are in the process of creating PageActions, which leads to an infinite loop (create vector, add page action, update page actions, delete vector, create vector, ...).

BUG=58141
TEST=ExtensionApiTest.TestCrash57333

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

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

3 files changed, 21 insertions, 7 deletions

diff --git a/chrome/browser/cocoa/location_bar/page_action_decoration.mm b/chrome/browser/cocoa/location_bar/page_action_decoration.mm
index 337cd2f..572a65a 100644
--- a/chrome/browser/cocoa/location_bar/page_action_decoration.mm
+++ b/chrome/browser/cocoa/location_bar/page_action_decoration.mm
@@ -32,7 +32,7 @@ PageActionDecoration::PageActionDecoration(
     LocationBarViewMac* owner,
     Profile* profile,
     ExtensionAction* page_action)
-    : owner_(owner),
+    : owner_(NULL),
       profile_(profile),
       page_action_(page_action),
       tracker_(this),
@@ -59,6 +59,10 @@ PageActionDecoration::PageActionDecoration(
 
   registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
       Source<Profile>(profile_));
+
+  // We set the owner last of all so that we can determine whether we are in
+  // the process of initializing this class or not.
+  owner_ = owner;
 }
 
 PageActionDecoration::~PageActionDecoration() {}
@@ -118,7 +122,11 @@ void PageActionDecoration::OnImageLoaded(
       page_action_icons_[page_action_->default_icon_path()] = *image;
   }
 
-  owner_->UpdatePageActions();
+  // If we have no owner, that means this class is still being constructed and
+  // we should not UpdatePageActions, since it leads to the PageActions being
+  // destroyed again and new ones recreated (causing an infinite loop).
+  if (owner_)
+    owner_->UpdatePageActions();
 }
 
 void PageActionDecoration::UpdateVisibility(TabContents* contents,
diff --git a/chrome/browser/extensions/page_action_apitest.cc b/chrome/browser/extensions/page_action_apitest.cc
index f91cadc..c03c030 100644
--- a/chrome/browser/extensions/page_action_apitest.cc
+++ b/chrome/browser/extensions/page_action_apitest.cc
@@ -177,9 +177,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ShowPageActionPopup) {
 
 // Test http://crbug.com/57333: that two page action extensions using the same
 // icon for the page action icon and the extension icon do not crash.
-//
-// Disabled while I investigate. http://crbug.com/58141.
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_TestCrash57333) {
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TestCrash57333) {
   // Load extension A.
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("page_action")
                                           .AppendASCII("crash_57333")
diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc
index 8d68fa5..6f02d2c 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/gtk/location_bar_view_gtk.cc
@@ -1240,7 +1240,7 @@ void LocationBarViewGtk::ContentSettingImageViewGtk::InfoBubbleClosing(
 LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
     LocationBarViewGtk* owner, Profile* profile,
     ExtensionAction* page_action)
-    : owner_(owner),
+    : owner_(NULL),
       profile_(profile),
       page_action_(page_action),
       last_icon_pixbuf_(NULL),
@@ -1278,6 +1278,10 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
                                  Extension::kPageActionIconMaxSize),
                        ImageLoadingTracker::DONT_CACHE);
   }
+
+  // We set the owner last of all so that we can determine whether we are in
+  // the process of initializing this class or not.
+  owner_ = owner;
 }
 
 LocationBarViewGtk::PageActionViewGtk::~PageActionViewGtk() {
@@ -1377,7 +1381,11 @@ void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(
       pixbufs_[page_action_->default_icon_path()] = pixbuf;
   }
 
-  owner_->UpdatePageActions();
+  // If we have no owner, that means this class is still being constructed and
+  // we should not UpdatePageActions, since it leads to the PageActions being
+  // destroyed again and new ones recreated (causing an infinite loop).
+  if (owner_)
+    owner_->UpdatePageActions();
 }
 
 void LocationBarViewGtk::PageActionViewGtk::TestActivatePageAction() {