[Resumption 11/12] Support resuming interrupted downloads from the downloads shelf.

chrome/browser/download/download_shelf_context_menu.cc

Index: chrome/browser/download/download_shelf_context_menu.cc
diff --git a/chrome/browser/download/download_shelf_context_menu.cc b/chrome/browser/download/download_shelf_context_menu.cc
index 34e4bbeb90739dc72ca95e9044e3bd49a3f2a2b5..cc501326bc043308f99968312e76203e769f179f 100644
--- a/chrome/browser/download/download_shelf_context_menu.cc
+++ b/chrome/browser/download/download_shelf_context_menu.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/download/download_shelf_context_menu.h"
 
+#include "base/command_line.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_item_model.h"
@@ -15,12 +16,23 @@
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/page_navigator.h"
+#include "content/public/common/content_switches.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using content::DownloadItem;
 using extensions::Extension;
 
+namespace {
+
+// Returns true if downloads resumption is enabled.
+bool IsDownloadResumptionEnabled() {
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+  return command_line.HasSwitch(switches::kEnableDownloadResumption);
+}
+
+} // namespace
+
 DownloadShelfContextMenu::~DownloadShelfContextMenu() {
   DetachFromDownloadItem();
 }
@@ -73,9 +85,11 @@ bool DownloadShelfContextMenu::IsCommandIdEnabled(int command_id) const {
       return download_item_->CanOpenDownload() &&
           !download_crx_util::IsExtensionDownload(*download_item_);
     case CANCEL:
-      return download_item_->IsPartialDownload();
+      return download_item_->IsPartialDownload() || download_item_->CanResume();

[Randy Smith (Not in Mondays), 2013/06/04 17:08:59]

Why not IsInterrupted() instead of CanResume()?  Seems more accurate to my
uneducated eye.  

[asanka, 2013/06/04 18:52:39]

In theory, IsPartialDownload() should imply that the download can be cancelled.
Thus the additional check should be unnecessary. That'll be fixed in an upcoming
CL. I can consider landing that beforehand since that would make the code
clearer.

Without that CL, CanResume() takes into consideration whether the download can
be resumed at all. Thus the option to cancel is only offered if its expected to
do something. IsInterrupted() would unconditionally offer to cancel interrupted
downloads. I've added a check to see if resumption is enabled so that in the
absence of the flag, the previous behavior is preserved.

[Randy Smith (Not in Mondays), 2013/06/05 20:05:34]

SGTM; I'm neutral about the order the CLs are landed in.
So long-term I'm uncomfortable with this, just because I think it's confusing
for some interrupted downloads to be cancelable and some not.  I guess I'll just
say that long-term you should think about potential confusion on the user end,
and make sure that the UI gives clues about why a download can/cannot be
resumed/cancelled.

[asanka, 2013/06/05 20:09:27]

Yup. Long term all interrupted downloads will be resumable (they would at least
attempt to restart). The constraint imposed by requiring the corresponding
WebContents to stick around will go away soon. The confusion should be
temporary.

     case TOGGLE_PAUSE:
       return download_item_->GetState() == DownloadItem::IN_PROGRESS;
+    case RESUME_INTERRUPTED:
+      return download_item_->CanResume();

[Randy Smith (Not in Mondays), 2013/06/04 17:08:59]

This is true for IN_PROGRESS downloads as well.  Won't that mean that both the
TOGGLE_PAUSE and RESUME_INTERRUPTED menus show up in the shelf for IN_PROGRESS
downloads?

[asanka, 2013/06/04 18:52:39]

IsCommandIdEnabled() is only called on items that have already been added to a
menu. I've added an additional condition to make sure.

[Randy Smith (Not in Mondays), 2013/06/05 20:05:34]

Ah, sorry, missed that.  

     case DISCARD:
     case KEEP:
     case LEARN_MORE_SCANNING:
@@ -136,6 +150,13 @@ void DownloadShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
           download_item_->Pause();
       }
       break;
+    case RESUME_INTERRUPTED:
+      // Check again, in case the download was cancelled before the user clicks
+      // the menu item. Also, it's possible the download automatically resumed
+      // in the meantime.
+      if (download_item_->CanResume())
+        download_item_->Resume();

[Randy Smith (Not in Mondays), 2013/06/04 17:08:59]

Any reason not to just unilaterally resume?  It looks like the resume code is a
no-op in the cases you care about.

[asanka, 2013/06/04 18:52:39]

Done.

+      break;
     case DISCARD:
       download_item_->Remove();
       break;
@@ -195,6 +216,8 @@ string16 DownloadShelfContextMenu::GetLabelForCommandId(int command_id) const {
       if (download_item_ && download_item_->IsPaused())
         return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_RESUME_ITEM);
       return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_PAUSE_ITEM);
+    case RESUME_INTERRUPTED:
+      return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_RESUME_ITEM);
     case DISCARD:
       return l10n_util::GetStringUTF16(IDS_DOWNLOAD_MENU_DISCARD);
     case KEEP:
@@ -265,22 +288,36 @@ ui::SimpleMenuModel* DownloadShelfContextMenu::GetFinishedMenuModel() {
 }
 
 ui::SimpleMenuModel* DownloadShelfContextMenu::GetInterruptedMenuModel() {
-#if defined(OS_WIN)
-  // The Help Center article is currently Windows specific.
-  // TODO(asanka): Enable this for other platforms when the article is expanded
-  // for other platforms.
+#if !defined(OS_WIN)
+  // If resumption isn't enabled and we aren't on Windows, then none of the
+  // options here are applicable.
+  if (!IsDownloadResumptionEnabled())
+    return GetInProgressMenuModel();
+#endif
+
   if (interrupted_download_menu_model_)
     return interrupted_download_menu_model_.get();
 
   interrupted_download_menu_model_.reset(new ui::SimpleMenuModel(this));
 
+  if (IsDownloadResumptionEnabled()) {
+    interrupted_download_menu_model_->AddItemWithStringId(
+        RESUME_INTERRUPTED, IDS_DOWNLOAD_MENU_RESUME_ITEM);
+  }
+#if defined(OS_WIN)
+  // The Help Center article is currently Windows specific.
+  // TODO(asanka): Enable this for other platforms when the article is expanded
+  // for other platforms.
   interrupted_download_menu_model_->AddItemWithStringId(
       LEARN_MORE_INTERRUPTED, IDS_DOWNLOAD_MENU_LEARN_MORE_INTERRUPTED);
+#endif
+  if (IsDownloadResumptionEnabled()) {
+    interrupted_download_menu_model_->AddSeparator(ui::NORMAL_SEPARATOR);
+    interrupted_download_menu_model_->AddItemWithStringId(
+        CANCEL, IDS_DOWNLOAD_MENU_CANCEL);
+  }
 
   return interrupted_download_menu_model_.get();
-#else
-  return GetInProgressMenuModel();
-#endif
 }
 
 ui::SimpleMenuModel* DownloadShelfContextMenu::GetMaliciousMenuModel() {