Support restricting browsing data removal for downloads by origin.

content/browser/download/download_manager_impl.cc

Index: content/browser/download/download_manager_impl.cc
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index ce39612f65fec16ac9b6887d8034575d3c119ba1..f22437939ae85b0df354d8c18416e8a3e3728e4d 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -44,6 +44,7 @@
 #include "net/base/request_priority.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/url_request/url_request_context.h"
+#include "url/origin.h"
 
 namespace content {
 namespace {
@@ -565,8 +566,21 @@ void DownloadManagerImpl::DownloadRemoved(DownloadItemImpl* download) {
   delete download;
 }
 
-int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
-                                                base::Time remove_end) {
+// Determines if the given URL should be removed. This is the case for URLs
+// that are same-origin with the given origin or where the given origin is
+// unique. The latter marks a sitation where we intend to clear all URLs.
+static bool IsRemovableURL(const GURL& url,

[Bernhard Bauer, 2015/08/04 07:49:20]

Move this into an anonymous namespace instead of declaring it as static.

[Timo Reimann, 2015/08/04 15:00:26]

Originally, I had it in an anonymous namespace and accidentally moved it out
again when I addressed phajdan.jr's concern to locate the function closer to the
call site.

I could wrap it in an anonymous namespace again where it is now, but that would
lead to the file having two anonymous namespaces (one at the very top and
another one just to accommodate IsRemovableURL). Not sure if this is common
practice in Chromium though.

If it's not, then I suggest we wait until I have resolved rdsmith's points. This
will likely end up in another refactoring of DownloadManager and friends,
thereby possibly rendering the function in question obsolete. WDYT?

[Bernhard Bauer, 2015/08/04 15:05:06]

I have seen separate anonymous namespaces; I don't think that's a big problem
(unless you subscribe to the notion that everything local to the file should
come before the stuff that's declared in the header, but that would directly
contradict what Paweł suggested, and it would equally apply to static vs.
anonymous namespace).

+                           const url::Origin& origin_to_clear) {
+  url::Origin url_origin(url);
+
+  return origin_to_clear.unique() ||
+         origin_to_clear.IsSameOriginWith(url_origin);
+}
+
+int DownloadManagerImpl::RemoveDownloadsBetween(
+    const url::Origin& origin_to_clear,
+    base::Time remove_begin,
+    base::Time remove_end) {
   int count = 0;
   DownloadMap::const_iterator it = downloads_.begin();
   while (it != downloads_.end()) {
@@ -575,7 +589,8 @@ int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
     // Increment done here to protect against invalidation below.
     ++it;
 
-    if (download->GetStartTime() >= remove_begin &&
+    if (IsRemovableURL(download->GetURL(), origin_to_clear) &&
+        download->GetStartTime() >= remove_begin &&
         (remove_end.is_null() || download->GetStartTime() < remove_end) &&
         (download->GetState() != DownloadItem::IN_PROGRESS)) {
       // Erases the download from downloads_.
@@ -587,12 +602,13 @@ int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
 }
 
 int DownloadManagerImpl::RemoveDownloads(base::Time remove_begin) {
-  return RemoveDownloadsBetween(remove_begin, base::Time());
+  return RemoveDownloadsBetween(url::Origin(), remove_begin, base::Time());
 }
 
 int DownloadManagerImpl::RemoveAllDownloads() {
   // The null times make the date range unbounded.
-  int num_deleted = RemoveDownloadsBetween(base::Time(), base::Time());
+  int num_deleted =
+      RemoveDownloadsBetween(url::Origin(), base::Time(), base::Time());
   RecordClearAllSize(num_deleted);
   return num_deleted;
 }