Fix for hook UninstallExtensionById, and PyAuto function to uninstall an app.

Revised the UninstallExtensionById automation hook to properly wait for
the EXTENSION_UNINSTALLED notification before sending a reply.  Also
implemented a PyAuto function "UninstallApp", one of the functions needed
to automate the context menu for an app on Chrome's new tab page.

BUG=75205
TEST=none

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

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

8 files changed, 134 insertions, 36 deletions

diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 94b80a3..0447445 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -504,6 +504,39 @@ void ExtensionInstallNotificationObserver::SendResponse(
   automation_->Send(reply_message_.release());
 }
 
+ExtensionUninstallObserver::ExtensionUninstallObserver(
+    AutomationProvider* automation,
+    IPC::Message* reply_message,
+    const std::string& id)
+    : automation_(automation->AsWeakPtr()),
+      reply_message_(reply_message),
+      id_(id) {
+  registrar_.Add(this, NotificationType::EXTENSION_UNINSTALLED,
+                 NotificationService::AllSources());
+}
+
+ExtensionUninstallObserver::~ExtensionUninstallObserver() {
+}
+
+void ExtensionUninstallObserver::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  if (!automation_) {
+    delete this;
+    return;
+  }
+
+  DCHECK(type == NotificationType::EXTENSION_UNINSTALLED);
+  UninstalledExtensionInfo* info =
+      Details<UninstalledExtensionInfo>(details).ptr();
+  if (id_ == info->extension_id) {
+    AutomationJSONReply(automation_, reply_message_.release())
+        .SendSuccess(NULL);
+    delete this;
+  }
+}
+
 ExtensionReadyNotificationObserver::ExtensionReadyNotificationObserver(
     ExtensionProcessManager* manager, AutomationProvider* automation, int id,
     IPC::Message* reply_message)
@@ -740,7 +773,7 @@ BrowserCountChangeNotificationObserver::BrowserCountChangeNotificationObserver(
 }
 
 BrowserCountChangeNotificationObserver::
