[Feed] Implement GetCurrentArticleSuggestions()

The Feed provides information about articles that the offline host must
pass along to Prefetch. However this is instigated by Prefetch from
native and is async, which causes some problems. In order to convert a
list of objects across the JNI, state is held in the FeedOfflineBridge
inside a vector. In order to strongly own the callback to Prefetch, it
is stored in the FeedOfflineHost, which guarantees Prefetch is still
alive. While at it we added a vector of pending callbacks for known
content which may be used one day by internals page for Feed.

This code is not actually invoked yet the FeedOfflineHost does
not yet provide itself to Prefetch.

Bug: 866123
Change-Id: Ie4cf9597d9fcaa47baadff060001974fa1375e6b
Reviewed-on: https://chromium-review.googlesource.com/1219937
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Gang Wu <gangwu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591222}

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedOfflineBridge.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.feed;
 
+import com.google.android.libraries.feed.api.knowncontent.ContentMetadata;
 import com.google.android.libraries.feed.api.knowncontent.ContentRemoval;
 import com.google.android.libraries.feed.api.knowncontent.KnownContentApi;
 import com.google.android.libraries.feed.common.functional.Consumer;
@@ -19,6 +20,7 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 /** Provides access to native implementations of OfflineIndicatorApi. */
 @JNINamespace("feed")
@@ -102,7 +104,7 @@ public class FeedOfflineBridge
      * Filters out any {@link ContentRemoval} that was not user driven, such as old articles being
      * garbage collected.
      *
-     * @param contentRemovd The articles being removed, may or may not be user driven.
+     * @param contentRemoved The articles being removed, may or may not be user driven.
      * @return All and only the user driven removals.
      */
     @VisibleForTesting
@@ -121,6 +123,19 @@ public class FeedOfflineBridge
         return Long.valueOf(id);
     }
 
+    @CalledByNative
+    private void getKnownContent() {
+        mKnownContentApi.getKnownContent((List<ContentMetadata> metadataList) -> {
+            for (ContentMetadata metadata : metadataList) {
+                long time_published_ms = TimeUnit.SECONDS.toMillis(metadata.getTimePublished());
+                nativeAppendContentMetadata(mNativeBridge, metadata.getUrl(), metadata.getTitle(),
+                        time_published_ms, metadata.getImageUrl(), metadata.getPublisher(),
+                        metadata.getFaviconUrl(), metadata.getSnippet());
+            }
+            nativeOnGetKnownContentDone(mNativeBridge);
+        });
+    }
+
     private native long nativeInit(Profile profile);
     private native void nativeDestroy(long nativeFeedOfflineBridge);
     private native Object nativeGetOfflineId(long nativeFeedOfflineBridge, String url);
@@ -129,4 +144,8 @@ public class FeedOfflineBridge
     private native void nativeOnContentRemoved(long nativeFeedOfflineBridge, String[] urlsRemoved);
     private native void nativeOnNewContentReceived(long nativeFeedOfflineBridge);
     private native void nativeOnNoListeners(long nativeFeedOfflineBridge);
+    private native void nativeAppendContentMetadata(long nativeFeedOfflineBridge, String url,
+            String title, long timePublishedMs, String imageUrl, String publisher,
+            String faviconUrl, String snippet);
+    private native void nativeOnGetKnownContentDone(long nativeFeedOfflineBridge);
 }

chrome/android/feed/core/java/src/org/chromium/chrome/browser/feed/FeedProcessScopeFactory.java

@@ -68,7 +68,7 @@ public class FeedProcessScopeFactory {
         schedulerBridge.initializeFeedDependencies(
                 sFeedProcessScope.getRequestManager(), sFeedProcessScope.getSessionManager());
 
-        // TODO(skym): Pass on the KnownContentApi when the FeedProcessScope provides one.
+        // TODO(skym): Use sFeedProcessScope.getKnownContentApi().
         sFeedOfflineIndicator = new FeedOfflineBridge(profile, null);
     }
 

chrome/browser/android/feed/feed_offline_bridge.cc

@@ -19,7 +19,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_android.h"
 #include "components/feed/content/feed_host_service.h"
-#include "components/offline_pages/core/offline_page_model.h"
+#include "components/feed/core/content_metadata.h"
 #include "jni/FeedOfflineBridge_jni.h"
 
 using base::android::JavaRef;
