[OfflinePages] Make website settings popup rely on asynchronous API

chrome/browser/android/offline_pages/offline_page_bridge.cc

 // Copyright 2015 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/android/offline_pages/offline_page_bridge.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/android/jni_array.h"
@@ skipping 36 matching lines @@
         ConvertUTF8ToJavaString(env, offline_page.url.spec()).obj(),
         offline_page.offline_id,
         ConvertUTF8ToJavaString(env, offline_page.client_id.name_space).obj(),
         ConvertUTF8ToJavaString(env, offline_page.client_id.id).obj(),
         ConvertUTF8ToJavaString(env, offline_page.GetOfflineURL().spec()).obj(),
         offline_page.file_size, offline_page.creation_time.ToJavaTime(),
         offline_page.access_count, offline_page.last_access_time.ToJavaTime());
   }
 }
 
+
+ScopedJavaLocalRef<jobject> ToJavaOfflinePageItem(

[dewittj, 2016/05/16 21:07:46]

nit: I think I prefer this as CreateJavaOfflinePageItem.

[chili, 2016/05/16 23:06:41]

I renamed this to 'ToJavaOfflinePageItem' to be consistent with the previous
method 'ToJavaOfflinePageList'.  Should we also rename that one to
'CreateJavaOfflinePageList' ?

[dewittj, 2016/05/17 17:36:02]

I guess it is a little different to me, since ToJavaOfflinePageList actually
doesn't create a list.  I leave it up to you :)

+    JNIEnv* env,
+    const OfflinePageItem& offline_page) {
+  return Java_OfflinePageBridge_createOfflinePageItem(
+      env, ConvertUTF8ToJavaString(env, offline_page.url.spec()).obj(),
+      offline_page.offline_id,
+      ConvertUTF8ToJavaString(env, offline_page.client_id.name_space).obj(),
+      ConvertUTF8ToJavaString(env, offline_page.client_id.id).obj(),
+      ConvertUTF8ToJavaString(env, offline_page.GetOfflineURL().spec()).obj(),
+      offline_page.file_size, offline_page.creation_time.ToJavaTime(),
+      offline_page.access_count, offline_page.last_access_time.ToJavaTime());
+}
+
 void CheckPagesExistOfflineCallback(
     const ScopedJavaGlobalRef<jobject>& j_callback_obj,
     const OfflinePageModel::CheckPagesExistOfflineResult& offline_pages) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
   std::vector<std::string> offline_pages_vector;
   for (const GURL& page : offline_pages)
     offline_pages_vector.push_back(page.spec());
 
   ScopedJavaLocalRef<jobjectArray> j_result_array =
@@ skipping 33 matching lines @@
 }
 
 void DeletePageCallback(const ScopedJavaGlobalRef<jobject>& j_callback_obj,
                         OfflinePageModel::DeletePageResult result) {
   JNIEnv* env = base::android::AttachCurrentThread();
 
   Java_DeletePageCallback_onDeletePageDone(
       env, j_callback_obj.obj(), static_cast<int>(result));
 }
 
+void SingleOfflinePageItemCallback(
+    const ScopedJavaGlobalRef<jobject>& j_callback_obj,
+    const OfflinePageModel::SingleOfflinePageItemResult& result) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaGlobalRef<jobject> emptyResult;

[dewittj, 2016/05/16 21:07:46]

should this be ScopedJavaLocalRef?

[chili, 2016/05/16 23:06:41]

Done.

