[Offline Pages] Fix several infobar bugs.

chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc

Index: chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
diff --git a/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc b/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
index c6227668269a875c5938e3f501c7820814f00979..6354ef09be7a8d31a67143508e5a3135ad671bdc 100644
--- a/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
+++ b/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
@@ -8,6 +8,7 @@
 
 #include "base/android/jni_string.h"
 #include "base/bind.h"
+#include "base/debug/stack_trace.h"

[fgorski, 2016/10/10 18:08:20]

Do we need this or was it for debugging?

[dewittj, 2016/10/10 18:37:02]

Removed.

 #include "base/guid.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ -66,6 +67,19 @@ content::WebContents* GetWebContentsFromJavaTab(
   return tab->web_contents();
 }
 
+OfflinePageModel* GetOfflinePageModelFromJavaTab(

[fgorski, 2016/10/10 18:08:20]

General comment about the code in this file. I think we should move it down to
download_ui_adapter (if that is possible). I didn't think about that with the
previous patch and I don't expect changes in this one, beyond adding a TODO.

[dewittj, 2016/10/10 18:37:02]

Acknowledged.

+    const ScopedJavaGlobalRef<jobject>& j_tab_ref) {
+  content::WebContents* web_contents = GetWebContentsFromJavaTab(j_tab_ref);
+  if (!web_contents)
+    return nullptr;
+
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext())
+          ->GetOriginalProfile();
+
+  return OfflinePageModelFactory::GetForBrowserContext(profile);
+}
+
 void SavePageIfNavigatedToURL(const GURL& query_url,
                               const ScopedJavaGlobalRef<jobject>& j_tab_ref) {
   content::WebContents* web_contents = GetWebContentsFromJavaTab(j_tab_ref);
@@ -120,14 +134,55 @@ void SavePageIfNavigatedToURL(const GURL& query_url,
   item.guid = client_id.id;
   item.url = url;
 
-  OfflinePageNotificationBridge bridge;
-  bridge.NotifyDownloadProgress(item);
+  notification_bridge.NotifyDownloadProgress(item);

[fgorski, 2016/10/10 18:08:21]

Good catch.

[dewittj, 2016/10/10 18:37:03]

Acknowledged.

 
   notification_bridge.ShowDownloadingToast();
   offline_page_model->SavePage(url, client_id, 0l, std::move(archiver),
                                base::Bind(&SavePageCallback, item));
 }
 
+void OnDeletePagesForInfoBar(const GURL& query_url,
+                             const ScopedJavaGlobalRef<jobject>& j_tab_ref,
+                             DeletePageResult result) {
+  SavePageIfNavigatedToURL(query_url, j_tab_ref);
+}
+
+void OnGetPagesForDeleteDone(const GURL& query_url,

[fgorski, 2016/10/10 18:08:20]

nit: How about naming it something related to what is happening in the method
and not when?

[dewittj, 2016/10/10 18:37:03]

how about DeletePagesForOverwrite?

+                             const ScopedJavaGlobalRef<jobject>& j_tab_ref,
+                             const MultipleOfflinePageItemResult& pages) {
+  OfflinePageModel* model = GetOfflinePageModelFromJavaTab(j_tab_ref);
+  if (!model)
+    return;
+
+  std::vector<int64_t> offline_ids;
+  for (auto& page : pages) {
+    if (page.client_id.name_space == kDownloadNamespace ||
+        page.client_id.name_space == kAsyncNamespace) {
+      offline_ids.emplace_back(page.offline_id);
+    }
+  }
+
+  model->DeletePagesByOfflineId(
+      offline_ids, base::Bind(&OnDeletePagesForInfoBar, query_url, j_tab_ref));

[fgorski, 2016/10/10 18:08:21]

nit: In the spirit of the above nit, why not simply &SavePageIfNavigatedToURL as
continuation? 

[dewittj, 2016/10/10 18:37:02]

Wanted that, but there is an extra parameter and I didn't know how to ignore
that in a bound function.

+}
+
+void OnInfoBarAction(const GURL& query_url,
+                     const ScopedJavaGlobalRef<jobject>& j_tab_ref,
+                     OfflinePageInfoBarDelegate::Action action) {
+  if (action == OfflinePageInfoBarDelegate::Action::CONFIRM) {
+    SavePageIfNavigatedToURL(query_url, j_tab_ref);
+    return;
+  }
+
+  OfflinePageModel* offline_page_model =
+      GetOfflinePageModelFromJavaTab(j_tab_ref);
+  if (!offline_page_model)

[fgorski, 2016/10/10 18:08:21]

we should probably discuss with PMs the fact that if any step here fails, we
don't give any message to the user.

[dewittj, 2016/10/10 18:37:02]

I don't really expect this to be possible, since we are already going for the
original profile.  Can you think of a case where this would be encountered?  If
so I totally agree.

+    return;
+

[fgorski, 2016/10/10 18:08:20]

please add a dcheck that the action is OVERWRITE at this point. 

[dewittj, 2016/10/10 18:37:02]

Done (used switch for better compile-time checking too.)

+  offline_page_model->GetPagesByOnlineURL(
+      query_url, base::Bind(&OnGetPagesForDeleteDone, query_url, j_tab_ref));
+}
+
 void RequestQueueDuplicateCheckDone(
     const GURL& query_url,
     const ScopedJavaGlobalRef<jobject>& j_tab_ref,
@@ -148,6 +203,7 @@ void RequestQueueDuplicateCheckDone(
 
 void ModelDuplicateCheckDone(const GURL& query_url,
                              const ScopedJavaGlobalRef<jobject>& j_tab_ref,
+                             const std::string& downloads,
                              bool has_duplicates) {
   content::WebContents* web_contents = GetWebContentsFromJavaTab(j_tab_ref);
   if (!web_contents)
@@ -155,7 +211,7 @@ void ModelDuplicateCheckDone(const GURL& query_url,
 
   if (has_duplicates) {
     OfflinePageInfoBarDelegate::Create(
-        base::Bind(&SavePageIfNavigatedToURL, query_url, j_tab_ref),
+        base::Bind(&OnInfoBarAction, query_url, j_tab_ref), downloads,
         query_url.spec(), web_contents);
     return;
   }
@@ -321,7 +377,9 @@ jlong OfflinePageDownloadBridge::GetOfflineIdByGuid(
 void OfflinePageDownloadBridge::StartDownload(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    const JavaParamRef<jobject>& j_tab) {
+    const JavaParamRef<jobject>& j_tab,
+    const JavaParamRef<jstring>& j_downloads_string) {
+  std::string downloads = ConvertJavaStringToUTF8(env, j_downloads_string);
   TabAndroid* tab = TabAndroid::GetNativeTab(env, j_tab);
   if (!tab)
     return;
@@ -336,7 +394,7 @@ void OfflinePageDownloadBridge::StartDownload(
 
   OfflinePageUtils::CheckExistenceOfPagesWithURL(
       tab->GetProfile()->GetOriginalProfile(), kDownloadNamespace, url,
-      base::Bind(&ModelDuplicateCheckDone, url, j_tab_ref));
+      base::Bind(&ModelDuplicateCheckDone, url, j_tab_ref, downloads));
 }
 
 void OfflinePageDownloadBridge::CancelDownload(