Use NTP query tile clicks for tile ordering

This CL use NTP clicks to help order the tiles.

BUG=1096224

Change-Id: I9d3ec7c9f2d8526a7ef6cd5e5f106a48b31d767a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418902
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809170}

chrome/android/java/src/org/chromium/chrome/browser/query_tiles/QueryTileSection.java

@@ -117,6 +117,9 @@ public class QueryTileSection {
     private void onTileClicked(ImageTile tile) {
         QueryTile queryTile = (QueryTile) tile;
         mTileUmaLogger.recordTileClicked(queryTile);
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.QUERY_TILES_LOCAL_ORDERING)) {
+            mTileProvider.onTileClicked(queryTile.id);
+        }
 
         boolean isLastLevelTile = queryTile.children.isEmpty();
         if (isLastLevelTile) {

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java

@@ -369,6 +369,7 @@ public abstract class ChromeFeatureList {
     public static final String QUERY_TILES = "QueryTiles";
     public static final String QUERY_TILES_IN_OMNIBOX = "QueryTilesInOmnibox";
     public static final String QUERY_TILES_ENABLE_QUERY_EDITING = "QueryTilesEnableQueryEditing";
+    public static final String QUERY_TILES_LOCAL_ORDERING = "QueryTilesLocalOrdering";
     public static final String QUIET_NOTIFICATION_PROMPTS = "QuietNotificationPrompts";
     public static final String REACHED_CODE_PROFILER = "ReachedCodeProfiler";
     public static final String READ_LATER = "ReadLater";

components/query_tiles/android/java/src/org/chromium/components/query_tiles/TestTileProvider.java

@@ -69,6 +69,9 @@ public class TestTileProvider implements TileProvider {
         callback.onResult(mTiles);
     }
 
+    @Override
+    public void onTileClicked(String tileId) {}
+
     private static List<QueryTile> buildTiles(String prefix, int levelsLeft, int count) {
         if (levelsLeft == 0) return null;
         List<QueryTile> children = new ArrayList<>();

components/query_tiles/android/java/src/org/chromium/components/query_tiles/TileProvider.java

@@ -19,4 +19,10 @@ public interface TileProvider {
      *         no tiles are found.
      */
     void getQueryTiles(Callback<List<QueryTile>> callback);
+
+    /**
+     * Called when a tile is clicked.
+     * @param tildId ID of the tile.
+     */
+    void onTileClicked(String tileId);
 }

components/query_tiles/android/java/src/org/chromium/components/query_tiles/bridges/TileProviderBridge.java

@@ -40,9 +40,16 @@ public class TileProviderBridge implements TileProvider {
         TileProviderBridgeJni.get().getQueryTiles(mNativeTileProviderBridge, this, callback);
     }
 
+    @Override
+    public void onTileClicked(String tildId) {
+        if (mNativeTileProviderBridge == 0) return;
+        TileProviderBridgeJni.get().onTileClicked(mNativeTileProviderBridge, tildId);
+    }
+
     @NativeMethods
     interface Natives {
         void getQueryTiles(long nativeTileProviderBridge, TileProviderBridge caller,
                 Callback<List<QueryTile>> callback);
+        void onTileClicked(long nativeTileProviderBridge, String tileId);
     }
 }

components/query_tiles/android/tile_provider_bridge.cc

@@ -68,4 +68,9 @@ void TileProviderBridge::GetQueryTiles(JNIEnv* env,
       &RunGetTilesCallback, ScopedJavaGlobalRef<jobject>(jcallback)));
 }
 
+void TileProviderBridge::OnTileClicked(JNIEnv* env,
+                                       const JavaParamRef<jstring>& j_tile_id) {
+  tile_service_->OnTileClicked(ConvertJavaStringToUTF8(env, j_tile_id));
+}
+
 }  // namespace query_tiles

components/query_tiles/android/tile_provider_bridge.h

@@ -32,6 +32,9 @@ class TileProviderBridge : public base::SupportsUserData::Data {
                      const JavaParamRef<jobject>& jcaller,
                      const JavaParamRef<jobject>& jcallback);
 
+  // Called when a tile is clicked.
+  void OnTileClicked(JNIEnv* env, const JavaParamRef<jstring>& j_tile_id);
+
  private:
   // A reference to the Java counterpart of this class.  See
   // TileProviderBridge.java.

components/query_tiles/internal/init_aware_tile_service.cc

@@ -113,6 +113,15 @@ void InitAwareTileService::SetServerUrl(const std::string& base_url) {
   }
 }
 
+void InitAwareTileService::OnTileClicked(const std::string& tile_id) {
+  if (IsReady()) {
+    tile_service_->OnTileClicked(tile_id);
+  } else if (!IsFailed()) {
+    MaybeCacheApiCall(base::BindOnce(&InitAwareTileService::OnTileClicked,
+                                     weak_ptr_factory_.GetWeakPtr(), tile_id));
+  }
+}
+
 Logger* InitAwareTileService::GetLogger() {
   return tile_service_->GetLogger();
 }

components/query_tiles/internal/init_aware_tile_service.h

