downloads: prevent "Clear all" from removing in progress downloads.

R=benjhayden@chromium.org
BUG=440741

Review URL: https://codereview.chromium.org/795353002

Cr-Commit-Position: refs/heads/master@{#308196}

chrome/browser/ui/webui/downloads_dom_handler.cc

@@ -532,8 +532,8 @@ void DownloadsDOMHandler::HandleClearAll(const base::ListValue* args) {
   CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_CLEAR_ALL);
 
   std::vector<content::DownloadItem*> downloads;
-  if (main_notifier_.GetManager())
-    main_notifier_.GetManager()->GetAllDownloads(&downloads);
+  if (GetMainNotifierManager())
+    GetMainNotifierManager()->GetAllDownloads(&downloads);
   if (original_notifier_ && original_notifier_->GetManager())
     original_notifier_->GetManager()->GetAllDownloads(&downloads);
   RemoveDownloads(downloads);
@@ -543,8 +543,10 @@ void DownloadsDOMHandler::RemoveDownloads(
     const std::vector<content::DownloadItem*>& to_remove) {
   std::set<uint32> ids;
   for (auto* download : to_remove) {
-    if (IsRemoved(*download))
+    if (IsRemoved(*download) ||
+        download->GetState() == content::DownloadItem::IN_PROGRESS) {
       continue;
+    }
 
     DownloadsDOMHandlerData::Create(download)->set_is_removed(true);
     ids.insert(download->GetId());
@@ -579,6 +581,10 @@ void DownloadsDOMHandler::ScheduleSendCurrentDownloads() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+content::DownloadManager* DownloadsDOMHandler::GetMainNotifierManager() {
+  return main_notifier_.GetManager();
+}
+
 void DownloadsDOMHandler::SendCurrentDownloads() {
   update_scheduled_ = false;
   content::DownloadManager::DownloadVector all_items, filtered_items;

chrome/browser/ui/webui/downloads_dom_handler.h

@@ -103,6 +103,9 @@ class DownloadsDOMHandler : public content::WebUIMessageHandler,
   // iteration. Protected rather than private for use in tests.
   void ScheduleSendCurrentDownloads();
 
+  // Protected for testing.
+  virtual content::DownloadManager* GetMainNotifierManager();
+
  private:
   // Shorthand for |observing_items_|, which tracks all items that this is
   // observing so that RemoveObserver will be called for all of them.

chrome/browser/ui/webui/downloads_dom_handler_browsertest.cc

@@ -14,6 +14,8 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/mock_download_item.h"
+#include "content/public/test/mock_download_manager.h"
 #include "content/public/test/test_utils.h"
 
 namespace {
@@ -53,7 +55,8 @@ class MockDownloadsDOMHandler : public DownloadsDOMHandler {
   explicit MockDownloadsDOMHandler(content::DownloadManager* dlm)
     : DownloadsDOMHandler(dlm),
       waiting_list_(false),
-      waiting_updated_(false) {
+      waiting_updated_(false),
+      manager_(nullptr) {
   }
   ~MockDownloadsDOMHandler() override {}
 
@@ -81,6 +84,8 @@ class MockDownloadsDOMHandler : public DownloadsDOMHandler {
   void reset_downloads_list() { downloads_list_.reset(); }
   void reset_download_updated() { download_updated_.reset(); }
 
+  void set_manager(content::DownloadManager* manager) { manager_ = manager; }
+
  protected:
   content::WebContents* GetWebUIWebContents() override { return NULL; }
 
@@ -102,11 +107,16 @@ class MockDownloadsDOMHandler : public DownloadsDOMHandler {
     }
   }
 
+  content::DownloadManager* GetMainNotifierManager() override {
+    return manager_ ? manager_ : DownloadsDOMHandler::GetMainNotifierManager();
+  }
+
  private:
   scoped_ptr<base::ListValue> downloads_list_;
   scoped_ptr<base::ListValue> download_updated_;
   bool waiting_list_;
   bool waiting_updated_;
+  content::DownloadManager* manager_;  // weak.
 
   DISALLOW_COPY_AND_ASSIGN(MockDownloadsDOMHandler);
 };
@@ -223,6 +233,23 @@ IN_PROC_BROWSER_TEST_F(DownloadsDOMHandlerTest, RemoveOneItem) {
   EXPECT_EQ(0, static_cast<int>(mock_handler_->downloads_list()->GetSize()));
 }
 
+IN_PROC_BROWSER_TEST_F(DownloadsDOMHandlerTest, ClearAllSkipsInProgress) {
+  content::MockDownloadManager manager;
+  mock_handler_->set_manager(&manager);
+
+  content::MockDownloadItem item;
+  EXPECT_CALL(item, GetState()).WillRepeatedly(
+      testing::Return(content::DownloadItem::IN_PROGRESS));
+  EXPECT_CALL(item, UpdateObservers()).Times(0);
+
+  std::vector<content::DownloadItem*> items;
+  items.push_back(&item);
+  EXPECT_CALL(manager, GetAllDownloads(testing::_)).WillOnce(
+      testing::SetArgPointee<0>(items));
+
+  mock_handler_->HandleClearAll(NULL);
+}
+
 // Tests that DownloadsDOMHandler detects new downloads and relays them to the
 // renderer.
 // crbug.com/159390: This test fails when daylight savings time ends.