@@ -55,8 +55,11 @@ static jlong JNI_FeedOfflineBridge_Init(
 FeedOfflineBridge::FeedOfflineBridge(const JavaRef<jobject>& j_this,
                                      FeedOfflineHost* offline_host)
     : j_this_(ScopedJavaGlobalRef<jobject>(j_this)),
-      offline_host_(offline_host) {
+      offline_host_(offline_host),
+      weak_factory_(this) {
   DCHECK(offline_host_);
+  offline_host_->Initialize(base::BindRepeating(
+      &FeedOfflineBridge::TriggerGetKnownContent, weak_factory_.GetWeakPtr()));
 }
 
 FeedOfflineBridge::~FeedOfflineBridge() = default;
@@ -106,4 +109,39 @@ void FeedOfflineBridge::OnNoListeners(
   offline_host_->OnNoListeners();
 }
 
+void FeedOfflineBridge::AppendContentMetadata(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& j_this,
+    const base::android::JavaRef<jstring>& j_url,
+    const base::android::JavaRef<jstring>& j_title,
+    const jlong j_time_published_ms,
+    const base::android::JavaRef<jstring>& j_image_url,
+    const base::android::JavaRef<jstring>& j_publisher,
+    const base::android::JavaRef<jstring>& j_favicon_url,
+    const base::android::JavaRef<jstring>& j_snippet) {
+  ContentMetadata metadata;
+  metadata.url = base::android::ConvertJavaStringToUTF8(env, j_url);
+  metadata.title = base::android::ConvertJavaStringToUTF8(env, j_title);
+  metadata.time_published = base::Time::FromJavaTime(j_time_published_ms);
+  metadata.image_url = base::android::ConvertJavaStringToUTF8(env, j_image_url);
+  metadata.publisher = base::android::ConvertJavaStringToUTF8(env, j_publisher);
+  metadata.favicon_url =
+      base::android::ConvertJavaStringToUTF8(env, j_favicon_url);
+  metadata.snippet = base::android::ConvertJavaStringToUTF8(env, j_snippet);
+  known_content_metadata_buffer_.push_back(std::move(metadata));
+}
+
+void FeedOfflineBridge::OnGetKnownContentDone(
+    JNIEnv* env,
+    const base::android::JavaRef<jobject>& j_this) {
+  offline_host_->OnGetKnownContentDone(
+      std::move(known_content_metadata_buffer_));
+}
+
+void FeedOfflineBridge::TriggerGetKnownContent() {
+  DCHECK(known_content_metadata_buffer_.empty());
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_FeedOfflineBridge_getKnownContent(env, j_this_);
+}
+
 }  // namespace feed

chrome/browser/android/feed/feed_offline_bridge.h

@@ -6,12 +6,13 @@
 #define CHROME_BROWSER_ANDROID_FEED_FEED_OFFLINE_BRIDGE_H_
 
 #include <jni.h>
-#include <stdint.h>
 #include <vector>
 
 #include "base/android/scoped_java_ref.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "components/feed/content/feed_offline_host.h"
+#include "components/feed/core/content_metadata.h"
 
 namespace feed {
 
@@ -48,13 +49,45 @@ class FeedOfflineBridge {
   void OnNoListeners(JNIEnv* env,
                      const base::android::JavaRef<jobject>& j_this);
 
+  // Used to convert from Java ContentMetadata to a native ContentMetadata, and
+  // put the resulting object into |known_content_metadata_buffer_|. When a
+  // GetKnownContent() call finishes, this method should be synchronously called
+  // for every piece of data, and then OnGetKnownContentDone() should be called.
+  void AppendContentMetadata(
+      JNIEnv* env,
+      const base::android::JavaRef<jobject>& j_this,
+      const base::android::JavaRef<jstring>& j_url,
+      const base::android::JavaRef<jstring>& j_title,
+      const jlong j_time_published_ms,
+      const base::android::JavaRef<jstring>& j_image_url,
+      const base::android::JavaRef<jstring>& j_publisher,
+      const base::android::JavaRef<jstring>& j_favicon_url,
+      const base::android::JavaRef<jstring>& j_snippet);
+
+  // Called to flush the contents of |known_content_metadata_buffer_| to the
+  // |offline_host_|. This should happen at the end of a GetKnownContent() call,
+  // and after AppendContentMetadata() is called for all data.
+  void OnGetKnownContentDone(JNIEnv* env,
+                             const base::android::JavaRef<jobject>& j_this);
+
  private:
+  // Starts an the async request for ContentMetadata through KnownContentApi's
+  // GetKnownContent(). Assumes the caller was FeedOfflineHost and will directly
+  // call FeedOfflineHost::OnGetKnownContentDone() on async completion.
+  void TriggerGetKnownContent();
+
   // Reference to the Java half of this bridge. Always valid.
   base::android::ScopedJavaGlobalRef<jobject> j_this_;
 
   // Object to which all Java to native calls are delegated.
   FeedOfflineHost* offline_host_;
 
+  // Temporarily holds ContentMetadata objects during the completion of a
+  // GetKnownContent call.
+  std::vector<ContentMetadata> known_content_metadata_buffer_;
+
+  base::WeakPtrFactory<FeedOfflineBridge> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(FeedOfflineBridge);
 };
 

components/feed/content/feed_offline_host.cc

@@ -15,11 +15,17 @@
 
 namespace feed {
 
+using offline_pages::OfflinePageItem;
+using offline_pages::OfflinePageModel;
+using offline_pages::PrefetchService;
+using offline_pages::PrefetchSuggestion;
+using offline_pages::SuggestionsProvider;
+
 namespace {
 
 // |url| is always set. Sometimes |original_url| is set. If |original_url| is
 // set it is returned by this method, otherwise fall back to |url|.
-const GURL& PreferOriginal(const offline_pages::OfflinePageItem& item) {
+const GURL& PreferOriginal(const OfflinePageItem& item) {
   return item.original_url.is_empty() ? item.url : item.original_url;
 }
 
@@ -39,9 +45,9 @@ class CallbackAggregator : public base::RefCounted<CallbackAggregator> {
         on_each_result_(std::move(on_each_result)),
         start_time_(base::Time::Now()) {}
 
-  void OnGetPages(const std::vector<offline_pages::OfflinePageItem>& pages) {
+  void OnGetPages(const std::vector<OfflinePageItem>& pages) {
     if (!pages.empty()) {
-      offline_pages::OfflinePageItem newest =
+      OfflinePageItem newest =
           *std::max_element(pages.begin(), pages.end(), [](auto lhs, auto rhs) {
             return lhs.creation_time < rhs.creation_time;
           });
@@ -75,18 +81,43 @@ class CallbackAggregator : public base::RefCounted<CallbackAggregator> {
   base::Time start_time_;
 };
 
+// Consumes |metadataVector|, moving as many of the fields as possible.
+std::vector<PrefetchSuggestion> ConvertMetadataToSuggestions(
+    std::vector<ContentMetadata> metadataVector) {
+  std::vector<PrefetchSuggestion> suggestionsVector;
+  for (ContentMetadata& metadata : metadataVector) {
+    // TODO(skym): Copy over published time when PrefetchSuggestion adds
+    // support.
+    PrefetchSuggestion suggestion;
+    suggestion.article_url = GURL(metadata.url);
+    suggestion.article_title = std::move(metadata.title);
+    suggestion.article_attribution = std::move(metadata.publisher);
+    suggestion.article_snippet = std::move(metadata.snippet);
+    suggestion.thumbnail_url = GURL(metadata.image_url);
+    suggestion.favicon_url = GURL(metadata.favicon_url);
+    suggestionsVector.push_back(std::move(suggestion));
+  }
+  return suggestionsVector;
+}
+
+void RunSuggestionCallbackWithConversion(
+    SuggestionsProvider::SuggestionCallback suggestions_callback,
+    std::vector<ContentMetadata> metadataVector) {
+  std::move(suggestions_callback)
+      .Run(ConvertMetadataToSuggestions(std::move(metadataVector)));
+}
+
 }  //  namespace
 
-FeedOfflineHost::FeedOfflineHost(
-    offline_pages::OfflinePageModel* offline_page_model,
-    offline_pages::PrefetchService* prefetch_service,
-    base::RepeatingClosure on_suggestion_consumed,
-    base::RepeatingClosure on_suggestions_shown)
+FeedOfflineHost::FeedOfflineHost(OfflinePageModel* offline_page_model,
+                                 PrefetchService* prefetch_service,
+                                 base::RepeatingClosure on_suggestion_consumed,
+                                 base::RepeatingClosure on_suggestions_shown)
     : offline_page_model_(offline_page_model),
       prefetch_service_(prefetch_service),
       on_suggestion_consumed_(on_suggestion_consumed),
       on_suggestions_shown_(on_suggestions_shown),
-      weak_ptr_factory_(this) {
+      weak_factory_(this) {
   DCHECK(offline_page_model_);
   DCHECK(prefetch_service_);
   DCHECK(!on_suggestion_consumed_.is_null());
@@ -95,6 +126,13 @@ FeedOfflineHost::FeedOfflineHost(
 
 FeedOfflineHost::~FeedOfflineHost() = default;
 
+void FeedOfflineHost::Initialize(
+    const base::RepeatingClosure& trigger_get_known_content) {
+  DCHECK(trigger_get_known_content_.is_null());
+  trigger_get_known_content_ = trigger_get_known_content;
+  // TODO(skym): Post task to call PrefetchService::SetSuggestionProvider().
+}
+
 base::Optional<int64_t> FeedOfflineHost::GetOfflineId(const std::string& url) {
   auto iter = url_hash_to_id_.find(base::Hash(url));
   return iter == url_hash_to_id_.end() ? base::Optional<int64_t>()
@@ -110,7 +148,7 @@ void FeedOfflineHost::GetOfflineStatus(
       base::MakeRefCounted<CallbackAggregator>(
           std::move(callback),
           base::BindRepeating(&FeedOfflineHost::CacheOfflinePageAndId,
-                              weak_ptr_factory_.GetWeakPtr()));
+                              weak_factory_.GetWeakPtr()));
 
   for (std::string url : urls) {
     offline_page_model_->GetPagesByURL(
@@ -130,10 +168,29 @@ void FeedOfflineHost::OnNoListeners() {
   // TODO(skym): Clear out local cache of offline data.
 }
 
+void FeedOfflineHost::OnGetKnownContentDone(
+    std::vector<ContentMetadata> suggestions) {
+  // While |suggestions| are movable, there might be multiple callbacks in
+  // |pending_known_content_callbacks_|. All but one callback will need to
+  // receive a full copy of |suggestions|, and the last one will received a
+  // moved copy.
+  for (size_t i = 0; i < pending_known_content_callbacks_.size(); i++) {
+    bool can_move = (i + 1 == pending_known_content_callbacks_.size());
+    std::move(pending_known_content_callbacks_[i])
+        .Run(can_move ? std::move(suggestions) : suggestions);
+  }
+  pending_known_content_callbacks_.clear();
+}
+
 void FeedOfflineHost::GetCurrentArticleSuggestions(
-    offline_pages::SuggestionsProvider::SuggestionCallback
-        suggestions_callback) {
-  // TODO(skym): Call into bridge callback.
+    SuggestionsProvider::SuggestionCallback suggestions_callback) {
+  pending_known_content_callbacks_.push_back(base::BindOnce(
+      &RunSuggestionCallbackWithConversion, std::move(suggestions_callback)));
+  // Trigger after push_back() in case triggering results in a synchronous
+  // response via OnGetKnownContentDone().
+  if (pending_known_content_callbacks_.size() <= 1) {
+    trigger_get_known_content_.Run();
+  }
 }
 
 void FeedOfflineHost::ReportArticleListViewed() {
@@ -144,19 +201,17 @@ void FeedOfflineHost::ReportArticleViewed(GURL article_url) {
   on_suggestions_shown_.Run();
 }
 
-void FeedOfflineHost::OfflinePageModelLoaded(
-    offline_pages::OfflinePageModel* model) {
+void FeedOfflineHost::OfflinePageModelLoaded(OfflinePageModel* model) {
   // Ignored.
 }
 
-void FeedOfflineHost::OfflinePageAdded(
-    offline_pages::OfflinePageModel* model,
-    const offline_pages::OfflinePageItem& added_page) {
+void FeedOfflineHost::OfflinePageAdded(OfflinePageModel* model,
+                                       const OfflinePageItem& added_page) {
   // TODO(skym): Call into bridge callback.
 }
 
 void FeedOfflineHost::OfflinePageDeleted(
-    const offline_pages::OfflinePageModel::DeletedPageInfo& page_info) {
+    const OfflinePageModel::DeletedPageInfo& page_info) {
   // TODO(skym): Call into bridge callback.
 }
 

components/feed/content/feed_offline_host.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "components/feed/core/content_metadata.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "components/offline_pages/core/prefetch/suggestions_provider.h"
 
@@ -32,12 +33,20 @@ namespace feed {
 class FeedOfflineHost : public offline_pages::SuggestionsProvider,
                         public offline_pages::OfflinePageModel::Observer {
  public:
+  using GetKnownContentCallback =
+      base::OnceCallback<void(std::vector<ContentMetadata>)>;
+
   FeedOfflineHost(offline_pages::OfflinePageModel* offline_page_model,
                   offline_pages::PrefetchService* prefetch_service,
                   base::RepeatingClosure on_suggestion_consumed,
                   base::RepeatingClosure on_suggestions_shown);
   ~FeedOfflineHost() override;
 
+  // Initialize with callbacks to call into bridge/Java side. Should only be
+  // called once, and done as soon as the bridge is ready. The FeedOfflineHost
+  // will not be fully ready to perform its function without these dependencies.
+  void Initialize(const base::RepeatingClosure& trigger_get_known_content);
+
   // Synchronously returns the offline id of the given page. The host will only
   // have knowledge of the page if it had previously returned status about it
   // through GetOfflineState() or as a notification. Otherwise the caller will
@@ -64,6 +73,10 @@ class FeedOfflineHost : public offline_pages::SuggestionsProvider,
   // to clear local tracking of offlined items.
   void OnNoListeners();
 
+  // Should be called when async GetKnownContent is completed. Broadcasts to all
+  // waiting consumers in |pending_known_content_callbacks_|.
+  void OnGetKnownContentDone(std::vector<ContentMetadata> suggestions);
+
   // offline_pages::SuggestionsProvider:
   void GetCurrentArticleSuggestions(
       offline_pages::SuggestionsProvider::SuggestionCallback
@@ -99,7 +112,16 @@ class FeedOfflineHost : public offline_pages::SuggestionsProvider,
   // is the offline id for the given page.
   base::flat_map<uint32_t, int64_t> url_hash_to_id_;
 
-  base::WeakPtrFactory<FeedOfflineHost> weak_ptr_factory_;
+  // Starts an the async request for ContentMetadata through KnownContentApi's
+  // GetKnownContent(). Will only be invoked when there isn't already an
+  // outstanding GetKnownContent().
+  base::RepeatingClosure trigger_get_known_content_;
+
+  // Holds all consumers of GetKnownContent(). It is assumed that there's an
+  // outstanding GetKnownContent() if and only if this vector is not empty.
+  std::vector<GetKnownContentCallback> pending_known_content_callbacks_;
+
+  base::WeakPtrFactory<FeedOfflineHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FeedOfflineHost);
 };

components/feed/core/BUILD.gn

@@ -8,6 +8,8 @@ if (is_android) {
 
 source_set("feed_core") {
   sources = [
+    "content_metadata.cc",
+    "content_metadata.h",
     "feed_content_database.cc",
     "feed_content_database.h",
     "feed_content_mutation.cc",

components/feed/core/content_metadata.cc

+// Copyright 2018 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 "components/feed/core/content_metadata.h"
+
+namespace feed {
+
+ContentMetadata::ContentMetadata() = default;
+
+ContentMetadata::ContentMetadata(const ContentMetadata&) = default;
+
+ContentMetadata::ContentMetadata(ContentMetadata&&) = default;
+
+ContentMetadata::~ContentMetadata() = default;
+
+}  // namespace feed

components/feed/core/content_metadata.h

+// Copyright 2018 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.
+
+#ifndef COMPONENTS_FEED_CORE_CONTENT_METADATA_H_
+#define COMPONENTS_FEED_CORE_CONTENT_METADATA_H_
+
+#include <string>
+
+#include "base/time/time.h"
+
+namespace feed {
+
+// Native counterpart of ContentMetadata.java.
+struct ContentMetadata {
+  ContentMetadata();
+  ContentMetadata(const ContentMetadata&);
+  ContentMetadata(ContentMetadata&&);
+  ~ContentMetadata();
+
+  // A link to the underlying article.
+  std::string url;
+
+  // The title of the article.
+  std::string title;
+
+  // The time the article was published, independent of when the device was
+  // downloaded this metadata.
+  base::Time time_published;
+
+  // A link to a thumbnail.
+  std::string image_url;
+
+  // The name of the publisher.
+  std::string publisher;
+
+  // A link to the favicon for the publisher's domain.
+  std::string favicon_url;
+
+  // A short description of the article, human readable.
+  std::string snippet;
+};
+
+}  // namespace feed
+
+#endif  // COMPONENTS_FEED_CORE_CONTENT_METADATA_H_