Fix pyauto flakiness by waiting for notification that the renderer process'

termination has been noticed by the browser before reloading the associated
tab.
BUG=64708
TEST=none

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

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

9 files changed, 98 insertions, 11 deletions

diff --git a/chrome/browser/automation/automation_provider_observers.cc b/chrome/browser/automation/automation_provider_observers.cc
index 3d6d5be..1cec458 100644
--- a/chrome/browser/automation/automation_provider_observers.cc
+++ b/chrome/browser/automation/automation_provider_observers.cc
@@ -1549,3 +1549,20 @@ void OnNotificationBalloonCountObserver::OnBalloonCollectionChanged() {
     delete this;
   }
 }
+
+RendererProcessClosedObserver::RendererProcessClosedObserver(
+    AutomationProvider* automation,
+    IPC::Message* reply_message)
+    : automation_(automation),
+      reply_message_(reply_message) {
+  registrar_.Add(this, NotificationType::RENDERER_PROCESS_CLOSED,
+                 NotificationService::AllSources());
+}
+
+void RendererProcessClosedObserver::Observe(
+    NotificationType type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  AutomationJSONReply(automation_, reply_message_).SendSuccess(NULL);
+  delete this;
+}
diff --git a/chrome/browser/automation/automation_provider_observers.h b/chrome/browser/automation/automation_provider_observers.h
index a1773e2..dd49775 100644
--- a/chrome/browser/automation/automation_provider_observers.h
+++ b/chrome/browser/automation/automation_provider_observers.h
@@ -989,5 +989,24 @@ class OnNotificationBalloonCountObserver {
   DISALLOW_COPY_AND_ASSIGN(OnNotificationBalloonCountObserver);
 };
 
+// Allows the automation provider to wait for a RENDERER_PROCESS_CLOSED
+// notification.
+class RendererProcessClosedObserver : public NotificationObserver {
+ public:
+  RendererProcessClosedObserver(AutomationProvider* automation,
+                                IPC::Message* reply_message);
+
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  NotificationRegistrar registrar_;
+  AutomationProvider* automation_;
+  IPC::Message* reply_message_;
+
+  DISALLOW_COPY_AND_ASSIGN(RendererProcessClosedObserver);
+};
+
 
 #endif  // CHROME_BROWSER_AUTOMATION_AUTOMATION_PROVIDER_OBSERVERS_H_
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index c6570da..bd75e2e 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -10,6 +10,8 @@
 #include "base/json/json_writer.h"
 #include "base/json/string_escape.h"
 #include "base/path_service.h"
+#include "base/process.h"
+#include "base/process_util.h"
 #include "base/stringprintf.h"
 #include "base/thread_restrictions.h"
 #include "base/time.h"