-~BrowserCountChangeNotificationObserver() {}
+    ~BrowserCountChangeNotificationObserver() {}
 
 void BrowserCountChangeNotificationObserver::Observe(
     NotificationType type,
@@ -1242,7 +1275,7 @@ AutomationProviderBookmarkModelObserver(
 }
 
 AutomationProviderBookmarkModelObserver::
-~AutomationProviderBookmarkModelObserver() {
+    ~AutomationProviderBookmarkModelObserver() {
   model_->RemoveObserver(this);
 }
 
@@ -1274,7 +1307,7 @@ AutomationProviderDownloadItemObserver::AutomationProviderDownloadItemObserver(
 }
 
 AutomationProviderDownloadItemObserver::
-~AutomationProviderDownloadItemObserver() {}
+    ~AutomationProviderDownloadItemObserver() {}
 
 void AutomationProviderDownloadItemObserver::OnDownloadUpdated(
     DownloadItem* download) {
@@ -1307,7 +1340,7 @@ AutomationProviderDownloadUpdatedObserver(
 }
 
 AutomationProviderDownloadUpdatedObserver::
-~AutomationProviderDownloadUpdatedObserver() {}
+    ~AutomationProviderDownloadUpdatedObserver() {}
 
 void AutomationProviderDownloadUpdatedObserver::OnDownloadUpdated(
     DownloadItem* download) {
@@ -1355,7 +1388,7 @@ AutomationProviderDownloadModelChangedObserver(
 }
 
 AutomationProviderDownloadModelChangedObserver::
-~AutomationProviderDownloadModelChangedObserver() {}
+    ~AutomationProviderDownloadModelChangedObserver() {}
 
 void AutomationProviderDownloadModelChangedObserver::ModelChanged() {
   download_manager_->RemoveObserver(this);
@@ -1373,7 +1406,7 @@ AutomationProviderSearchEngineObserver::AutomationProviderSearchEngineObserver(
 }
 
 AutomationProviderSearchEngineObserver::
-~AutomationProviderSearchEngineObserver() {}
+    ~AutomationProviderSearchEngineObserver() {}
 
 void AutomationProviderSearchEngineObserver::OnTemplateURLModelChanged() {
   TemplateURLModel* url_model = provider_->profile()->GetTemplateURLModel();
@@ -1434,7 +1467,7 @@ AutomationProviderImportSettingsObserver(
 }
 
 AutomationProviderImportSettingsObserver::
-~AutomationProviderImportSettingsObserver() {}
+    ~AutomationProviderImportSettingsObserver() {}
 
 void AutomationProviderImportSettingsObserver::ImportStarted() {
 }
@@ -1461,7 +1494,7 @@ AutomationProviderGetPasswordsObserver::AutomationProviderGetPasswordsObserver(
 }
 
 AutomationProviderGetPasswordsObserver::
-~AutomationProviderGetPasswordsObserver() {}
+    ~AutomationProviderGetPasswordsObserver() {}
 
 void AutomationProviderGetPasswordsObserver::OnPasswordStoreRequestDone(
     CancelableRequestProvider::Handle handle,
@@ -1509,7 +1542,7 @@ AutomationProviderBrowsingDataObserver::AutomationProviderBrowsingDataObserver(
 }
 
 AutomationProviderBrowsingDataObserver::
-~AutomationProviderBrowsingDataObserver() {}
+    ~AutomationProviderBrowsingDataObserver() {}
 
 void AutomationProviderBrowsingDataObserver::OnBrowsingDataRemoverDone() {
   if (provider_)
@@ -2029,7 +2062,7 @@ WaitForProcessLauncherThreadToGoIdleObserver(
 }
 
 WaitForProcessLauncherThreadToGoIdleObserver::
-~WaitForProcessLauncherThreadToGoIdleObserver() {
+    ~WaitForProcessLauncherThreadToGoIdleObserver() {
 }
 
 void WaitForProcessLauncherThreadToGoIdleObserver::
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index 1802da8..1fb0c3e 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -290,6 +290,28 @@ class ExtensionInstallNotificationObserver : public NotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(ExtensionInstallNotificationObserver);
 };
 
+// Observes when an extension has been uninstalled.
+class ExtensionUninstallObserver : public NotificationObserver {
+ public:
+  ExtensionUninstallObserver(AutomationProvider* automation,
+                             IPC::Message* reply_message,
+                             const std::string& id);
+  virtual ~ExtensionUninstallObserver();
+
+  // Implementation of NotificationObserver.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  NotificationRegistrar registrar_;
+  base::WeakPtr<AutomationProvider> automation_;
+  scoped_ptr<IPC::Message> reply_message_;
+  std::string id_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionUninstallObserver);
+};
+
 // Observes when an extension has finished loading and is ready for use. Also
 // checks for possible install errors.
 class ExtensionReadyNotificationObserver : public NotificationObserver {
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index b93d9a0..d718d38 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -3859,20 +3859,32 @@ void TestingAutomationProvider::UninstallExtensionById(
     Browser* browser,
     DictionaryValue* args,
     IPC::Message* reply_message) {
-  AutomationJSONReply reply(this, reply_message);
   std::string id;
   if (!args->GetString("id", &id)) {
-    reply.SendError("Must include string id.");
+    AutomationJSONReply(this, reply_message).SendError(
+        "Must include string id.");
     return;
   }
   ExtensionService* service = profile()->GetExtensionService();
   if (!service) {
-    reply.SendError("No extensions service.");
+    AutomationJSONReply(this, reply_message).SendError(
+        "No extensions service.");
+    return;
+  }
+
+  if (!service->GetExtensionById(id, true) &&
+      !service->GetTerminatedExtension(id)) {
+    // The extension ID does not correspond to any extension, whether crashed
+    // or not.
+    AutomationJSONReply(this, reply_message).SendError(base::StringPrintf(
+        "Extension does not exist: %s.", id.c_str()));
     return;
   }
-  ExtensionUnloadNotificationObserver observer;
+
+  // Wait for a notification indicating that the extension with the given ID
+  // has been uninstalled.  This observer will delete itself.
+  new ExtensionUninstallObserver(this, reply_message, id);
   service->UninstallExtension(id, false);
-  reply.SendSuccess(NULL);
 }
 
 // Sample json input:
diff --git a/chrome/test/functional/ntp.py b/chrome/test/functional/ntp.py
index 1ea8abe..e014613 100644
--- a/chrome/test/functional/ntp.py
+++ b/chrome/test/functional/ntp.py
@@ -1,5 +1,5 @@
 #!/usr/bin/python
-# Copyright (c) 2010 The Chromium Authors. All rights reserved.
+# Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
@@ -13,6 +13,12 @@ import test_utils
 class NTPTest(pyauto.PyUITest):
   """Test of the NTP."""
 
+  _EXPECTED_DEFAULT_APPS = [
+    {
+      u'name': u'Chrome Web Store'
+    }
+  ]
+
   def Debug(self):
     """Test method for experimentation.
 
@@ -335,12 +341,7 @@ class NTPTest(pyauto.PyUITest):
   def testGetAppsInNewProfile(self):
     """Ensures that the only app in a new profile is the Web Store app."""
     app_info = self.GetNTPApps()
-    expected_app_info = [
-      {
-        u'name': u'Chrome Web Store'
-      }
-    ]
-    self._VerifyAppInfo(app_info, expected_app_info)
+    self._VerifyAppInfo(app_info, self._EXPECTED_DEFAULT_APPS)
 
   def testGetAppsWhenInstallApp(self):
     """Ensures that an installed app is reflected in the app info in the NTP."""
@@ -351,12 +352,10 @@ class NTPTest(pyauto.PyUITest):
     app_info = self.GetNTPApps()
     expected_app_info = [
       {
-        u'name': u'Chrome Web Store'
-      },
-      {
         u'name': u'Countdown'
       }
     ]
+    expected_app_info.extend(self._EXPECTED_DEFAULT_APPS)
     self._VerifyAppInfo(app_info, expected_app_info)
 
   def testGetAppsWhenInstallNonApps(self):
@@ -373,13 +372,30 @@ class NTPTest(pyauto.PyUITest):
     self.assertTrue(self.SetTheme(theme_crx_file), msg='Theme install failed.')
     # Verify that no apps are listed on the NTP except for the Web Store.
     app_info = self.GetNTPApps()
+    self._VerifyAppInfo(app_info, self._EXPECTED_DEFAULT_APPS)
+
+  def testUninstallApp(self):
+    """Ensures that an uninstalled app is reflected in the NTP app info."""
+    # First, install an app and verify that it exists in the NTP app info.
+    app_crx_file = pyauto.FilePath(
+        os.path.abspath(os.path.join(self.DataDir(), 'pyauto_private', 'apps',
+                                     'countdown.crx')))
+    installed_app_id = self.InstallApp(app_crx_file)
+    self.assertTrue(installed_app_id, msg='App install failed.')
+    app_info = self.GetNTPApps()
     expected_app_info = [
       {
-        u'name': u'Chrome Web Store'
+        u'name': u'Countdown'
       }
     ]
+    expected_app_info.extend(self._EXPECTED_DEFAULT_APPS)
     self._VerifyAppInfo(app_info, expected_app_info)
 
+    # Next, uninstall the app and verify that it is removed from the NTP.
+    self.UninstallApp(installed_app_id)
+    app_info = self.GetNTPApps()
+    self._VerifyAppInfo(app_info, self._EXPECTED_DEFAULT_APPS)
+
   def _VerifyThumbnailOrMenuMode(self, actual_info, expected_info):
     """Verifies that the expected thumbnail/menu info matches the actual info.
 
diff --git a/chrome/test/pyautolib/pyauto.py b/chrome/test/pyautolib/pyauto.py
index edb4243..89de09a 100644
--- a/chrome/test/pyautolib/pyauto.py
+++ b/chrome/test/pyautolib/pyauto.py
@@ -1603,7 +1603,7 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     Uses InstallExtension().
 
     Returns:
-      True, on success.
+      The ID of the installed theme, on success.  The empty string, otherwise.
     """
     return self.InstallExtension(crx_file_path, True)
 
@@ -2144,10 +2144,19 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     return until the app is installed.
 
     Returns:
-      True, on success.
+      The ID of the installed app, on success.  The empty string, otherwise.
     """
     return self.InstallExtension(app_crx_file_path, False)
 
+  def UninstallApp(self, app_id):
+    """Uninstalls the specified app synchronously.
+
+    Args:
+      app_id: The string ID of the app to uninstall.  It can be retrieved
+              through the call to GetNTPApps above.
+    """
+    return self.UninstallExtensionById(app_id)
+
   def KillRendererProcess(self, pid):
     """Kills the given renderer process.
 
diff --git a/chrome/test/pyautolib/pyautolib.cc b/chrome/test/pyautolib/pyautolib.cc
index 7af1db8..667c597 100644
--- a/chrome/test/pyautolib/pyautolib.cc
+++ b/chrome/test/pyautolib/pyautolib.cc
@@ -201,10 +201,14 @@ int PyUITestBase::GetBrowserWindowCount() {
   return num_windows;
 }
 
-bool PyUITestBase::InstallExtension(const FilePath& crx_file, bool with_ui) {
+std::string PyUITestBase::InstallExtension(const FilePath& crx_file,
+                                           bool with_ui) {
   scoped_refptr<ExtensionProxy> proxy =
       automation()->InstallExtension(crx_file, with_ui);
-  return proxy.get() != NULL;
+  std::string id;
+  if (!proxy.get() || !proxy.get()->GetId(&id))
+    return "";
+  return id;
 }
 
 bool PyUITestBase::GetBookmarkBarVisibility() {
diff --git a/chrome/test/pyautolib/pyautolib.h b/chrome/test/pyautolib/pyautolib.h
index da52d79..5e5c22d 100644
--- a/chrome/test/pyautolib/pyautolib.h
+++ b/chrome/test/pyautolib/pyautolib.h
@@ -121,9 +121,10 @@ class PyUITestBase : public UITestBase {
   // Fetch the number of browser windows. Includes popups.
   int GetBrowserWindowCount();
 
-  // Installs the extension crx. Returns true only if extension was installed
-  // and loaded successfully. Overinstalls will fail.
-  bool InstallExtension(const FilePath& crx_file, bool with_ui);
+  // Installs the extension crx.  Returns the extension ID only if the extension
+  // was installed and loaded successfully.  Otherwise, returns the empty
+  // string.  Overinstalls will fail.
+  std::string InstallExtension(const FilePath& crx_file, bool with_ui);
 
   // Returns bookmark bar visibility state.
   bool GetBookmarkBarVisibility();
diff --git a/chrome/test/pyautolib/pyautolib.i b/chrome/test/pyautolib/pyautolib.i
index e9db1de..a35660d 100644
--- a/chrome/test/pyautolib/pyautolib.i
+++ b/chrome/test/pyautolib/pyautolib.i
@@ -359,9 +359,10 @@ class PyUITestBase {
   bool IsBrowserRunning();
 
   %feature("docstring", "Install an extension from the given file.  The file "
-           "must be specified with an absolute path. Returns True if "
-           "successfully installed and loaded.") InstallExtension;
-  bool InstallExtension(const FilePath& crx_file, bool with_ui);
+           "must be specified with an absolute path. Returns the extension ID "
+           "if successfully installed and loaded. Otherwise, returns the empty "
+           "string.") InstallExtension;
+  std::string InstallExtension(const FilePath& crx_file, bool with_ui);
 
   %feature("docstring", "Get a proxy to the browser window at the given "
                         "zero-based index.") GetBrowserWindow;