Modifying some downloads hooks to act per-window (so that incognito windows...

Modifying some downloads hooks to act per-window (so that incognito windows can be correctly targetted) and updating some surrounding test cases.

BUG=59259

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

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

chrome/browser/automation/testing_automation_provider.cc

@@ -2521,14 +2521,15 @@ void TestingAutomationProvider::GetDownloadsInfo(Browser* browser,
                                                  IPC::Message* reply_message) {
   AutomationJSONReply reply(this, reply_message);
 
-  if (!profile_->HasCreatedDownloadManager()) {
+  if (!browser->profile()->HasCreatedDownloadManager()) {
       reply.SendError("no download manager");
       return;
   }
 
   scoped_ptr<DictionaryValue> return_value(new DictionaryValue);
   std::vector<DownloadItem*> downloads;
-  profile_->GetDownloadManager()->GetAllDownloads(FilePath(), &downloads);
+  browser->profile()->GetDownloadManager()->
+      GetAllDownloads(FilePath(), &downloads);
 
   ListValue* list_of_downloads = new ListValue;
   for (std::vector<DownloadItem*>::iterator it = downloads.begin();
@@ -2548,18 +2549,18 @@ void TestingAutomationProvider::WaitForDownloadsToComplete(
     IPC::Message* reply_message) {
 
   // Look for a quick return.
-  if (!profile_->HasCreatedDownloadManager()) {
+  if (!browser->profile()->HasCreatedDownloadManager()) {
     // No download manager.
     AutomationJSONReply(this, reply_message).SendSuccess(NULL);
     return;
   }
   std::vector<DownloadItem*> downloads;
-  profile_->GetDownloadManager()->GetCurrentDownloads(FilePath(), &downloads);
+  browser->profile()->GetDownloadManager()->
+      GetCurrentDownloads(FilePath(), &downloads);
   if (downloads.empty()) {
     AutomationJSONReply(this, reply_message).SendSuccess(NULL);
     return;
   }
-
   // The observer owns itself.  When the last observed item pings, it
   // deletes itself.
   AutomationProviderDownloadItemObserver* item_observer =
@@ -2602,7 +2603,7 @@ void TestingAutomationProvider::PerformActionOnDownload(
   int id;
   std::string action;
 
-  if (!profile_->HasCreatedDownloadManager()) {
+  if (!browser->profile()->HasCreatedDownloadManager()) {
     AutomationJSONReply(this, reply_message).SendError("No download manager.");
     return;
   }
@@ -2612,7 +2613,7 @@ void TestingAutomationProvider::PerformActionOnDownload(
     return;
   }
 
-  DownloadManager* download_manager = profile_->GetDownloadManager();
+  DownloadManager* download_manager = browser->profile()->GetDownloadManager();
   DownloadItem* selected_item = GetDownloadItemFromId(id, download_manager);
   if (!selected_item) {
     AutomationJSONReply(this, reply_message).SendError(

chrome/test/functional/downloads.py

@@ -133,13 +133,15 @@ class DownloadsTest(pyauto.PyUITest):
     downloaded_pkg = os.path.join(self.GetDownloadDirectory().value(),
                                   'a_zip_file.zip')
     self._ClearLocalDownloadState(downloaded_pkg)
+    self.RunCommand(pyauto.IDC_NEW_INCOGNITO_WINDOW)
+
+    # Trigger download and wait in new incognito window.
+    self.DownloadAndWaitForStart(file_url, 1)
+    self.WaitForAllDownloadsToComplete(1)
+    incognito_downloads = self.GetDownloadsInfo(1).Downloads()
 
-    self.RunCommand(pyauto.IDC_NEW_INCOGNITO_WINDOW)  # open incognito window
-    # Downloads from incognito window do not figure in GetDownloadsInfo()
-    # since the download manager's list doesn't contain it.
-    # Using WaitUntil is the only resort.
-    self.NavigateToURL(file_url, 1, 0)
-    self.assertTrue(self.WaitUntil(lambda: os.path.exists(downloaded_pkg)))
+    # Verify that download info exists in the correct profile.
+    self.assertEqual(len(incognito_downloads), 1)
     self.assertTrue(self._EqualFileContents(file_path, downloaded_pkg))
     self.assertTrue(self.IsDownloadShelfVisible(1))
 
@@ -518,7 +520,7 @@ class DownloadsTest(pyauto.PyUITest):
     self.PerformActionOnDownload(id, 'open')
     self.WaitForAllDownloadsToComplete()
     unzip_file_name = downloaded_pkg + '.cpgz'
-    # Verify that the file was correctly downloaded
+    # Verify that the file was correctly downloaded.
     self.assertTrue(self.WaitUntil(lambda: os.path.exists(unzip_file_name)),
                     'Unzipped folder %s missing.' % unzip_file_name)
     self.assertTrue(os.path.exists(downloaded_pkg),
@@ -548,7 +550,7 @@ class DownloadsTest(pyauto.PyUITest):
       return old_percentage == 100 or percent > old_percentage,
     self.assertTrue(self.WaitUntil(_PercentInc),
         msg='Download percentage value is not increasing')
-    # Once download is completed, percentage is 100
+    # Once download is completed, percentage is 100.
     self.WaitForAllDownloadsToComplete()
     downloads = self.GetDownloadsInfo().Downloads()
     self.assertEqual(downloads[0]['PercentComplete'], 100,
@@ -570,17 +572,21 @@ class DownloadsTest(pyauto.PyUITest):
                                         'a_zip_file (1).zip')
     self._ClearLocalDownloadState(downloaded_pkg_regul)
     self._ClearLocalDownloadState(downloaded_pkg_incog)
-    self.NavigateToURL(file_url, 0, 0)
+
+    self.DownloadAndWaitForStart(file_url, 0)
+    self.WaitForAllDownloadsToComplete(0)
+
     self.RunCommand(pyauto.IDC_NEW_INCOGNITO_WINDOW)
-    self.NavigateToURL(file_url, 1, 0)
-    self.WaitForAllDownloadsToComplete()
+    self.DownloadAndWaitForStart(file_url, 1)
+    self.WaitForAllDownloadsToComplete(1)
 
-    # Verify download in regular Window.
+    # Verify download in regular window.
     self.assertTrue(os.path.exists(downloaded_pkg_regul))
     self.assertTrue(self._EqualFileContents(file_path, downloaded_pkg_regul))
 
-    # Verify download in Incognito Window.
-    # WaitForAllDownloadsToComplete does not wait for incognito downloads
+    # Verify download in incognito window.
+    # bug 69738 WaitForAllDownloadsToComplete is flaky for this test case.
+    # Using extra WaitUntil until this is resolved.
     self.assertTrue(self.WaitUntil(
         lambda: os.path.exists(downloaded_pkg_incog)))
     self.assertTrue(self._EqualFileContents(file_path, downloaded_pkg_incog))

chrome/test/pyautolib/pyauto.py

@@ -388,7 +388,7 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     """
     return bookmark_model.BookmarkModel(self._GetBookmarksAsJSON())
 
-  def GetDownloadsInfo(self):
+  def GetDownloadsInfo(self, windex=0):
     """Return info about downloads.
 
     This includes all the downloads recognized by the history system.
@@ -397,7 +397,8 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
       an instance of downloads_info.DownloadInfo
     """
     return download_info.DownloadInfo(
-        self._SendJSONRequest(0, json.dumps({'command': 'GetDownloadsInfo'})))
+        self._SendJSONRequest(
+            windex, json.dumps({'command': 'GetDownloadsInfo'})))
 
   def GetOmniboxInfo(self, windex=0):
     """Return info about Omnibox.
@@ -657,13 +658,14 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     }
     self._GetResultFromJSONRequest(cmd_dict)
 
-  def WaitForAllDownloadsToComplete(self):
+  def WaitForAllDownloadsToComplete(self, windex=0):
     """Wait for all downloads to complete.
 
     Note: This method does not work for dangerous downloads. Use
     WaitForGivenDownloadsToComplete (below) instead.
     """
-    self._GetResultFromJSONRequest({'command': 'WaitForAllDownloadsToComplete'})
+    cmd_dict = {'command': 'WaitForAllDownloadsToComplete'}
+    self._GetResultFromJSONRequest(cmd_dict, windex=windex)
 
   def WaitForDownloadToComplete(self, download_path, timeout=-1):
     """Wait for the given downloads to complete.
@@ -728,7 +730,7 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     }
     return self._GetResultFromJSONRequest(cmd_dict, windex=window_index)
 
-  def DownloadAndWaitForStart(self, file_url):
+  def DownloadAndWaitForStart(self, file_url, windex=0):
     """Trigger download for the given url and wait for downloads to start.
 
     It waits for download by looking at the download info from Chrome, so
@@ -738,11 +740,16 @@ class PyUITest(pyautolib.PyUITestBase, unittest.TestCase):
     download, it's fine to start another one even if the first one hasn't
     completed.
     """
-    num_downloads = len(self.GetDownloadsInfo().Downloads())
-    self.NavigateToURL(file_url)  # Trigger download.
+    try:
+      num_downloads = len(self.GetDownloadsInfo(windex).Downloads())
+    except JSONInterfaceError:
+      num_downloads = 0
+
+    self.NavigateToURL(file_url, windex)  # Trigger download.
     # It might take a while for the download to kick in, hold on until then.
     self.assertTrue(self.WaitUntil(
-        lambda: len(self.GetDownloadsInfo().Downloads()) == num_downloads + 1))
+        lambda: len(self.GetDownloadsInfo(windex).Downloads()) >
+                num_downloads))
 
   def SetWindowDimensions(
       self, x=None, y=None, width=None, height=None, windex=0):

chrome/test/pyautolib/pyautolib.cc

@@ -74,6 +74,11 @@ void PyUITestBase::NavigateToURL(const char* url_string) {
   UITestBase::NavigateToURL(url);
 }
 
+void PyUITestBase::NavigateToURL(const char* url_string, int window_index) {
+  GURL url(url_string);
+  UITestBase::NavigateToURL(url, window_index);
+}
+
 void PyUITestBase::NavigateToURL(
     const char* url_string, int window_index, int tab_index) {
   GURL url(url_string);

chrome/test/pyautolib/pyautolib.h

@@ -54,6 +54,9 @@ class PyUITestBase : public UITestBase {
   // Navigate to the given URL in the active tab. Blocks until page loaded.
   void NavigateToURL(const char* url_string);
 
+  // Navigate to the given URL in the active tab in the given window.
+  void NavigateToURL(const char* url_string, int window_index);
+
   // Navigate to the given URL in given tab in the given window.
   // Blocks until page loaded.
   void NavigateToURL(const char* url_string, int window_index, int tab_index);

chrome/test/pyautolib/pyautolib.i

@@ -220,6 +220,7 @@ class PyUITestBase {
            "if it's not active already. Blocks until page has loaded.")
       NavigateToURL;
   void NavigateToURL(const char* url_string);
+  void NavigateToURL(const char* url_string, int window_index);
   void NavigateToURL(const char* url_string, int window_index, int tab_index);
 
   %feature("docstring", "Reload the active tab in the given window (or first "

chrome/test/ui/ui_test.cc

@@ -273,6 +273,10 @@ void UITestBase::NavigateToURL(const GURL& url) {
   NavigateToURL(url, 0, GetActiveTabIndex(0));
 }
 
+void UITestBase::NavigateToURL(const GURL& url, int window_index) {
+  NavigateToURL(url, window_index, GetActiveTabIndex(window_index));
+}
+
 void UITestBase::NavigateToURL(const GURL& url, int window_index, int
     tab_index) {
   NavigateToURLBlockUntilNavigationsComplete(url, 1, window_index, tab_index);

chrome/test/ui/ui_test.h

@@ -120,6 +120,9 @@ class UITestBase {
   // This method doesn't return until the navigation is complete.
   void NavigateToURL(const GURL& url);
 
+  // Navigate to the given URL in the active tab of the given app window.
+  void NavigateToURL(const GURL& url, int window_index);
+
   // Same as above, except in the given tab and window.
   void NavigateToURL(const GURL& url, int window_index, int tab_index);