Bug 1328937 - HighlightItem: Move data into separate Highlight / Metadata model classes. r=Grisha
☠☠ backed out by dc639e004927 ☠ ☠
authorSebastian Kaspari <s.kaspari@gmail.com>
Thu, 05 Jan 2017 20:02:54 +0100
changeset 328845 927275a3ea2c4d9bb539801b5f3a006187fe9f23
parent 328844 e1dbcf0b70199a5451588cca6d2a1afdd699a246
child 328846 1d0bc7e990fe114c6b730cb317da5f6c0b94e36e
push id31190
push usercbook@mozilla.com
push dateWed, 11 Jan 2017 15:21:29 +0000
treeherdermozilla-central@5493551203ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGrisha
bugs1328937
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1328937 - HighlightItem: Move data into separate Highlight / Metadata model classes. r=Grisha MozReview-Commit-ID: LfeoFZB9MiL
mobile/android/base/java/org/mozilla/gecko/home/activitystream/model/Highlight.java
mobile/android/base/java/org/mozilla/gecko/home/activitystream/model/Metadata.java
mobile/android/base/java/org/mozilla/gecko/home/activitystream/stream/HighlightItem.java
mobile/android/base/moz.build
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/model/Highlight.java
@@ -0,0 +1,87 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.home.activitystream.model;
+
+import android.database.Cursor;
+import android.support.annotation.Nullable;
+import android.text.format.DateUtils;
+
+import org.mozilla.gecko.activitystream.Utils;
+import org.mozilla.gecko.db.BrowserContract;
+
+public class Highlight {
+    private final String title;
+    private final String url;
+    private final Utils.HighlightSource source;
+    private final long time;
+
+    private Metadata metadata;
+
+    private @Nullable Boolean isPinned;
+    private @Nullable Boolean isBookmarked;
+
+    public static Highlight fromCursor(Cursor cursor) {
+        return new Highlight(cursor);
+    }
+
+    private Highlight(Cursor cursor) {
+        title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE));
+        url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
+        source = Utils.highlightSource(cursor);
+        time = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE));
+
+        metadata = Metadata.fromCursor(cursor);
+
+        updateState();
+    }
+
+    private void updateState() {
+        // We can only be certain of bookmark state if an item is a bookmark item.
+        // Otherwise, due to the underlying highlights query, we have to look up states when
+        // menus are displayed.
+        switch (source) {
+            case BOOKMARKED:
+                isBookmarked = true;
+                isPinned = null;
+                break;
+            case VISITED:
+                isBookmarked = null;
+                isPinned = null;
+                break;
+            default:
+                throw new IllegalArgumentException("Unknown source: " + source);
+        }
+    }
+
+    public String getTitle() {
+        return title;
+    }
+
+    public String getUrl() {
+        return url;
+    }
+
+    public Metadata getMetadata() {
+        return metadata;
+    }
+
+    public Boolean isBookmarked() {
+        return isBookmarked;
+    }
+
+    public Boolean isPinned() {
+        return isPinned;
+    }
+
+    public String getRelativeTimeSpan() {
+        return DateUtils.getRelativeTimeSpanString(
+                        time, System.currentTimeMillis(), DateUtils.MINUTE_IN_MILLIS, 0).toString();
+    }
+
+    public Utils.HighlightSource getSource() {
+        return source;
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/model/Metadata.java
@@ -0,0 +1,49 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.home.activitystream.model;
+
+import android.database.Cursor;
+import android.text.TextUtils;
+import android.util.Log;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+import org.mozilla.gecko.db.BrowserContract;
+
+public class Metadata {
+    private static final String LOGTAG = "GeckoMetadata";
+
+    public static Metadata fromCursor(Cursor cursor) {
+        return new Metadata(
+                cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA)));
+    }
+
+    private String provider;
+
+    private Metadata(String json) {
+        if (TextUtils.isEmpty(json)) {
+            // Just use default values. It's better to have an empty Metadata object instead of
+            // juggling with null values.
+            return;
+        }
+
+        try {
+            JSONObject object = new JSONObject(json);
+
+            provider = object.optString("provider");
+        } catch (JSONException e) {
+            Log.w(LOGTAG, "JSONException while parsing metadata", e);
+        }
+    }
+
+    public boolean hasProvider() {
+        return !TextUtils.isEmpty(provider);
+    }
+
+    public String getProvider() {
+        return provider;
+    }
+}
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/stream/HighlightItem.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/stream/HighlightItem.java
@@ -2,36 +2,31 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.home.activitystream.stream;
 
 import android.database.Cursor;
 import android.graphics.Color;
