Use DownloadItem::GetState() in chrome/

chrome/browser/automation/testing_automation_provider.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/automation/testing_automation_provider.h"
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 2599 matching lines @@
 
   DownloadManager* download_manager =
       BrowserContext::GetDownloadManager(browser->profile());
   DownloadItem* selected_item = download_manager->GetDownload(id);
   if (!selected_item) {
     AutomationJSONReply(this, reply_message)
         .SendError(base::StringPrintf("No download with an id of %d\n", id));
     return;
   }
 
+  DownloadItem::DownloadState download_state = selected_item->GetState();
+
   // We need to be IN_PROGRESS for these actions.
   if ((action == "pause" || action == "resume" || action == "cancel") &&
-      !selected_item->IsInProgress()) {
+      download_state != DownloadItem::IN_PROGRESS) {

[asanka, 2013/05/28 18:43:15]

This check is necessary for 'pause' and 'resume' given that the same check is
performed below. Also, .SendError() can be called twice if this condition is
met.

You'll still need it for 'cancel'.

     AutomationJSONReply(this, reply_message)
         .SendError("Selected DownloadItem is not in progress.");
   }
 
   if (action == "open") {
     selected_item->AddObserver(
         new AutomationProviderDownloadUpdatedObserver(
             this, reply_message, true, browser->profile()->IsOffTheRecord()));
     selected_item->OpenDownload();
   } else if (action == "toggle_open_files_like_this") {
@@ skipping 11 matching lines @@
     selected_item->Remove();
   } else if (action == "decline_dangerous_download") {
     new AutomationProviderDownloadModelChangedObserver(
         this, reply_message, download_manager);
     selected_item->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
   } else if (action == "save_dangerous_download") {
     selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
         this, reply_message, false, browser->profile()->IsOffTheRecord()));
     selected_item->DangerousDownloadValidated();
   } else if (action == "pause") {
-    if (!selected_item->IsInProgress() || selected_item->IsPaused()) {
-      // Action would be a no-op; respond right from here.  No-op implies
-      // the test is poorly written or failing, so make it an error return.
-      if (!selected_item->IsInProgress()) {
-        AutomationJSONReply(this, reply_message)
-            .SendError("Action 'pause' called on download in termal state.");
-      } else {
-        AutomationJSONReply(this, reply_message)
-            .SendError("Action 'pause' called on already paused download.");
-      }
+    // If action will be a no-op; respond right from here.  No-op implies
+    // the test is poorly written or failing, so make it an error return.
+    if (download_state != DownloadItem::IN_PROGRESS) {
+      AutomationJSONReply(this, reply_message)
+          .SendError("Action 'pause' called on download in termal state.");
+    } else if (selected_item->IsPaused()) {
+      AutomationJSONReply(this, reply_message)
+          .SendError("Action 'pause' called on already paused download.");
     } else {
       selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
           this, reply_message, false, browser->profile()->IsOffTheRecord()));
       selected_item->Pause();
     }
   } else if (action == "resume") {
-    if (!selected_item->IsInProgress() || !selected_item->IsPaused()) {
-      // Action would be a no-op; respond right from here.  No-op implies
-      // the test is poorly written or failing, so make it an error return.
-      if (!selected_item->IsInProgress()) {
-        AutomationJSONReply(this, reply_message)
-            .SendError("Action 'resume' called on download in termal state.");
-      } else {
-        AutomationJSONReply(this, reply_message)
-            .SendError("Action 'resume' called on unpaused download.");
-      }
+    // If action will be a no-op; respond right from here.  No-op implies
+    // the test is poorly written or failing, so make it an error return.
+    if (download_state != DownloadItem::IN_PROGRESS) {
+      AutomationJSONReply(this, reply_message)
+          .SendError("Action 'resume' called on download in termal state.");
+    } else if (!selected_item->IsPaused()) {
+      AutomationJSONReply(this, reply_message)
+          .SendError("Action 'resume' called on unpaused download.");
     } else {
       selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
           this, reply_message, false, browser->profile()->IsOffTheRecord()));
       selected_item->Resume();
     }
   } else if (action == "cancel") {
     selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
         this, reply_message, false, browser->profile()->IsOffTheRecord()));
     selected_item->Cancel(true);
   } else {
@@ skipping 3000 matching lines @@
   if (g_browser_process)
     g_browser_process->GetAutomationProviderList()->RemoveProvider(this);
 }
 
 void TestingAutomationProvider::EnsureTabSelected(Browser* browser,
                                                   WebContents* tab) {
   TabStripModel* tab_strip = browser->tab_strip_model();
   if (tab_strip->GetActiveWebContents() != tab)
     tab_strip->ActivateTabAt(tab_strip->GetIndexOfWebContents(tab), true);
 }