[EoS] Adds a summary image API.

This allows us to show an image for a catalog when no category ID is known.

Bug: 966024
Change-Id: I5e78f3cb41effaa0ee2ea075c87fe521e740944b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637406Reviewed-by: Cathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664767}

chrome/android/java/src/org/chromium/chrome/browser/explore_sites/ExploreSitesBridge.java

@@ -53,6 +53,10 @@ public class ExploreSitesBridge {
         nativeGetIcon(profile, siteID, callback);
     }
 
+    /**
+     * Returns a Bitmap representing a summary of the sites available in the catalog for a specific
+     * category.
+     */
     public static void getCategoryImage(
             Profile profile, int categoryID, int pixelSize, Callback<Bitmap> callback) {
         if (sCatalogForTesting != null) {
@@ -61,6 +65,16 @@ public class ExploreSitesBridge {
         nativeGetCategoryImage(profile, categoryID, pixelSize, callback);
     }
 
+    /**
+     * Returns a Bitmap representing a summary of the sites available in the catalog.
+     */
+    public static void getSummaryImage(Profile profile, int pixelSize, Callback<Bitmap> callback) {
+        if (sCatalogForTesting != null) {
+            callback.onResult(null);
+        }
+        nativeGetSummaryImage(profile, pixelSize, callback);
+    }
+
     /**
      * Causes a network request for updating the catalog.
      */
@@ -152,6 +166,9 @@ public class ExploreSitesBridge {
     private static native void nativeGetCategoryImage(
             Profile profile, int categoryID, int pixelSize, Callback<Bitmap> callback);
 
+    private static native void nativeGetSummaryImage(
+            Profile profile, int pixelSize, Callback<Bitmap> callback);
+
     private static native void nativeBlacklistSite(Profile profile, String url);
 
     private static native void nativeRecordClick(Profile profile, String url, int type);

chrome/browser/android/explore_sites/clear_catalog_task.cc

@@ -25,8 +25,8 @@ void ClearCatalogTask::Run() {
         if (!ExploreSitesSchema::InitMetaTable(db, &meta_table))
           return false;
 
-        meta_table.DeleteKey("current_catalog");
-        meta_table.DeleteKey("downloading_catalog");
+        meta_table.DeleteKey(ExploreSitesSchema::kCurrentCatalogKey);
+        meta_table.DeleteKey(ExploreSitesSchema::kDownloadingCatalogKey);
 
         sql::Statement site_clear_statement(
             db->GetCachedStatement(SQL_FROM_HERE, "DELETE FROM sites"));

chrome/browser/android/explore_sites/explore_sites_bridge.cc

@@ -252,4 +252,27 @@ void JNI_ExploreSitesBridge_GetCategoryImage(
       base::BindOnce(&ImageReady,
                      ScopedJavaGlobalRef<jobject>(j_callback_obj)));
 }
+
+// static
+void JNI_ExploreSitesBridge_GetSummaryImage(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& j_profile,
+    const jint j_pixel_size,
+    const JavaParamRef<jobject>& j_callback_obj) {
+  Profile* profile = ProfileAndroid::FromProfileAndroid(j_profile);
+  DCHECK(profile);
+
+  ExploreSitesService* service =
+      ExploreSitesServiceFactory::GetForBrowserContext(profile);
+  if (!service) {
+    DLOG(ERROR) << "Unable to create the ExploreSitesService!";
+    base::android::RunBooleanCallbackAndroid(j_callback_obj, false);
+    return;
+  }
+
+  service->GetSummaryImage(
+      j_pixel_size, base::BindOnce(&ImageReady, ScopedJavaGlobalRef<jobject>(
+                                                    j_callback_obj)));
+}
+
 }  // namespace explore_sites

chrome/browser/android/explore_sites/explore_sites_schema.cc

@@ -13,6 +13,9 @@ namespace explore_sites {
 constexpr int ExploreSitesSchema::kCurrentVersion;
 constexpr int ExploreSitesSchema::kCompatibleVersion;
 
+const char ExploreSitesSchema::kCurrentCatalogKey[] = "current_catalog";
+const char ExploreSitesSchema::kDownloadingCatalogKey[] = "downloading_catalog";
+
 // Schema versions changelog:
 // * 1: Initial version.
 // * 2: Version with ntp shown count.
@@ -156,4 +159,19 @@ bool ExploreSitesSchema::CreateOrUpgradeIfNeeded(sql::Database* db) {
   return current_version == kCurrentVersion;
 }
 
+// static
+std::pair<std::string, std::string> ExploreSitesSchema::GetVersionTokens(
+    sql::Database* db) {
+  sql::MetaTable meta_table;
+  if (!InitMetaTable(db, &meta_table))
+    return std::make_pair("", "");
+
+  std::string current_version_token, downloading_version_token;
+
+  meta_table.GetValue(kCurrentCatalogKey, &current_version_token);
+  meta_table.GetValue(kDownloadingCatalogKey, &downloading_version_token);
+
+  return std::make_pair(current_version_token, downloading_version_token);
+}
+
 }  // namespace explore_sites

chrome/browser/android/explore_sites/explore_sites_schema.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_ANDROID_EXPLORE_SITES_EXPLORE_SITES_SCHEMA_H_
 
 #include <string>
+#include <tuple>
 
 namespace sql {
 class Database;
@@ -21,6 +22,9 @@ class ExploreSitesSchema {
   static constexpr int kCurrentVersion = 2;
   static constexpr int kCompatibleVersion = 1;
 
+  static const char kCurrentCatalogKey[];
+  static const char kDownloadingCatalogKey[];
+
   // Initializes the given meta table using the appropriate versions.
   static bool InitMetaTable(sql::Database* db, sql::MetaTable* meta_table);
 
@@ -28,6 +32,10 @@ class ExploreSitesSchema {
   // a metadata table. Returns |true| if the database is ready to be used,
   // |false| if creation or upgrades failed.
   static bool CreateOrUpgradeIfNeeded(sql::Database* db);
+
+  // Returns a pair representing the current and downloading version tokens.
+  static std::pair<std::string, std::string> GetVersionTokens(
+      sql::Database* db);
 };
 
 }  // namespace explore_sites

chrome/browser/android/explore_sites/explore_sites_service.h

@@ -28,6 +28,11 @@ class ExploreSitesService : public KeyedService {
                                 int pixel_size,
                                 BitmapCallback callback) = 0;
 
+  // Returns via callback an image representing a summary of the current
+  // catalog. This image is composed from multiple site images.
+  // Returns |nullptr| if there was an error.
+  virtual void GetSummaryImage(int pixel_size, BitmapCallback callback) = 0;
+
   // Returns via callback the image for a site. This is typically the site
   // favicon. Returns |nullptr| if there was an error or no match for |site_id|.
   virtual void GetSiteImage(int site_id, BitmapCallback callback) = 0;

chrome/browser/android/explore_sites/explore_sites_service_impl.cc

@@ -97,6 +97,16 @@ void ExploreSitesServiceImpl::GetCategoryImage(int category_id,
                      pixel_size)));
 }
 