-import android.support.annotation.Nullable;
 import android.text.TextUtils;
-import android.text.format.DateUtils;
-import android.util.Log;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.ImageView;
 import android.widget.TextView;
 
-import org.json.JSONException;
-import org.json.JSONObject;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.activitystream.ActivityStream;
 import org.mozilla.gecko.activitystream.ActivityStreamTelemetry;
 import org.mozilla.gecko.activitystream.Utils;
-import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.home.HomePager;
 import org.mozilla.gecko.home.activitystream.menu.ActivityStreamContextMenu;
+import org.mozilla.gecko.home.activitystream.model.Highlight;
 import org.mozilla.gecko.icons.IconCallback;
 import org.mozilla.gecko.icons.IconResponse;
 import org.mozilla.gecko.icons.Icons;
 import org.mozilla.gecko.util.DrawableUtil;
 import org.mozilla.gecko.util.TouchTargetUtil;
 import org.mozilla.gecko.util.ViewUtil;
 import org.mozilla.gecko.widget.FaviconView;
 
@@ -39,24 +34,17 @@ import java.util.concurrent.Future;
 
 import static org.mozilla.gecko.activitystream.ActivityStream.extractLabel;
 
 public class HighlightItem extends StreamItem implements IconCallback {
     private static final String LOGTAG = "GeckoHighlightItem";
 
     public static final int LAYOUT_ID = R.layout.activity_stream_card_history_item;
 
-    private String title;
-    private String url;
-    private JSONObject metadata;
-
-    private @Nullable Boolean isPinned;
-    private @Nullable Boolean isBookmarked;
-
-    private Utils.HighlightSource source;
+    private Highlight highlight;
 
     private final FaviconView vIconView;
     private final TextView vLabel;
     private final TextView vTimeSince;
     private final TextView vSourceView;
     private final TextView vPageView;
     private final ImageView vSourceIconView;
 
@@ -84,95 +72,62 @@ public class HighlightItem extends Strea
 
         TouchTargetUtil.ensureTargetHitArea(menuButton, itemView);
 
         menuButton.setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
                 ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder()
                         .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_HIGHLIGHTS)
-                        .forHighlightSource(source);
+                        .forHighlightSource(highlight.getSource());
 
                 ActivityStreamContextMenu.show(v.getContext(),
                         menuButton,
                         extras,
                         ActivityStreamContextMenu.MenuMode.HIGHLIGHT,
-                        title, url, isBookmarked, isPinned,
+                        highlight.getTitle(), highlight.getUrl(), highlight.isBookmarked(), highlight.isPinned(),
                         onUrlOpenListener, onUrlOpenInBackgroundListener,
                         vIconView.getWidth(), vIconView.getHeight());
 
                 Telemetry.sendUIEvent(
                         TelemetryContract.Event.SHOW,
                         TelemetryContract.Method.CONTEXT_MENU,
                         extras.build()
                 );
             }
         });
 
         ViewUtil.enableTouchRipple(menuButton);
     }
 
     public void bind(Cursor cursor, int tilesWidth, int tilesHeight) {
-        final long time = cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.DATE));
-        final String ago = DateUtils.getRelativeTimeSpanString(time, System.currentTimeMillis(), DateUtils.MINUTE_IN_MILLIS, 0).toString();
-
-        title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.History.TITLE));
-        url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
-        source = Utils.highlightSource(cursor);
+        highlight = Highlight.fromCursor(cursor);
 
-        try {
-            final String rawMetadata = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Highlights.METADATA));
-            if (rawMetadata != null) {
-                metadata = new JSONObject(rawMetadata);
-            }
-        } catch (JSONException e) {
-            Log.w(LOGTAG, "JSONException while parsing page metadata", e);
-        }
-
-        vLabel.setText(title);
-        vTimeSince.setText(ago);
+        vLabel.setText(highlight.getTitle());
+        vTimeSince.setText(highlight.getRelativeTimeSpan());
 
         ViewGroup.LayoutParams layoutParams = vIconView.getLayoutParams();
         layoutParams.width = tilesWidth - tilesMargin;
         layoutParams.height = tilesHeight;
         vIconView.setLayoutParams(layoutParams);
 
