Use DownloadItem::GetState() in chrome/

chrome/browser/ui/webui/downloads_dom_handler.cc

 // Copyright (c) 2012 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/ui/webui/downloads_dom_handler.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/basictypes.h"
@@ skipping 128 matching lines @@
   file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name);
   file_value->SetString("file_name", file_name);
   file_value->SetString("url", download_item->GetURL().spec());
   file_value->SetBoolean("otr", incognito);
   file_value->SetInteger("total", static_cast<int>(
       download_item->GetTotalBytes()));
   file_value->SetBoolean("file_externally_removed",
                          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");
-    }
+  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->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->SetInteger(
+          "percent", static_cast<int>(download_item->PercentComplete()));
+      file_value->SetInteger(
+          "received", static_cast<int>(download_item->GetReceivedBytes()));
+      break;
 
-    file_value->SetString("progress_status_text",
-        download_util::GetProgressStatusText(download_item));
+    case content::DownloadItem::INTERRUPTED:
+      file_value->SetString("state", "INTERRUPTED");
 
-    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->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;
 }
 
 // Filters out extension downloads and downloads that don't have a filename yet.
 bool IsDownloadDisplayable(const content::DownloadItem& item) {
   return (!download_crx_util::IsExtensionDownload(item) &&
           !item.IsTemporary() &&
           !item.GetFileNameToReportUser().empty() &&
@@ skipping 129 matching lines @@
   content::DownloadItem* file = GetDownloadByValue(args);
   if (file)
     file->OpenDownload();
 }
 
 void DownloadsDOMHandler::HandleDrag(const base::ListValue* args) {
   CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_DRAG);
   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);
   gfx::NativeView view = web_contents->GetView()->GetNativeView();
   {
     // Enable nested tasks during DnD, while |DragDownload()| blocks.
     base::MessageLoop::ScopedNestableTaskAllower allow(
         base::MessageLoop::current());
     download_util::DragDownload(file, icon, view);
   }
@@ skipping 179 matching lines @@
 }
 
 void DownloadsDOMHandler::CallDownloadsList(const base::ListValue& downloads) {
   web_ui()->CallJavascriptFunction("downloadsList", downloads);
 }
 
 void DownloadsDOMHandler::CallDownloadUpdated(
     const base::ListValue& download_item) {
   web_ui()->CallJavascriptFunction("downloadUpdated", download_item);
 }