@@ -2132,6 +2134,9 @@ void TestingAutomationProvider::SendJSONRequest(int handle,
   handler_map["RestoreAllNTPMostVisitedThumbnails"] =
       &TestingAutomationProvider::RestoreAllNTPMostVisitedThumbnails;
 
+  handler_map["KillRendererProcess"] =
+      &TestingAutomationProvider::KillRendererProcess;
+
   if (handler_map.find(std::string(command)) != handler_map.end()) {
     (this->*handler_map[command])(browser, dict_value, reply_message);
   } else {
@@ -4377,6 +4382,27 @@ void TestingAutomationProvider::RestoreAllNTPMostVisitedThumbnails(
   reply.SendSuccess(NULL);
 }
 
+void TestingAutomationProvider::KillRendererProcess(
+    Browser* browser,
+    DictionaryValue* args,
+    IPC::Message* reply_message) {
+  int pid;
+  if (!args->GetInteger("pid", &pid)) {
+    AutomationJSONReply(this, reply_message).
+        SendError("'pid' key missing or invalid.");
+    return;
+  }
+  base::ProcessHandle process;
+  if (!base::OpenProcessHandle(static_cast<base::ProcessId>(pid), &process)) {
+    AutomationJSONReply(this, reply_message).SendError(base::StringPrintf(
+        "Failed to open process handle for pid %d", pid));
+    return;
+  }
+  new RendererProcessClosedObserver(this, reply_message);
+  base::KillProcess(process, 0, false);
+  base::CloseProcessHandle(process);
+}
+
 void TestingAutomationProvider::WaitForTabCountToBecome(
     int browser_handle,
     int target_tab_count,
diff --git a/chrome/browser/automation/testing_automation_provider.h b/chrome/browser/automation/testing_automation_provider.h
index b1af561..ce6c127 100644
--- a/chrome/browser/automation/testing_automation_provider.h
+++ b/chrome/browser/automation/testing_automation_provider.h
@@ -744,6 +744,12 @@ class TestingAutomationProvider : public AutomationProvider,
                                           DictionaryValue* args,
                                           IPC::Message* reply_message);
 
+  // Kills the given renderer process and returns after the associated
+  // RenderProcessHost receives notification of its closing.
+  void KillRendererProcess(Browser* browser,
+                           DictionaryValue* args,
+                           IPC::Message* reply_message);
+
   void WaitForTabCountToBecome(int browser_handle,
                                int target_tab_count,
                                IPC::Message* reply_message);
diff --git a/chrome/test/functional/PYAUTO_TESTS b/chrome/test/functional/PYAUTO_TESTS
index b5950d1..566c93d 100644
--- a/chrome/test/functional/PYAUTO_TESTS
+++ b/chrome/test/functional/PYAUTO_TESTS
@@ -33,8 +33,6 @@
     'databases',
     # crbug.com/66714
     '-databases.DatabasesTest.testIncognitoDBPersistentAcrossTabs',
-    # Reloading after kill doesn't load the page, crbug.com/64708.
-    '-databases.DatabasesTest.testModificationsPersistAfterRendererCrash',
     'downloads',
     'find_in_page',
     # Turkish I problem. crbug.com/60638
diff --git a/chrome/test/functional/browser.py b/chrome/test/functional/browser.py
index 4228b9c..ac1785a 100644
--- a/chrome/test/functional/browser.py
+++ b/chrome/test/functional/browser.py
@@ -141,7 +141,7 @@ class BrowserTest(pyauto.PyUITest):
     self.NavigateToURL(flash_url)
     flash_process_id1 = self._GetFlashProcessesInfo()[0]['pid']
     self.Kill(flash_process_id1)
-    self.GetBrowserWindow(0).GetTab(0).Reload()  # Reload
+    self.ReloadActiveTab()
     flash_processes = self._GetFlashProcessesInfo()
     self.assertEqual(1, len(flash_processes))
     self.assertNotEqual(flash_process_id1, flash_processes[0]['pid'])
@@ -163,19 +163,17 @@ class BrowserTest(pyauto.PyUITest):
     # In case if we create 100 processes for 100 tabs, then we are failing.
     self.fail(msg='Got 100 renderer processes')
 
-  def testKillAndRelodRenderer(self):
+  def testKillAndReloadRenderer(self):
     """Verify that reloading of renderer is possible,
        after renderer is killed"""
     test_url = self.GetFileURLForDataPath('english_page.html')
     self.NavigateToURL(test_url)
     pid1 = self.GetBrowserInfo()['windows'][0]['tabs'][0]['renderer_pid']
-    self.Kill(pid1)
-    tab = self.GetBrowserWindow(0).GetTab(0)
-    tab.Reload()
+    self.KillRendererProcess(pid1)
+    self.ReloadActiveTab()
     pid2 = self.GetBrowserInfo()['windows'][0]['tabs'][0]['renderer_pid']
     self.assertNotEqual(pid1, pid2)
 
 
 if __name__ == '__main__':
   pyauto_functional.Main()
-
diff --git a/chrome/test/functional/databases.py b/chrome/test/functional/databases.py
index 471f07a..08fd76c 100644
--- a/chrome/test/functional/databases.py
+++ b/chrome/test/functional/databases.py
@@ -276,7 +276,8 @@ class DatabasesTest(pyauto.PyUITest):
     self.NavigateToURL(self.TEST_PAGE_URL)
     self._CreateTable()
     self._InsertRecord('1')
-    self.Kill(self.GetBrowserInfo()['windows'][0]['tabs'][0]['renderer_pid'])
+    self.KillRendererProcess(
+        self.GetBrowserInfo()['windows'][0]['tabs'][0]['renderer_pid'])
     self.ReloadActiveTab()
     self.assertEqual(['1'], self._GetRecords())
 
diff --git a/chrome/test/functional/stress.py b/chrome/test/functional/stress.py
index c64315fc..f5866ac 100644
--- a/chrome/test/functional/stress.py
+++ b/chrome/test/functional/stress.py
@@ -235,7 +235,8 @@ class StressTest(pyauto.PyUITest):
     info = self.GetBrowserInfo()
     # Kill all renderer processes
     for tab_index in range(self.GetTabCount(0)):
-      self.Kill(info['windows'][0]['tabs'][tab_index]['renderer_pid'])
+      self.KillRendererProcess(
+          info['windows'][0]['tabs'][tab_index]['renderer_pid'])
     self._ReloadAllTabs()
     self._CloseAllTabs()
 
diff --git a/chrome/test/pyautolib/pyauto.py b/chrome/test/pyautolib/pyauto.py
index 054e51e..d762bc0 100644
--- a/chrome/test/pyautolib/pyauto.py
+++ b/chrome/test/pyautolib/pyauto.py
@@ -255,7 +255,10 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
 
   @staticmethod
   def Kill(pid):
-    """Terminate the given pid."""
+    """Terminate the given pid.
+
+    If the pid refers to a renderer, use KillRendererProcess instead.
+    """
     if PyUITest.IsWin():
       subprocess.call(['taskkill.exe', '/T', '/F', '/PID', str(pid)])
     else:
@@ -1966,6 +1969,24 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     if self.GetNTPThumbnailIndex(thumbnail) == -1:
       raise NTPThumbnailNotShownError()
 
+  def KillRendererProcess(self, pid):
+    """Kills the given renderer process.
+
+    This will return only after the browser has received notice of the renderer
+    close.
+
+    Args:
+      pid: the process id of the renderer to kill
+
+    Raises:
+      pyauto_errors.JSONInterfaceError if the automation call returns an error.
+    """
+    cmd_dict = {
+        'command': 'KillRendererProcess',
+        'pid': pid
+    }
+    return self._GetResultFromJSONRequest(cmd_dict)
+
 
 class PyUITestSuite(pyautolib.PyUITestSuiteBase, unittest.TestSuite):
   """Base TestSuite for PyAuto UI tests."""