-        updateStateForSource(source);
-        updateUiForSource(source);
-        updatePage(metadata, url);
+        updateUiForSource(highlight.getSource());
+        updatePage();
 
         if (ongoingIconLoad != null) {
             ongoingIconLoad.cancel(true);
         }
 
         ongoingIconLoad = Icons.with(itemView.getContext())
-                .pageUrl(url)
+                .pageUrl(highlight.getUrl())
                 .skipNetwork()
                 .build()
                 .execute(this);
     }
 
-    private void updateStateForSource(Utils.HighlightSource source) {
-        // We can only be certain of bookmark state if an item is a bookmark item.
-        // Otherwise, due to the underlying highlights query, we have to look up states when
-        // menus are displayed.
-        switch (source) {
-            case BOOKMARKED:
-                isBookmarked = true;
-                isPinned = null;
-                break;
-            case VISITED:
-                isBookmarked = null;
-                isPinned = null;
-                break;
-            default:
-                throw new IllegalArgumentException("Unknown source: " + source);
-        }
-    }
-
     private void updateUiForSource(Utils.HighlightSource source) {
         switch (source) {
             case BOOKMARKED:
                 vSourceView.setText(R.string.activity_stream_highlight_label_bookmarked);
                 vSourceView.setVisibility(View.VISIBLE);
                 vSourceIconView.setImageResource(R.drawable.ic_as_bookmarked);
                 break;
             case VISITED:
@@ -182,34 +137,28 @@ public class HighlightItem extends Strea
                 break;
             default:
                 vSourceView.setVisibility(View.INVISIBLE);
                 vSourceIconView.setImageResource(0);
                 break;
         }
     }
 
-    private void updatePage(final JSONObject metadata, final String url) {
+    private void updatePage() {
         // First try to set the provider name from the page's metadata.
-
-        try {
-            if (metadata != null && metadata.has("provider")) {
-                vPageView.setText(metadata.getString("provider"));
-                return;
-            }
-        } catch (JSONException e) {
-            // Broken JSON? Continue with fallback.
+        if (highlight.getMetadata().hasProvider()) {
+            vPageView.setText(highlight.getMetadata().getProvider());
+            return;
         }
 
         // If there's no provider name available then let's try to extract one from the URL.
-
-        extractLabel(itemView.getContext(), url, false, new ActivityStream.LabelCallback() {
+        extractLabel(itemView.getContext(), highlight.getUrl(), false, new ActivityStream.LabelCallback() {
             @Override
             public void onLabelExtracted(String label) {
-                vPageView.setText(TextUtils.isEmpty(label) ? url : label);
+                vPageView.setText(TextUtils.isEmpty(label) ? highlight.getUrl() : label);
             }
         });
     }
 
     @Override
     public void onIconResponse(IconResponse response) {
         vIconView.updateImage(response);
     }
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -465,16 +465,18 @@ gbjar.sources += ['java/org/mozilla/geck
     'health/SessionInformation.java',
     'health/StubbedHealthRecorder.java',
     'home/activitystream/ActivityStream.java',
     'home/activitystream/ActivityStreamHomeFragment.java',
     'home/activitystream/ActivityStreamHomeScreen.java',
     'home/activitystream/menu/ActivityStreamContextMenu.java',
     'home/activitystream/menu/BottomSheetContextMenu.java',
     'home/activitystream/menu/PopupContextMenu.java',
+    'home/activitystream/model/Highlight.java',
+    'home/activitystream/model/Metadata.java',
     'home/activitystream/stream/HighlightItem.java',
     'home/activitystream/stream/HighlightsTitle.java',
     'home/activitystream/stream/StreamItem.java',
     'home/activitystream/stream/TopPanel.java',
     'home/activitystream/stream/WelcomePanel.java',
     'home/activitystream/StreamRecyclerAdapter.java',
     'home/activitystream/topsites/CirclePageIndicator.java',
     'home/activitystream/topsites/TopSitesCard.java',