Use DownloadItem::GetState() in chrome/

chrome/browser/ui/webui/downloads_dom_handler.cc

Index: chrome/browser/ui/webui/downloads_dom_handler.cc
diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc
index fc22628aeb1fd5f251c50d651c234be1764705e5..4f514954cb40f289a09a507e4d3d40e132febbdc 100644
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc
@@ -146,59 +146,70 @@ DictionaryValue* CreateDownloadItemValue(
                          download_item->GetFileExternallyRemoved());
   file_value->SetBoolean("retry", false); // Overridden below if needed.
 
-  if (download_item->IsInProgress()) {
-    if (download_item->IsDangerous()) {
-      file_value->SetString("state", "DANGEROUS");
-      // These are the only danger states that the UI is equipped to handle.
-      DCHECK(download_item->GetDangerType() ==
-                 content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE ||
-             download_item->GetDangerType() ==
-                 content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL ||
-             download_item->GetDangerType() ==
-                 content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT ||
-             download_item->GetDangerType() ==
-                 content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT ||
-             download_item->GetDangerType() ==
-                 content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST);
-      const char* danger_type_value =
-          GetDangerTypeString(download_item->GetDangerType());
-      file_value->SetString("danger_type", danger_type_value);
-    } else if (download_item->IsPaused()) {
-      file_value->SetString("state", "PAUSED");
-    } else {
-      file_value->SetString("state", "IN_PROGRESS");
-    }
-
-    file_value->SetString("progress_status_text",
-        download_util::GetProgressStatusText(download_item));
-
-    file_value->SetInteger("percent",
-        static_cast<int>(download_item->PercentComplete()));
-    file_value->SetInteger("received",
-        static_cast<int>(download_item->GetReceivedBytes()));
-  } else if (download_item->IsInterrupted()) {
-    file_value->SetString("state", "INTERRUPTED");
-
-    file_value->SetString("progress_status_text",
-        download_util::GetProgressStatusText(download_item));
-
-    file_value->SetInteger("percent",
-        static_cast<int>(download_item->PercentComplete()));
-    file_value->SetInteger("received",
-        static_cast<int>(download_item->GetReceivedBytes()));
-    file_value->SetString("last_reason_text",
-                          download_model.GetInterruptReasonText());
-    if (content::DOWNLOAD_INTERRUPT_REASON_CRASH ==
-        download_item->GetLastReason())
+  switch (download_item->GetState()) {
+    case content::DownloadItem::IN_PROGRESS:
+      if (download_item->IsDangerous()) {
+        file_value->SetString("state", "DANGEROUS");
+        // These are the only danger states that the UI is equipped to handle.
+        DCHECK(download_item->GetDangerType() ==
+               content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE ||
+               download_item->GetDangerType() ==
+               content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL ||
+               download_item->GetDangerType() ==
+               content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT ||
+               download_item->GetDangerType() ==
+               content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT ||
+               download_item->GetDangerType() ==
+               content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST);
+        const char* danger_type_value =
+            GetDangerTypeString(download_item->GetDangerType());
+        file_value->SetString("danger_type", danger_type_value);
+      } else if (download_item->IsPaused()) {
+        file_value->SetString("state", "PAUSED");
+      } else {
+        file_value->SetString("state", "IN_PROGRESS");
+      }
+
+      file_value->SetString(
+          "progress_status_text",
+          download_util::GetProgressStatusText(download_item));
+
+      file_value->SetInteger(
+          "percent", static_cast<int>(download_item->PercentComplete()));
+      file_value->SetInteger(
+          "received", static_cast<int>(download_item->GetReceivedBytes()));
+      break;
+
+    case content::DownloadItem::INTERRUPTED:
+      file_value->SetString("state", "INTERRUPTED");
+
+      file_value->SetString(
+          "progress_status_text",
+          download_util::GetProgressStatusText(download_item));
+
+      file_value->SetInteger(
+          "percent", static_cast<int>(download_item->PercentComplete()));
+      file_value->SetInteger(
+          "received", static_cast<int>(download_item->GetReceivedBytes()));
+      file_value->SetString("last_reason_text",
+                            download_model.GetInterruptReasonText());
+      if (content::DOWNLOAD_INTERRUPT_REASON_CRASH ==
+          download_item->GetLastReason())
+        file_value->SetBoolean("retry", true);
+      break;
+
+    case content::DownloadItem::CANCELLED:
+      file_value->SetString("state", "CANCELLED");
       file_value->SetBoolean("retry", true);
-  } else if (download_item->IsCancelled()) {
-    file_value->SetString("state", "CANCELLED");
-    file_value->SetBoolean("retry", true);
-  } else if (download_item->IsComplete()) {
-    DCHECK(!download_item->IsDangerous());
-    file_value->SetString("state", "COMPLETE");
-  } else {
-    NOTREACHED() << "state undefined";
+      break;
+
+    case content::DownloadItem::COMPLETE:
+      DCHECK(!download_item->IsDangerous());
+      file_value->SetString("state", "COMPLETE");
+      break;
+
+    default:

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

Nit: You could use 'case MAX_DOWNLOAD_STATE' instead of the default label. That
way the compiler will catch it if a new state is introduced, but no case label
exists to handle it.

+      NOTREACHED() << "state undefined";
   }
 
   return file_value;
@@ -348,7 +359,8 @@ void DownloadsDOMHandler::HandleDrag(const base::ListValue* args) {
   content::DownloadItem* file = GetDownloadByValue(args);
   content::WebContents* web_contents = GetWebUIWebContents();
   // |web_contents| is only NULL in the test.
-  if (!file || !web_contents || !file->IsComplete())
+  if (!file || !web_contents
+      || file->GetState() != content::DownloadItem::COMPLETE)

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

Nit: The style guide prefers wrapping after boolean operators, not before.

     return;
   gfx::Image* icon = g_browser_process->icon_manager()->LookupIconFromFilepath(
       file->GetTargetFilePath(), IconLoader::NORMAL);