[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
--- 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)) {