[temporary] Bug 1234315 - Load SavedReaderViewHelper during DB onLoad instead of on UI thread r?nalexander draft
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 30 Mar 2016 11:23:55 -0700
changeset 346025 3580529549db4350975aeab6b8866579d514bc15
parent 345971 1775058f9170b88525f32362edcf8003b5b050bd
child 346026 e588f8699f3559647c34eacf5886413e4f434e27
child 347676 efa4944a77defacd6429c88ed7f0a56acdb1c893
push id14217
push userahunt@mozilla.com
push dateWed, 30 Mar 2016 19:43:04 +0000
reviewersnalexander
bugs1234315
milestone48.0a1
[temporary] Bug 1234315 - Load SavedReaderViewHelper during DB onLoad instead of on UI thread r?nalexander We'll want to squash this into the previous commit, assuming this is the approach we want. MozReview-Commit-ID: JacH1AMExYy
mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
mobile/android/base/java/org/mozilla/gecko/reader/SavedReaderViewHelper.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -1250,16 +1250,24 @@ public final class BrowserDatabaseHelper
             }
         }
     }
 
     @Override
     public void onOpen(SQLiteDatabase db) {
         debug("Opening browser.db: " + db.getPath());
 
+        // Force explicit readercache loading - we won't access readercache state for bookmarks
+        // until we actually know what our bookmarks are. Bookmarks are stored in the DB, hence
+        // it is sufficient to ensure that the readercache is loaded before the DB can be accessed.
+        // Note, this takes ~4-6ms to load on an N4 (compared to 20-50ms for most DB queries), and
+        // is only done once, hence this shouldn't have noticeable impact on performance. Moreover
+        // this is run on a background thread and therefore won't block UI code during startup.
+        SavedReaderViewHelper.getSavedReaderViewHelper(mContext).loadItems();
+
         Cursor cursor = null;
         try {
             cursor = db.rawQuery("PRAGMA foreign_keys=ON", null);
         } finally {
             if (cursor != null)
                 cursor.close();
         }
         cursor = null;
--- a/mobile/android/base/java/org/mozilla/gecko/reader/SavedReaderViewHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/reader/SavedReaderViewHelper.java
@@ -56,47 +56,75 @@ public class SavedReaderViewHelper {
 
     private static final String PATH = "path";
     private static final String SIZE = "size";
 
     private static final String DIRECTORY = "readercache";
     private static final String FILE_NAME = "items.json";
     private static final String FILE_PATH = DIRECTORY + "/" + FILE_NAME;
 
-    private final JSONObject mItems;
+    // We use null to indicate that the cache hasn't yet been loaded. Loading has to be explicitly
+    // requested by client code, and must happen on the background thread. Attempting to access
+    // items (which happens mainly on the UI thread) before explicitly loading them is not permitted.
+    private JSONObject mItems = null;
 
     private final Context mContext;
 
-    private ReaderCache(Context context) {
+    private static SavedReaderViewHelper instance = null;
+
+    private SavedReaderViewHelper(Context context) {
         mContext = context;
+    }
 
-        JSONObject items;
+    public static synchronized SavedReaderViewHelper getSavedReaderViewHelper(final Context context) {
+        if (instance == null) {
+            instance = new SavedReaderViewHelper(context);
+        }
+
+        return instance;
+    }
+
+    public synchronized void loadItems() {
+        ThreadUtils.assertNotOnUiThread();
+
+        if (mItems != null) {
+            return;
+        }
+
         try {
-            items = GeckoProfile.get(mContext).readJSONObjectFromFile(FILE_PATH);
+            mItems = GeckoProfile.get(mContext).readJSONObjectFromFile(FILE_PATH);
         } catch (IOException e) {
-            Log.i(LOG_TAG, "Couldn't read readercache file, initialising empty readercache instead", e);
-            items = new JSONObject();
+            Log.i(LOG_TAG, "Couldn't read saved reader view list from file, initialising empty list instead", e);
+            mItems = new JSONObject();
         }
-        mItems = items;
+    }
+
+    private void assertItemsLoaded() {
+        if (mItems == null) {
+            throw new IllegalStateException("SavedReaderView items must be explicitly loaded using loadItems() before access.");
+        }
     }
 
     private JSONObject makeItem(@NonNull String path, long size) throws JSONException {
         final JSONObject item = new JSONObject();
 
         item.put(PATH, path);
         item.put(SIZE, size);
 
         return item;
     }
 
     public synchronized boolean isURLCached(@NonNull final String URL) {
+        assertItemsLoaded();
         return mItems.has(URL);
     }
 
     public synchronized void put(@NonNull final String pageURL, @NonNull final String path, final long size) {
+        assertItemsLoaded();
+
         try {
             mItems.put(pageURL, makeItem(path, size));
         } catch (JSONException e) {
             Log.w(LOG_TAG, "Item insertion failed:", e);
             // This should never happen, absent any errors in our own implementation
             throw new IllegalStateException("Failure inserting into SavedReaderViewHelper json");
         }
 
@@ -107,30 +135,33 @@ public class SavedReaderViewHelper {
                 annotations.insertReaderviewUrl(mContext.getContentResolver(), pageURL);
 
                 commit();
             }
         });
     }
 
     protected synchronized void remove(@NonNull final String pageURL) {
+        assertItemsLoaded();
+
         mItems.remove(pageURL);
 
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 UrlAnnotations annotations = GeckoProfile.get(mContext).getDB().getUrlAnnotations();
                 annotations.deleteReaderviewUrl(mContext.getContentResolver(), pageURL);
 
                 commit();
             }
         });
     }
 
     public synchronized int size() {
+        assertItemsLoaded();
         return mItems.length();
     }
 
     private synchronized void commit() {
         ThreadUtils.assertOnBackgroundThread();
 
         GeckoProfile profile = GeckoProfile.get(mContext);
         File cacheDir = new File(profile.getDir(), DIRECTORY);
@@ -138,27 +169,16 @@ public class SavedReaderViewHelper {
         if (!cacheDir.exists()) {
             Log.i(LOG_TAG, "No preexisting cache directory, creating now");
             cacheDir.mkdir();
         }
 
         profile.writeFile(FILE_PATH, mItems.toString());
     }
 
-    private static SavedReaderViewHelper ourSavedReaderViewHelper = null;
-
-    public static synchronized SavedReaderViewHelper getSavedReaderViewHelper(final Context context) {
-        if (ourSavedReaderViewHelper != null) {
-            return ourSavedReaderViewHelper;
-        }
-
-        ourSavedReaderViewHelper = new SavedReaderViewHelper(context);
-        return ourSavedReaderViewHelper;
-    }
-
     /**
      * Return the Reader View URL for a given URL if it is contained in the cache. Returns the
      * plain URL if the page is not cached.
      */
     public static String getReaderURLIfCached(final Context context, @NonNull final String pageURL) {
         SavedReaderViewHelper rch = getSavedReaderViewHelper(context);
 
         if (rch.isURLCached(pageURL)) {