Bug 1234319 - Post: cleanup/remove OnReadingListEventListener r=mcomella draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 08 Mar 2016 09:15:43 -0800
changeset 345253 cff67d12f2f165fd8f9116b29075a8223ba3fe49
parent 345252 d9d4b356305cd40caadd32ab84e08ed81b70b4ba
child 345254 d3bba5ed13b60b14e8554d601424ed360fa7334a
push id14029
push userahunt@mozilla.com
push dateMon, 28 Mar 2016 16:56:10 +0000
reviewersmcomella
bugs1234319
milestone48.0a1
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
@@ -666,17 +666,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 ReaderCache rch = ReaderCache.getReaderCache(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");
         }
 
         ReaderCache rch = ReaderCache.getReaderCache(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 ReaderCache
         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();
         }