+void ExploreSitesServiceImpl::GetSummaryImage(int pixel_size,
+                                              BitmapCallback callback) {
+  task_queue_.AddTask(std::make_unique<GetImagesTask>(
+      explore_sites_store_.get(), GetImagesTask::DataType::kSummary,
+      kFaviconsPerCategoryImage,
+      base::BindOnce(&ExploreSitesServiceImpl::ComposeCategoryImage,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback),
+                     pixel_size)));
+}
+
 void ExploreSitesServiceImpl::GetSiteImage(int site_id,
                                            BitmapCallback callback) {
   task_queue_.AddTask(std::make_unique<GetImagesTask>(

chrome/browser/android/explore_sites/explore_sites_service_impl.h

@@ -46,6 +46,7 @@ class ExploreSitesServiceImpl : public ExploreSitesService,
   void GetCategoryImage(int category_id,
                         int pixel_size,
                         BitmapCallback callback) override;
+  void GetSummaryImage(int pixel_size, BitmapCallback callback) override;
   void GetSiteImage(int site_id, BitmapCallback callback) override;
   void UpdateCatalogFromNetwork(bool is_immediate_fetch,
                                 const std::string& accept_languages,

chrome/browser/android/explore_sites/get_catalog_task.cc

@@ -49,7 +49,7 @@ std::string UpdateCurrentCatalogIfNewer(sql::MetaTable* meta_table,
   DCHECK(meta_table);
   std::string downloading_version_token;
   // See if there is a downloading catalog.
-  if (!meta_table->GetValue("downloading_catalog",
+  if (!meta_table->GetValue(ExploreSitesSchema::kDownloadingCatalogKey,
                             &downloading_version_token)) {
     // No downloading catalog means no change required.
     return current_version_token;
@@ -57,9 +57,10 @@ std::string UpdateCurrentCatalogIfNewer(sql::MetaTable* meta_table,
 
   // Update the current version.
   current_version_token = downloading_version_token;
-  if (!meta_table->SetValue("current_catalog", current_version_token))
+  if (!meta_table->SetValue(ExploreSitesSchema::kCurrentCatalogKey,
+                            current_version_token))
     return "";
-  meta_table->DeleteKey("downloading_catalog");
+  meta_table->DeleteKey(ExploreSitesSchema::kDownloadingCatalogKey);
 
   return downloading_version_token;
 }
@@ -90,13 +91,15 @@ GetCatalogSync(bool update_current, sql::Database* db) {
   // currently in use, don't change it.  This is an error, should have been
   // caught before we got here.
   std::string catalog_version_token;
-  if (!meta_table.GetValue("current_catalog", &catalog_version_token) ||
+  if (!meta_table.GetValue(ExploreSitesSchema::kCurrentCatalogKey,
+                           &catalog_version_token) ||
       catalog_version_token.empty()) {
     DVLOG(1)
         << "Didn't find current catalog value. Attempting to use downloading.";
     // If there is no current catalog, use downloading catalog and mark it as
     // current.  If there is no downloading catalog, return no catalog.
-    meta_table.GetValue("downloading_catalog", &catalog_version_token);
+    meta_table.GetValue(ExploreSitesSchema::kDownloadingCatalogKey,
+                        &catalog_version_token);
     if (catalog_version_token.empty())
       return std::make_pair(GetCatalogStatus::kNoCatalog, nullptr);
 

chrome/browser/android/explore_sites/get_images_task.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/android/explore_sites/get_images_task.h"
 
+#include <tuple>
+
 #include "base/bind.h"
 #include "chrome/browser/android/explore_sites/explore_sites_schema.h"
 #include "sql/database.h"
@@ -21,6 +23,16 @@ WHERE category_id = ? AND LENGTH(favicon) > 0 AND NOT removed
 AND site_blacklist.url IS NULL
 LIMIT ?;)";
 
+const char kSelectSummaryImagesSql[] = R"(SELECT favicon
+FROM sites
+INNER JOIN categories ON (sites.category_id = categories.category_id)
+LEFT JOIN site_blacklist ON (sites.url = site_blacklist.url)
+WHERE LENGTH(favicon) > 0 AND NOT removed
+AND version_token = ?
+AND site_blacklist.url IS NULL
+ORDER BY sites.category_id ASC, sites.site_id ASC
+LIMIT ?;)";
+
 const char kSelectSiteImageSql[] =
     "SELECT favicon FROM sites WHERE site_id = ?;";
 
@@ -47,6 +59,36 @@ EncodedImageList GetCategoryImagesSync(int category_id,
   return result;
 }
 
+EncodedImageList GetSummaryImagesSync(int max_images, sql::Database* db) {
+  DCHECK(db);
+
+  std::string current_version_token, downloading_version_token;
+  std::tie(current_version_token, downloading_version_token) =
+      ExploreSitesSchema::GetVersionTokens(db);
+
+  std::string token_to_use = current_version_token.empty()
+                                 ? downloading_version_token
+                                 : current_version_token;
+
+  if (token_to_use.empty())
+    return EncodedImageList();
+
+  sql::Statement category_statement(
+      db->GetCachedStatement(SQL_FROM_HERE, kSelectSummaryImagesSql));
+  category_statement.BindString(0, token_to_use);
+  category_statement.BindInt64(1, max_images);
+
+  EncodedImageList result;
+  while (category_statement.Step()) {
+    int byte_length = category_statement.ColumnByteLength(0);
+    result.push_back(std::make_unique<std::vector<uint8_t>>(byte_length));
+    category_statement.ColumnBlobAsVector(0, result.back().get());
+  }
+  if (!category_statement.Succeeded())
+    return EncodedImageList();
+  return result;
+}
+
 EncodedImageList GetSiteImageSync(int site_id, sql::Database* db) {
   DCHECK(db);
 
@@ -86,6 +128,17 @@ GetImagesTask::GetImagesTask(ExploreSitesStore* store,
       callback_(std::move(callback)),
       weak_ptr_factory_(this) {}
 
+GetImagesTask::GetImagesTask(ExploreSitesStore* store,
+                             DataType data_type,
+                             int max_images,
+                             EncodedImageListCallback callback)
+    : store_(store),
+      data_type_(data_type),
+      id_(0),
+      max_results_(max_images),
+      callback_(std::move(callback)),
+      weak_ptr_factory_(this) {}
+
 GetImagesTask::~GetImagesTask() = default;
 
 void GetImagesTask::Run() {
@@ -102,6 +155,11 @@ void GetImagesTask::Run() {
                                      weak_ptr_factory_.GetWeakPtr()),
                       EncodedImageList());
       break;
+    case DataType::kSummary:
+      store_->Execute(base::BindOnce(&GetSummaryImagesSync, max_results_),
+                      base::BindOnce(&GetImagesTask::FinishedExecuting,
+                                     weak_ptr_factory_.GetWeakPtr()),
+                      EncodedImageList());
   }
 }
 

chrome/browser/android/explore_sites/get_images_task.h

@@ -24,6 +24,8 @@ namespace explore_sites {
 // specific.
 class GetImagesTask : public Task {
  public:
+  enum class DataType { kCategory, kSite, kSummary };
+
   GetImagesTask(ExploreSitesStore* store,
                 int category_id,
                 int max_images,
@@ -33,6 +35,11 @@ class GetImagesTask : public Task {
                 int site_id,
                 EncodedImageListCallback callback);
 
+  GetImagesTask(ExploreSitesStore* store,
+                DataType data_type,
+                int max_images,
+                EncodedImageListCallback callback);
+
   ~GetImagesTask() override;
 
  private:
@@ -43,7 +50,6 @@ class GetImagesTask : public Task {
 
   ExploreSitesStore* store_;  // outlives this class.
 
-  enum class DataType { kCategory, kSite };
   DataType data_type_;
   int id_;
   int max_results_;

chrome/browser/android/explore_sites/get_images_task_unittest.cc

@@ -43,6 +43,9 @@ class ExploreSitesGetImagesTaskTest : public TaskTestBase {
   }
 
   void PopulateTestingCatalog();
+  void UpdateCatalogVersions(std::string current_version,
+                             std::string downloading_version);
+  void PopulateInvalidRowsInCatalog();
 
   void ExpectEmptyImageList(EncodedImageList images) {
     EXPECT_EQ(0U, images.size());
@@ -53,6 +56,15 @@ class ExploreSitesGetImagesTaskTest : public TaskTestBase {
         [&](EncodedImageList result) { last_result = std::move(result); });
   }
 
+  void ExpectSummaryBitmapsInOrder() {
+    std::vector<std::string> ordered_bitmaps = {"bytes1", "bytes3", "bytes2"};
+    EXPECT_EQ(3U, last_result.size());
+    for (int i = 0; i < 3; i++) {
+      std::vector<uint8_t>& result = *last_result[i];
+      EXPECT_EQ(ordered_bitmaps[i], std::string(result.begin(), result.end()));
+    }
+  }
+
   EncodedImageList last_result;
 
  private:
@@ -89,6 +101,46 @@ VALUES
   }));
 }
 
+void ExploreSitesGetImagesTaskTest::UpdateCatalogVersions(
+    std::string current_version,
+    std::string downloading_version) {
+  ExecuteSync(base::BindLambdaForTesting([&](sql::Database* db) {
+    sql::MetaTable meta_table;
+    ExploreSitesSchema::InitMetaTable(db, &meta_table);
+    meta_table.SetValue(ExploreSitesSchema::kDownloadingCatalogKey,
+                        downloading_version);
+    meta_table.SetValue(ExploreSitesSchema::kCurrentCatalogKey,
+                        current_version);
+    return true;
+  }));
+}
+
+void ExploreSitesGetImagesTaskTest::PopulateInvalidRowsInCatalog() {
+  ExecuteSync(base::BindLambdaForTesting([](sql::Database* db) {
+    sql::MetaTable meta_table;
+    ExploreSitesSchema::InitMetaTable(db, &meta_table);
+    meta_table.SetValue(ExploreSitesSchema::kDownloadingCatalogKey, "5678");
+    meta_table.DeleteKey(ExploreSitesSchema::kCurrentCatalogKey);
+    sql::Statement insert(db->GetUniqueStatement(R"(
+INSERT INTO categories
+(category_id, version_token, type, label)
+VALUES
+(1, "XXXX", 1, "bad_1"),
+(2, "XXXX", 2, "bad_2");)"));
+    if (!insert.Run())
+      return false;
+
+    sql::Statement insert_sites(db->GetUniqueStatement(R"(
+INSERT INTO sites
+(site_id, url, category_id, title, favicon)
+VALUES
+(5, "https://www.bad.com/1", 1, "bad", "bad - used unknown version"),
+(6, "https://www.bad.com/2", 2, "bad", "bad - used unknown version");
+    )"));
+    return insert_sites.Run();
+  }));
+}
+
 TEST_F(ExploreSitesGetImagesTaskTest, StoreFailure) {
   store()->SetInitializationStatusForTesting(InitializationStatus::kFailure,
                                              false);
@@ -199,4 +251,35 @@ VALUES
   EXPECT_EQ("bytes7", std::string(result3.begin(), result3.end()));
 }
 
+TEST_F(ExploreSitesGetImagesTaskTest, SummaryImage) {
+  PopulateTestingCatalog();
+  PopulateInvalidRowsInCatalog();
+  UpdateCatalogVersions("5678", "XXXX");
+  GetImagesTask task(store(), GetImagesTask::DataType::kSummary, 3,
+                     StoreResult());
+  RunTask(&task);
+  ExpectSummaryBitmapsInOrder();
+}
+
+TEST_F(ExploreSitesGetImagesTaskTest,
+       SummaryImageUsesDownloadingCatalogIfNecessary) {
+  PopulateTestingCatalog();
+  PopulateInvalidRowsInCatalog();
+  UpdateCatalogVersions("", "5678");
+  GetImagesTask task(store(), GetImagesTask::DataType::kSummary, 3,
+                     StoreResult());
+  RunTask(&task);
+  ExpectSummaryBitmapsInOrder();
+}
+
+TEST_F(ExploreSitesGetImagesTaskTest, SummaryImageNoResultsIfNoCatalogVersion) {
+  PopulateTestingCatalog();
+  PopulateInvalidRowsInCatalog();
+  UpdateCatalogVersions("", "");
+  GetImagesTask task(store(), GetImagesTask::DataType::kSummary, 3,
+                     StoreResult());
+  RunTask(&task);
+  EXPECT_EQ(0U, last_result.size());
+}
+
 }  // namespace explore_sites

chrome/browser/android/explore_sites/get_version_task.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/android/explore_sites/get_version_task.h"
 
 #include <string>
+#include <tuple>
 
 #include "base/bind.h"
 #include "chrome/browser/android/explore_sites/explore_sites_schema.h"
@@ -19,23 +20,13 @@ std::string GetVersionSync(sql::Database* db) {
   if (!db)
     return "";
 
-  sql::MetaTable meta_table;
-  if (!ExploreSitesSchema::InitMetaTable(db, &meta_table))
-    return "";
-
-  std::string catalog_version_token;
-  if (meta_table.GetValue("downloading_catalog", &catalog_version_token) &&
-      !catalog_version_token.empty()) {
-    return catalog_version_token;
-  }
-
-  catalog_version_token = "";
-  if (meta_table.GetValue("current_catalog", &catalog_version_token) &&
-      !catalog_version_token.empty()) {
-    return catalog_version_token;
-  }
+  std::string catalog_version_token, downloading_version_token;
+  std::tie(catalog_version_token, downloading_version_token) =
+      ExploreSitesSchema::GetVersionTokens(db);
 
-  return "";
+  if (!downloading_version_token.empty())
+    return downloading_version_token;
+  return catalog_version_token;
 }
 
 GetVersionTask::GetVersionTask(ExploreSitesStore* store,

chrome/browser/android/explore_sites/import_catalog_task.cc

@@ -61,7 +61,8 @@ bool ImportCatalogSync(std::string version_token,
   // currently in use, don't change it.  This is an error, should have been
   // caught before we got here.
   std::string current_version_token;
-  if (meta_table.GetValue("current_catalog", &current_version_token) &&
+  if (meta_table.GetValue(ExploreSitesSchema::kCurrentCatalogKey,
+                          &current_version_token) &&
       current_version_token == version_token) {
     return false;
   }
@@ -92,7 +93,8 @@ bool ImportCatalogSync(std::string version_token,
 
   // Update the downloading catalog version number to match what we are
   // importing.
-  if (!meta_table.SetValue("downloading_catalog", version_token))
+  if (!meta_table.SetValue(ExploreSitesSchema::kDownloadingCatalogKey,
+                           version_token))
     return false;
 
   // Then insert each category.