Zine: support multiple sections in the ui

chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
index 2aec52832f4da90f22231b9e7f8aae6c378de4ec..fe436ed797770357b508981d1e194d37172afa86 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
@@ -26,19 +26,22 @@ public class SnippetsBridge {
      * An observer for events in the snippets service.
      */
     public interface SnippetsObserver {
-        void onSnippetsReceived(List<SnippetArticleListItem> snippets);
+        /** Called when suggestions for a category were received. */
+        void onSuggestionsReceived(
+                @SuggestionsCategory int category, List<SnippetArticleListItem> suggestions);

[Philipp Keck, 2016/08/01 15:28:57]

ContentSuggestionsCategory is now simply called Category.
We can leave @SuggestionsCategory as is, or maybe find a new name? Is there a
convention for naming these? I'd say it would be convenient if there was a
deterministic way to derive the annotation name from the enum name.
"Suggestions" as a term is technically already taken and different from "content
suggestions".

[Bernhard Bauer, 2016/08/01 15:37:24]

Yeah, I wasn't super happy about the name either, but given that
https://codereview.chromium.org/1932183002/ is going to land Very Soon Now™,
this will be moot and we can remove the manual @IntDef completely once that
happens.

[Bernhard Bauer, 2016/08/02 13:03:27]

And in fact that happened about five minutes after I wrote this comment :) So,
changed now.

(I realize that at some point in the future I might have to remove a bunch of
these annotations again, when we're going to have _not_ well-known categories,
i.e. purely server-side defined ones).

[Philipp Keck, 2016/08/02 13:26:21]

That also happened a couple of minutes after your comment -- so we now do allow
any nonnegative int for categories. "[ContentSuggestions]Category" also isn't an
enum anymore, it's now a class. So the @KnownCategoriesEnum shouldn't be applied
there. Admittedly, we currently don't have a provider that makes use of that
possibility. But the ContentSuggestionsService interface already allows it.

[Bernhard Bauer, 2016/08/02 14:38:40]

Well, yes, but in practice the category values we have right now are in fact
well-known and part of the KnownCategories enum. 
KnownCategories is still an enum (class), no?

 
-        /** Called when the ARTICLES category changed its status. */
-        void onCategoryStatusChanged(int newStatus);
+        /** Called when the status for a category has changed. */
+        void onCategoryStatusChanged(
+                @SuggestionsCategory int category, @SuggestionsCategoryStatus int newStatus);
     }
 
-    public static boolean isCategoryStatusAvailable(int status) {
+    public static boolean isCategoryStatusAvailable(@SuggestionsCategoryStatus int status) {
         // Note: This code is duplicated in content_suggestions_category_status.cc.
         return status == ContentSuggestionsCategoryStatus.AVAILABLE_LOADING
                 || status == ContentSuggestionsCategoryStatus.AVAILABLE;
     }
 
-    public static boolean isCategoryStatusInitOrAvailable(int status) {
+    public static boolean isCategoryStatusInitOrAvailable(@SuggestionsCategoryStatus int status) {
         // Note: This code is duplicated in content_suggestions_category_status.cc.
         return status == ContentSuggestionsCategoryStatus.INITIALIZING
                 || isCategoryStatusAvailable(status);
@@ -121,29 +124,37 @@ public class SnippetsBridge {
         nativeSetObserver(mNativeSnippetsBridge, observer == null ? null : this);
     }
 
-    public int getCategoryStatus() {
+    @SuggestionsCategoryStatus
+    public int getCategoryStatus(@SuggestionsCategory int category) {
         assert mNativeSnippetsBridge != 0;
-        return nativeGetCategoryStatus(mNativeSnippetsBridge);
+        return nativeGetCategoryStatus(mNativeSnippetsBridge, category);
     }
 
     @CalledByNative
-    private void onSnippetsAvailable(String[] ids, String[] titles, String[] urls, String[] ampUrls,
-            String[] previewText, long[] timestamps, String[] publishers, float[] scores) {
+    private static List<SnippetArticleListItem> createSuggestionList() {
+        return new ArrayList<>();
+    }
+
+    @CalledByNative
+    private static void addSuggestion(List<SnippetArticleListItem> suggestions, String id,

[Philipp Keck, 2016/08/01 15:28:57]

Have you seen this solution (also looks a bit hacky):
https://codereview.chromium.org/2166523002/diff/1/chrome/browser/android/ntp/...
Not sure if it's preferable. I can always change this when I refactor the
snippets bridge for separate sections.

[Bernhard Bauer, 2016/08/01 15:37:24]

Yes, I considered that, but I didn't really want to hardcode the class name as a
string. I then considered getting the class from Java, but at that point figured
I could just have Java create the array for me :)

+            String title, String publisher, String previewText, String url, String ampUrl,
+            long timestamp, float score) {
+        int position = suggestions.size();
+        suggestions.add(new SnippetArticleListItem(id, title, publisher, previewText, url, ampUrl,
+                timestamp, score, position));
+    }
+
+    @CalledByNative
+    private void onSuggestionsAvailable(int category, List<SnippetArticleListItem> suggestions) {
         assert mNativeSnippetsBridge != 0;
         assert mObserver != null;
 
-        List<SnippetArticleListItem> newSnippets = new ArrayList<>(ids.length);
-        for (int i = 0; i < ids.length; i++) {
-            newSnippets.add(new SnippetArticleListItem(ids[i], titles[i], publishers[i],
-                    previewText[i], urls[i], ampUrls[i], timestamps[i], scores[i], i));
-        }
-
-        mObserver.onSnippetsReceived(newSnippets);
+        mObserver.onSuggestionsReceived(category, suggestions);
     }
 
     @CalledByNative
-    private void onCategoryStatusChanged(int newStatus) {
-        if (mObserver != null) mObserver.onCategoryStatusChanged(newStatus);
+    private void onCategoryStatusChanged(int category, int newStatus) {
+        if (mObserver != null) mObserver.onCategoryStatusChanged(category, newStatus);
     }
 
     private native long nativeInit(Profile profile);
@@ -156,5 +167,5 @@ public class SnippetsBridge {
             Callback<Boolean> callback, String url);
     private native void nativeFetchImage(
             long nativeNTPSnippetsBridge, String snippetId, Callback<Bitmap> callback);
-    private native int nativeGetCategoryStatus(long nativeNTPSnippetsBridge);
+    private native int nativeGetCategoryStatus(long nativeNTPSnippetsBridge, int category);
 }