[Downloads] Ask DownloadHistory if a download was from history.

chrome/browser/download/download_ui_controller.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/download/download_ui_controller.h"
 
+#include "base/callback.h"
 #include "base/stl_util.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_item_model.h"
+#include "chrome/browser/download/download_service.h"
+#include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 
 #if defined(OS_ANDROID)
 #include "content/public/browser/android/download_controller_android.h"
 #else
 #include "chrome/browser/profiles/profile.h"
@@ skipping 11 matching lines @@
   DefaultUIControllerDelegateAndroid() {}
   virtual ~DefaultUIControllerDelegateAndroid() {}
 
  private:
   // DownloadUIController::Delegate
   virtual void NotifyDownloadStarting(content::DownloadItem* item) OVERRIDE;
 };
 
 void DefaultUIControllerDelegateAndroid::NotifyDownloadStarting(
     content::DownloadItem* item) {
+  // The Android DownloadController is only interested in IN_PROGRESS downloads.
+  // Ones which are INTERRUPTED etc. can't be handed over to the Android
+  // DownloadManager.
+  if (item->GetState() != content::DownloadItem::IN_PROGRESS)
+    return;
+
   // GET downloads without authentication are delegated to the Android
   // DownloadManager. Chrome is responsible for the rest.  See
   // InterceptDownloadResourceThrottle::ProcessDownloadRequest().
   content::DownloadControllerAndroid::Get()->OnDownloadStarted(item);
 }
 
 #else  // OS_ANDROID
 
 class DefaultUIControllerDelegate : public DownloadUIController::Delegate {
  public:
@@ skipping 21 matching lines @@
     browser = chrome::FindLastActiveWithProfile(profile_,
                                                 chrome::GetActiveDesktop());
   }
 
   if (browser)
     browser->ShowDownload(item);
 }
 
 #endif  // !OS_ANDROID
 
+// Predicate that returns true if the given download was not restored from
+// history. Used to construct a DownloadFilter based on a DownloadHistory.
+bool IsNewDownload(DownloadHistory* download_history,
+                   const content::DownloadItem* download_item) {
+  // Incognito profiles won't have a DownloadHistory. All downloads on such
+  // profiles are considered new.
+  return !download_history ||
+         !download_history->WasRestoredFromHistory(download_item);
+}
+
 } // namespace
 
 DownloadUIController::Delegate::~Delegate() {
 }
 
-DownloadUIController::DownloadUIController(content::DownloadManager* manager,
-                                           scoped_ptr<Delegate> delegate)
+DownloadUIController::DownloadFilter
+DownloadUIController::NewDownloadFilterFromDownloadHistory(
+    DownloadHistory* download_history) {
+  return base::Bind(&IsNewDownload, download_history);
+}
+
+DownloadUIController::DownloadUIController(
+    content::DownloadManager* manager,
+    const DownloadFilter& new_download_filter,
+    scoped_ptr<Delegate> delegate)
     : download_notifier_(manager, this),
+      new_download_filter_(new_download_filter),
       delegate_(delegate.Pass()) {
+  DCHECK(!new_download_filter_.is_null());
   if (!delegate_) {
 #if defined(OS_ANDROID)
     delegate_.reset(new DefaultUIControllerDelegateAndroid());
 #else
     // The delegate should not be invoked after the profile has gone away. This
     // should be the case since DownloadUIController is owned by
     // DownloadService, which in turn is a profile keyed service.
     delegate_.reset(new DefaultUIControllerDelegate(
         Profile::FromBrowserContext(manager->GetBrowserContext())));
 #endif
   }
 }
 
 DownloadUIController::~DownloadUIController() {
 }
 
 void DownloadUIController::OnDownloadCreated(content::DownloadManager* manager,
                                              content::DownloadItem* item) {
   // If this isn't a new download, there's nothing to do.
-  if (item->GetState() != content::DownloadItem::IN_PROGRESS)
+  if (!new_download_filter_.Run(item))

[Randy Smith (Not in Mondays), 2014/04/10 18:03:23]

There's something basic I'm not understanding here.  This code behaves the same
(modulo the change in this CL) as the old code, but why is that necessary?  If
we're restoring a download from the history, don't we want it to show up in the
UI as well?

[asanka, 2014/04/17 21:16:10]

This code was refactored somewhat, but the question is still valid.

Perhaps this class should be renamed, but its purpose is to pick out downloads
that the user should be actively displayed to the user, and then figure out how
to display such downloads. There's already a mechanism for checking for all
downloads that download history is using.

     return;
 
   DownloadItemModel(item).SetShouldNotifyUI(true);
   // SavePackage downloads are created in a state where they can be shown in the
   // browser. Call OnDownloadUpdated() once to notify the UI immediately.
   OnDownloadUpdated(manager, item);
 }
 
 void DownloadUIController::OnDownloadUpdated(content::DownloadManager* manager,
                                              content::DownloadItem* item) {
   // Ignore if we've already notified the UI about |item| or if it isn't a new
   // download.
   if (!DownloadItemModel(item).ShouldNotifyUI())
     return;
 
   // Wait until the target path is determined.
   if (item->GetTargetFilePath().empty())
     return;
 
-  // Can't be complete. That would imply that we didn't receive an
-  // OnDownloadUpdated() after the target was determined.
-  DCHECK_NE(content::DownloadItem::COMPLETE, item->GetState());
-
   DownloadItemModel(item).SetShouldNotifyUI(false);
   delegate_->NotifyDownloadStarting(item);
 }