Bug 1234319 - Post: cleanup/remove OnReadingListEventListener r=mcomella
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 08 Mar 2016 09:15:43 -0800
changeset 292508 32c0ef305a73ad75a61e0a6b4e25f514ef6aec53
parent 292507 1ee8f56284d9887ac08b92063ba47416b29aa661
child 292509 3d82a3fa52dbe1b7621364a4c3ad3d92c5ce5d12
push id74863
push userryanvm@gmail.com
push dateSat, 09 Apr 2016 19:23:17 +0000
treeherdermozilla-inbound@ee048ce0f8d5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcomella
bugs1234319
milestone48.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 1234319 - Post: cleanup/remove OnReadingListEventListener r=mcomella MozReview-Commit-ID: 5mSmqPQk744
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testReadingListProvider.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -692,17 +692,17 @@ public class BrowserApp extends GeckoApp
         // Init suggested sites engine in BrowserDB.
         final SuggestedSites suggestedSites = new SuggestedSites(appContext, distribution);
         final BrowserDB db = getProfile().getDB();
         db.setSuggestedSites(suggestedSites);
 
         JavaAddonManager.getInstance().init(appContext);
         mSharedPreferencesHelper = new SharedPreferencesHelper(appContext);
         mOrderedBroadcastHelper = new OrderedBroadcastHelper(appContext);
-        mReadingListHelper = new ReadingListHelper(appContext, getProfile(), null);
+        mReadingListHelper = new ReadingListHelper(appContext, getProfile());
         mAccountsHelper = new AccountsHelper(appContext, getProfile());
 
         final AdjustHelperInterface adjustHelper = AdjustConstants.getAdjustHelper();
         adjustHelper.onCreate(this, AdjustConstants.MOZ_INSTALL_TRACKING_ADJUST_SDK_APP_TOKEN);
 
         // Adjust stores enabled state so this is only necessary because users may have set
         // their data preferences before this feature was implemented and we need to respect
         // those before upload can occur in Adjust.onResume.
--- a/mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/reader/ReadingListHelper.java
@@ -6,212 +6,64 @@ package org.mozilla.gecko.reader;
 
 import org.json.JSONException;
 import org.json.JSONObject;
 
 import org.mozilla.gecko.AboutPages;
 import org.mozilla.gecko.EventDispatcher;
 import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.GeckoProfile;
-import org.mozilla.gecko.annotation.RobocopTarget;
-import org.mozilla.gecko.db.BrowserContract.ReadingListItems;
 import org.mozilla.gecko.db.BrowserDB;
-import org.mozilla.gecko.db.ReadingListAccessor;
 import org.mozilla.gecko.favicons.Favicons;
 import org.mozilla.gecko.util.EventCallback;
 import org.mozilla.gecko.util.NativeEventListener;
 import org.mozilla.gecko.util.NativeJSObject;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.util.UIAsyncTask;
 
-import android.content.ContentResolver;
-import android.content.ContentValues;
 import android.content.Context;