+
+  if (result) {
+      Java_SingleOfflinePageItemCallback_onResult(

[dewittj, 2016/05/16 21:07:46]

This code would be simpler if you just rename emptyResult to j_result and in
this if statement you can say j_result = ToJavaOfflinePageItem(env, *result);

Then you only have one place to call
Java_SingleOfflinePageItemCallback_onResult.

[chili, 2016/05/16 23:06:41]

I did start with that in a previous iteration.  Perhaps due to lack of C++fu, it
didn't compile with some error like "unable to copy ScopedJavaGlobalRef to
ScopedJavaGlobalRef."  Will try again.

+         env, j_callback_obj.obj(), ToJavaOfflinePageItem(env, *result).obj());
+  } else {
+      Java_SingleOfflinePageItemCallback_onResult(
+         env, j_callback_obj.obj(), emptyResult.obj());
+  }
+}
 
 }  // namespace
 
 static jboolean IsOfflinePagesEnabled(JNIEnv* env,
                                           const JavaParamRef<jclass>& clazz) {
   return offline_pages::IsOfflinePagesEnabled();
 }
 
 static jboolean IsOfflineBookmarksEnabled(JNIEnv* env,
                                           const JavaParamRef<jclass>& clazz) {
@@ skipping 136 matching lines @@
 }
 
 ScopedJavaLocalRef<jobject> OfflinePageBridge::GetPageByOfflineId(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     jlong offline_id) {
   const OfflinePageItem* offline_page =
       offline_page_model_->MaybeGetPageByOfflineId(offline_id);
   if (!offline_page)
     return ScopedJavaLocalRef<jobject>();
-  return CreateOfflinePageItem(env, *offline_page);
+  return ToJavaOfflinePageItem(env, *offline_page);
 }
 
 ScopedJavaLocalRef<jobject> OfflinePageBridge::GetBestPageForOnlineURL(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jstring>& online_url) {
   const OfflinePageItem* offline_page =
       offline_page_model_->MaybeGetBestPageForOnlineURL(
           GURL(ConvertJavaStringToUTF8(env, online_url)));
   if (!offline_page)
     return ScopedJavaLocalRef<jobject>();
-  return CreateOfflinePageItem(env, *offline_page);
+  return ToJavaOfflinePageItem(env, *offline_page);
 }
 
-ScopedJavaLocalRef<jobject> OfflinePageBridge::GetPageByOfflineUrl(
+void OfflinePageBridge::GetPageByOfflineUrl(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
-    const JavaParamRef<jstring>& j_offline_url) {
-  const OfflinePageItem* offline_page =
-      offline_page_model_->MaybeGetPageByOfflineURL(
-          GURL(ConvertJavaStringToUTF8(env, j_offline_url)));
-  if (!offline_page)
-    return ScopedJavaLocalRef<jobject>();
-  return CreateOfflinePageItem(env, *offline_page);
+    const JavaParamRef<jstring>& j_offline_url,
+    const JavaParamRef<jobject>& j_callback_obj) {
+  DCHECK(j_callback_obj);
+
+  ScopedJavaGlobalRef<jobject> j_callback_ref;
+  j_callback_ref.Reset(env, j_callback_obj);
+
+  offline_page_model_->GetPageByOfflineURL(
+          GURL(ConvertJavaStringToUTF8(env, j_offline_url)),
+          base::Bind(&SingleOfflinePageItemCallback, j_callback_ref));
 }
 
 void OfflinePageBridge::SavePage(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jobject>& j_callback_obj,
     const JavaParamRef<jobject>& j_web_contents,
     const JavaParamRef<jstring>& j_namespace,
     const JavaParamRef<jstring>& j_client_id) {
   DCHECK(j_callback_obj);
@@ skipping 70 matching lines @@
       total_space_bytes, free_space_bytes, reporting_after_delete);
 }
 
 void OfflinePageBridge::NotifyIfDoneLoading() const {
   if (!offline_page_model_->is_loaded())
     return;
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_OfflinePageBridge_offlinePageModelLoaded(env, java_ref_.obj());
 }
 
-ScopedJavaLocalRef<jobject> OfflinePageBridge::CreateOfflinePageItem(
-    JNIEnv* env,
-    const OfflinePageItem& offline_page) const {
-  return Java_OfflinePageBridge_createOfflinePageItem(
-      env, ConvertUTF8ToJavaString(env, offline_page.url.spec()).obj(),
-      offline_page.offline_id,
-      ConvertUTF8ToJavaString(env, offline_page.client_id.name_space).obj(),
-      ConvertUTF8ToJavaString(env, offline_page.client_id.id).obj(),
-      ConvertUTF8ToJavaString(env, offline_page.GetOfflineURL().spec()).obj(),
-      offline_page.file_size, offline_page.creation_time.ToJavaTime(),
-      offline_page.access_count, offline_page.last_access_time.ToJavaTime());
-}
 
 ScopedJavaLocalRef<jobject> OfflinePageBridge::CreateClientId(
     JNIEnv* env,
     const ClientId& client_id) const {
   return Java_OfflinePageBridge_createClientId(
       env,
       ConvertUTF8ToJavaString(env, client_id.name_space).obj(),
       ConvertUTF8ToJavaString(env, client_id.id).obj());
 }
 
 bool RegisterOfflinePageBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace android
 }  // namespace offline_pages