@@ -34,6 +34,7 @@ class InitAwareTileService : public TileService {
   void CancelTask() override;
   void PurgeDb() override;
   void SetServerUrl(const std::string& base_url) override;
+  void OnTileClicked(const std::string& tile_id) override;
   Logger* GetLogger() override;
 
   void OnTileServiceInitialized(bool success);

components/query_tiles/internal/init_aware_tile_service_unittest.cc

@@ -46,6 +46,7 @@ class MockInitializableTileService : public InitializableTileService {
   MOCK_METHOD(void, PurgeDb, (), (override));
   MOCK_METHOD(Logger*, GetLogger, (), (override));
   MOCK_METHOD(void, SetServerUrl, (const std::string&), (override));
+  MOCK_METHOD(void, OnTileClicked, (const std::string&), (override));
 
   // Callback stubs.
   MOCK_METHOD(void, GetTilesCallbackStub, (TileList), ());

components/query_tiles/internal/tile_group.cc

@@ -7,7 +7,7 @@
 #include <sstream>
 #include <utility>
 
-#include "tile_utils.h"
+#include "components/query_tiles/internal/tile_utils.h"
 
 namespace query_tiles {
 

components/query_tiles/internal/tile_manager.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string_split.h"
 #include "base/task_runner.h"
@@ -18,6 +19,7 @@
 #include "components/query_tiles/internal/tile_iterator.h"
 #include "components/query_tiles/internal/tile_manager.h"
 #include "components/query_tiles/internal/tile_utils.h"
+#include "components/query_tiles/switches.h"
 
 namespace query_tiles {
 namespace {
@@ -160,7 +162,8 @@ class TileManagerImpl : public TileManager {
     }
 
     // Keep the stats group in memory for tile score calculation.
-    if (loaded_groups.find(kTileStatsGroup) != loaded_groups.end()) {
+    if (loaded_groups.find(kTileStatsGroup) != loaded_groups.end() &&
+        base::FeatureList::IsEnabled(features::kQueryTilesLocalOrdering)) {
       tile_stats_group_ = std::move(loaded_groups[kTileStatsGroup]);
       // prevent the stats group from being deleted.
       loaded_groups.erase(kTileStatsGroup);
@@ -229,6 +232,18 @@ class TileManagerImpl : public TileManager {
     NOTIMPLEMENTED();
   }
 
+  void OnTileClicked(const std::string& tile_id) override {
+    // If the tile stats haven't been created, create it here.
+    if (!tile_stats_group_) {
+      tile_stats_group_ = std::make_unique<TileGroup>();
+      tile_stats_group_->id = kTileStatsGroup;
+    }
+    tile_stats_group_->OnTileClicked(tile_id);
+    // It's fine if |tile_stats_group_| is not saved, so no callback needs to
+    // be passed to Update().
+    store_->Update(kTileStatsGroup, *tile_stats_group_, base::DoNothing());
+  }
+
   // Indicates if the db is fully initialized, rejects calls if not.
   bool initialized_;
 

components/query_tiles/internal/tile_manager.h

@@ -53,6 +53,9 @@ class TileManager {
   // Dump the group. Used for debugging in WebUI only.
   virtual TileGroup* GetTileGroup() = 0;
 
+  // Called when a tile is clicked.
+  virtual void OnTileClicked(const std::string& tile_id) = 0;
+
   TileManager();
   virtual ~TileManager() = default;
 

components/query_tiles/internal/tile_service_impl.cc

@@ -141,6 +141,10 @@ TileGroup* TileServiceImpl::GetTileGroup() {
   return tile_manager_->GetTileGroup();
 }
 
+void TileServiceImpl::OnTileClicked(const std::string& tile_id) {
+  tile_manager_->OnTileClicked(tile_id);
+}
+
 Logger* TileServiceImpl::GetLogger() {
   return logger_.get();
 }

components/query_tiles/internal/tile_service_impl.h

@@ -56,6 +56,7 @@ class TileServiceImpl : public InitializableTileService,
   void CancelTask() override;
   void PurgeDb() override;
   void SetServerUrl(const std::string& base_url) override;
+  void OnTileClicked(const std::string& tile_id) override;
   Logger* GetLogger() override;
 
   // TileServiceScheduler::Delegate implementation.

components/query_tiles/internal/tile_service_impl_unittest.cc

@@ -42,6 +42,7 @@ class MockTileManager : public TileManager {
   MOCK_METHOD(void, SetAcceptLanguagesForTesting, (const std::string&));
   MOCK_METHOD(TileGroupStatus, PurgeDb, ());
   MOCK_METHOD(TileGroup*, GetTileGroup, ());
+  MOCK_METHOD(void, OnTileClicked, (const std::string&));
 };
 
 class MockTileServiceScheduler : public TileServiceScheduler {

components/query_tiles/test/fake_tile_service.cc

@@ -103,6 +103,8 @@ void FakeTileService::PurgeDb() {}
 
 void FakeTileService::SetServerUrl(const std::string& url) {}
 
+void FakeTileService::OnTileClicked(const std::string& url) {}
+
 Logger* FakeTileService::GetLogger() {
   return nullptr;
 }

components/query_tiles/test/fake_tile_service.h

@@ -32,6 +32,7 @@ class FakeTileService : public TileService {
   void CancelTask() override;
   void PurgeDb() override;
   void SetServerUrl(const std::string& url) override;
+  void OnTileClicked(const std::string& tile_id) override;
   Logger* GetLogger() override;
 
   std::vector<std::unique_ptr<Tile>> tiles_;

components/query_tiles/tile_service.h

@@ -50,6 +50,9 @@ class TileService : public KeyedService, public base::SupportsUserData {
   // Used for setting the server url for test.
   virtual void SetServerUrl(const std::string& base_url) = 0;
 
+  // Called when a tile was clicked.
+  virtual void OnTileClicked(const std::string& tile_id) = 0;
+
   // Returns a Logger instance that is meant to be used by logging and debug UI
   // components in the larger system.
   virtual Logger* GetLogger() = 0;