-import android.database.ContentObserver;
-import android.database.Cursor;
 import android.util.Log;
 
 public final class ReadingListHelper implements NativeEventListener {
     private static final String LOGTAG = "GeckoReadingListHelper";
 
-    public interface OnReadingListEventListener {
-        void onAddedToReadingList(String url);
-        void onRemovedFromReadingList(String url);
-        void onAlreadyInReadingList(String url);
-    }
-
-    private enum ReadingListEvent {
-        ADDED,
-        REMOVED,
-        ALREADY_EXISTS
-    }
-
     protected final Context context;
     private final BrowserDB db;
-    private final ReadingListAccessor readingListAccessor;
-    private final ContentObserver contentObserver;
-    private final OnReadingListEventListener onReadingListEventListener;
 
-    volatile boolean fetchInBackground = true;
-
-    public ReadingListHelper(Context context, GeckoProfile profile, OnReadingListEventListener listener) {
+    public ReadingListHelper(Context context, GeckoProfile profile) {
         this.context = context;
         this.db = profile.getDB();
-        this.readingListAccessor = db.getReadingListAccessor();
 
         EventDispatcher.getInstance().registerGeckoThreadListener((NativeEventListener) this,
-            "Reader:AddToList", "Reader:UpdateList", "Reader:FaviconRequest", "Reader:AddedToCache");
-
-        contentObserver = new ContentObserver(null) {
-            @Override
-            public void onChange(boolean selfChange) {
-                if (fetchInBackground) {
-                    fetchContent();
-                }
-            }
-        };
-
-        this.readingListAccessor.registerContentObserver(context, contentObserver);
-
-        onReadingListEventListener = listener;
+            "Reader:FaviconRequest", "Reader:AddedToCache");
     }
 
     public void uninit() {
         EventDispatcher.getInstance().unregisterGeckoThreadListener((NativeEventListener) this,
-            "Reader:AddToList", "Reader:UpdateList", "Reader:FaviconRequest", "Reader:AddedToCache");
-
-        context.getContentResolver().unregisterContentObserver(contentObserver);
+            "Reader:FaviconRequest", "Reader:AddedToCache");
     }
 
     @Override
     public void handleMessage(final String event, final NativeJSObject message,
                               final EventCallback callback) {
         switch(event) {
-            // Added from web context menu.
-            case "Reader:AddToList": {
-                handleAddToList(callback, message);
-                break;
-            }
-            case "Reader:UpdateList": {
-                handleUpdateList(message);
-                break;
-            }
             case "Reader:FaviconRequest": {
                 handleReaderModeFaviconRequest(callback, message.getString("url"));
                 break;
             }
             case "Reader:AddedToCache": {
                 // AddedToCache is a one way message: callback will be null, and we therefore shouldn't
                 // attempt to handle it.
                 handleAddedToCache(message.getString("url"), message.getString("path"), message.getInt("size"));
                 break;
             }
         }
     }
 
     /**
-     * A page can be added to the ReadingList by long-tap of the page-action
-     * icon, or by tapping the readinglist-add icon in the ReaderMode banner.
-     *
-     * This method will only add new items, not update existing items.
-     */
-    private void handleAddToList(final EventCallback callback, final NativeJSObject message) {
-        final ContentResolver cr = context.getContentResolver();
-        final String url = message.getString("url");
-
-        // We can't access a NativeJSObject from the background thread, so we need to get the
-        // values here, even if we may not use them to insert an item into the DB.
-        final ContentValues values = getContentValues(message);
-
-        ThreadUtils.postToBackgroundThread(new Runnable() {
-            @Override
-            public void run() {
-                if (readingListAccessor.isReadingListItem(cr, url)) {
-                    handleEvent(ReadingListEvent.ALREADY_EXISTS, url);
-                    callback.sendError("URL already in reading list: " + url);
-                } else {
-                    readingListAccessor.addReadingListItem(cr, values);
-                    handleEvent(ReadingListEvent.ADDED, url);
-                    callback.sendSuccess(url);
-                }
-            }
-        });
-    }
-
-    /**
-     * Updates a reading list item with new meta data.
-     */
-    private void handleUpdateList(final NativeJSObject message) {
-        final ContentResolver cr = context.getContentResolver();
-        final ContentValues values = getContentValues(message);
-
-        ThreadUtils.postToBackgroundThread(new Runnable() {
-            @Override
-            public void run() {
-                readingListAccessor.updateReadingListItem(cr, values);
-            }
-        });
-    }
-
-    /**
-     * Creates reading list item content values from JS message.
-     */
-    private ContentValues getContentValues(NativeJSObject message) {
-        final ContentValues values = new ContentValues();
-        if (message.has("id")) {
-            values.put(ReadingListItems._ID, message.getInt("id"));
-        }
-
-        // url is actually required...
-        String url = null;
-        if (message.has("url")) {
-            url = message.getString("url");
-            values.put(ReadingListItems.URL, url);
-        }
-
-        String title = null;
-        if (message.has("title")) {
-            title = message.getString("title");
-            values.put(ReadingListItems.TITLE, title);
-        }
-
-        // TODO: message actually has "length", but that's no use for us. See Bug 1127451.
-        if (message.has("word_count")) {
-            values.put(ReadingListItems.WORD_COUNT, message.getInt("word_count"));
-        }
-
-        if (message.has("excerpt")) {
-            values.put(ReadingListItems.EXCERPT, message.getString("excerpt"));
-        }
-
-        if (message.has("status")) {
-            final int status = message.getInt("status");
-            values.put(ReadingListItems.CONTENT_STATUS, status);
-            if (status == ReadingListItems.STATUS_FETCHED_ARTICLE) {
-                if (message.has("resolved_title")) {
-                    values.put(ReadingListItems.RESOLVED_TITLE, message.getString("resolved_title"));
-                } else {
-                    if (title != null) {
-                        values.put(ReadingListItems.RESOLVED_TITLE, title);
-                    }
-                }
-                if (message.has("resolved_url")) {
-                    values.put(ReadingListItems.RESOLVED_URL, message.getString("resolved_url"));
-                } else {
-                    if (url != null) {
-                        values.put(ReadingListItems.RESOLVED_URL, url);
-                    }
-                }
-            }
-        }
-
-        return values;
-    }
-
-    /**
      * Gecko (ReaderMode) requests the page favicon to append to the
      * document head for display.
      */
     private void handleReaderModeFaviconRequest(final EventCallback callback, final String url) {
         (new UIAsyncTask.WithoutParams<String>(ThreadUtils.getBackgroundHandler()) {
             @Override
             public String doInBackground() {
                 return Favicons.getFaviconURLForPageURL(db, context.getContentResolver(), url);
@@ -234,64 +86,16 @@ public final class ReadingListHelper imp
     }
 
     private void handleAddedToCache(final String url, final String path, final int size) {
         final SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context);
 
         rch.put(url, path, size);
     }
 
-    /**
-     * Handle various reading list events (and display appropriate toasts).
-     */
-    private void handleEvent(final ReadingListEvent event, final String url) {
-        ThreadUtils.postToUiThread(new Runnable() {
-            @Override
-            public void run() {
-                switch (event) {
-                    case ADDED:
-                        onReadingListEventListener.onAddedToReadingList(url);
-                        break;
-                    case REMOVED:
-                        onReadingListEventListener.onRemovedFromReadingList(url);
-                        break;
-                    case ALREADY_EXISTS:
-                        onReadingListEventListener.onAlreadyInReadingList(url);
-                        break;
-                }
-            }
-        });
-    }
-
-    private void fetchContent() {
-        ThreadUtils.postToBackgroundThread(new Runnable() {
-            @Override
-            public void run() {
-                final Cursor c = readingListAccessor.getReadingListUnfetched(context.getContentResolver());
-                if (c == null) {
-                    return;
-                }
-                try {
-                    while (c.moveToNext()) {
-                        JSONObject json = new JSONObject();
-                        try {
-                            json.put("id", c.getInt(c.getColumnIndexOrThrow(ReadingListItems._ID)));
-                            json.put("url", c.getString(c.getColumnIndexOrThrow(ReadingListItems.URL)));
-                            GeckoAppShell.notifyObservers("Reader:FetchContent", json.toString());
-                        } catch (JSONException e) {
-                            Log.e(LOGTAG, "Failed to fetch reading list content for item");
-                        }
-                    }
-                } finally {
-                    c.close();
-                }
-            }
-        });
-    }
-
     public static void cacheReaderItem(final String url, Context context) {
         if (AboutPages.isAboutReader(url)) {
             throw new IllegalArgumentException("Page url must be original (not about:reader) url");
         }
 
         SavedReaderViewHelper rch = SavedReaderViewHelper.getSavedReaderViewHelper(context);
 
         if (!rch.isURLCached(url)) {
@@ -310,18 +114,9 @@ public final class ReadingListHelper imp
             GeckoAppShell.notifyObservers("Reader:RemoveFromCache", url);
         }
 
         // When removing items from the cache we can probably spare ourselves the async callback
         // that we use when adding cached items. We know the cached item will be gone, hence
         // we no longer need to track it in the SavedReaderViewHelper
         rch.remove(url);
     }
-
-    @RobocopTarget
-    /**
-     * Test code will want to disable background fetches to avoid upsetting
-     * the test harness. Call this by accessing the instance from BrowserApp.
-     */
-    public void disableBackgroundFetches() {
-        fetchInBackground = false;
-    }
 }
--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testReadingListProvider.java
+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/testReadingListProvider.java
@@ -86,23 +86,16 @@ public class testReadingListProvider ext
     };
 
     @Override
     public void setUp() throws Exception {
         super.setUp(sProviderFactory, BrowserContract.READING_LIST_AUTHORITY, DB_NAME);
         for (TestCase test: TESTS_TO_RUN) {
             mTests.add(test);
         }
-
-        // Disable background fetches of content, because it causes the test harness
-        // to kill us for network fetches.
-        Activity a = getActivity();
-        if (a instanceof BrowserApp) {
-            ((BrowserApp) a).getReadingListHelper().disableBackgroundFetches();
-        }
     }
 
     public void testReadingListProviderTests() throws Exception {
         for (Runnable test : mTests) {
             setTestName(test.getClass().getSimpleName());
             ensureEmptyDatabase();
             test.run();
         }