[Enhanced Bookmark]Rewrite initialization logic

chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java b/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java
index 12c84cba40d82c066a88d2968cea15b881ea3992..f26eac2d22ff91250248ea7d8e59abbc57e2e774 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java
@@ -7,10 +7,8 @@ package org.chromium.chrome.browser.enhancedbookmarks;
 import android.app.Activity;
 import android.app.ActivityManager;
 import android.content.Context;
-import android.preference.PreferenceManager;
 import android.support.v4.widget.DrawerLayout;
 import android.text.TextUtils;
-import android.util.Log;
 import android.view.Gravity;
 import android.view.View;
 import android.view.ViewGroup;
@@ -25,17 +23,12 @@ import org.chromium.chrome.browser.bookmark.BookmarksBridge.BookmarkItem;
 import org.chromium.chrome.browser.bookmark.BookmarksBridge.BookmarkModelObserver;
 import org.chromium.chrome.browser.favicon.LargeIconBridge;
 import org.chromium.chrome.browser.ntp.NewTabPageUma;
-import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
 import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksShim;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.snackbar.SnackbarManager;
 import org.chromium.chrome.browser.snackbar.SnackbarManager.SnackbarManageable;
 import org.chromium.components.bookmarks.BookmarkId;
-import org.chromium.ui.base.DeviceFormFactor;
 
-import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
-import java.net.URLEncoder;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -48,7 +41,6 @@ import java.util.Stack;
  * {@link EnhancedBookmarkActivity} (phone) and {@link EnhancedBookmarkPage} (tablet).
  */
 public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
-    private static final String PREF_LAST_USED_URL = "enhanced_bookmark_last_used_url";
     private static final int FAVICON_MAX_CACHE_SIZE_BYTES = 10 * 1024 * 1024; // 10MB
 
     private Activity mActivity;
@@ -66,6 +58,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
     private EnhancedBookmarkDrawerListView mDrawerListView;
     private final Stack<UIState> mStateStack = new Stack<>();
     private LargeIconBridge mLargeIconBridge;
+    private String mInitialUrl;
 
     private final BookmarkModelObserver mBookmarkModelObserver = new BookmarkModelObserver() {
 
@@ -93,11 +86,6 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
         }
 
         @Override
-        public void bookmarkModelLoaded() {
-            initializeIfBookmarkModelLoaded();
-        }
-
-        @Override
         public void bookmarkModelChanged() {
             // If the folder no longer exists in folder mode, we need to fall back. Relying on the
             // default behavior by setting the folder mode again.
@@ -108,6 +96,18 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
         }
     };
 
+    private final Runnable mModelLoadedRunnable = new Runnable() {
+        @Override
+        public void run() {
+            mSearchView.onEnhancedBookmarkDelegateInitialized(EnhancedBookmarkManager.this);
+            mDrawerListView.onEnhancedBookmarkDelegateInitialized(EnhancedBookmarkManager.this);
+            mContentView.onEnhancedBookmarkDelegateInitialized(EnhancedBookmarkManager.this);
+            if (!TextUtils.isEmpty(mInitialUrl)) {
+                setState(UIState.createStateFromUrl(mInitialUrl, mEnhancedBookmarksModel));
+            }
+        }
+    };
+
     /**
      * Creates an instance of {@link EnhancedBookmarkManager}. It also initializes resources,
      * bookmark models and jni bridges.
@@ -126,7 +126,8 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
                 ((SnackbarManageable) activity).getSnackbarManager());
         mSearchView = (EnhancedBookmarkSearchView) getView().findViewById(R.id.eb_search_view);
         mEnhancedBookmarksModel.addObserver(mBookmarkModelObserver);
-        initializeIfBookmarkModelLoaded();
+        initializeToLoadingState();
+        mEnhancedBookmarksModel.doAfterBookmarkModelLoaded(mModelLoadedRunnable);
 
         // Load partner bookmarks explicitly. We load partner bookmarks in the deferred startup
         // code, but that might be executed much later. Especially on L, showing loading
@@ -207,75 +208,33 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
     }
 
     /**
-     * Updates UI based on the new URL on tablet. If the bookmark model is not loaded yet, creates a
-     * temporary loading state carrying this url. This method is supposed to align with
+     * Updates UI based on the new URL. If the bookmark model is not loaded yet, cache the url and
+     * it will be picked up later when the model is loaded. This method is supposed to align with
      * {@link EnhancedBookmarkPage#updateForUrl(String)}
      * <p>
      * @param url The url to navigate to.
      */
     public void updateForUrl(String url) {
-        if (mEnhancedBookmarksModel != null && mEnhancedBookmarksModel.isBookmarkModelLoaded()) {
-            setState(UIState.createStateFromUrl(url, mEnhancedBookmarksModel));
-        } else {
-            // Note this does not guarantee to update the UI, as at this time the onCreateView()
-            // might not has even been called yet.
-            setState(UIState.createLoadingState(url));
-        }
-    }
+        // Bookmark model is null if the manager has been destroyed.
+        if (mEnhancedBookmarksModel == null) return;
 
-    /**
-     * Initialization method that has 3 different behaviors based on whether bookmark model is
-     * loaded. If the bookmark model is not loaded yet, it pushes a loading state to backstack which
-     * contains the url from preference. If the model is loaded and the backstack is empty, it
-     * creates a state by fetching the last visited bookmark url stored in preference. If the
-     * bookmark model is loaded but backstack contains a pending loading state, it creates a new
-     * state by getting the url of the loading state and replace the previous loading state with the
-     * new normal state.
-     */
-    private void initializeIfBookmarkModelLoaded() {
         if (mEnhancedBookmarksModel.isBookmarkModelLoaded()) {
-            mSearchView.onEnhancedBookmarkDelegateInitialized(this);
-            mDrawerListView.onEnhancedBookmarkDelegateInitialized(this);
-            mContentView.onEnhancedBookmarkDelegateInitialized(this);
-            if (mStateStack.isEmpty()) {
-                setState(UIState.createStateFromUrl(getUrlFromPreference(),
-                        mEnhancedBookmarksModel));
-            } else if (mStateStack.peek().mState == UIState.STATE_LOADING) {
-                String url = mStateStack.pop().mUrl;
-                setState(UIState.createStateFromUrl(url, mEnhancedBookmarksModel));
-            }
+            setState(UIState.createStateFromUrl(url, mEnhancedBookmarksModel));
         } else {
-            mContentView.showLoadingUi();
-            mDrawerListView.showLoadingUi();
-            mContentView.showLoadingUi();
-            if (mStateStack.isEmpty() || mStateStack.peek().mState != UIState.STATE_LOADING) {
-                setState(UIState.createLoadingState(getUrlFromPreference()));
-            } else if (!mStateStack.isEmpty()) {
-                // Refresh the UI. This is needed because on tablet, updateForUrl might set up
-                // loading state too early and at that time all UI components are not created yet.
-                // Therefore we need to set the previous loading state once again to trigger all UI
-                // updates.
-                setState(mStateStack.pop());
-            }
+            mInitialUrl = url;
         }
     }
 
     /**
-     * Saves url to preference. Note this method should be used after the main view is attached to
-     * an activity.
-     */
-    private void saveUrlToPreference(String url) {
-        PreferenceManager.getDefaultSharedPreferences(mActivity).edit()
-                .putString(PREF_LAST_USED_URL, url).apply();
-    }
-
-    /**
-     * Fetches url to preference. Note this method should be used after the main view is attached to
-     * an activity.
+     * Puts all UI elements to loading state. This state might be overridden synchronously by
+     * {@link #updateForUrl(String)}, if the bookmark model is already loaded.
      */
-    private String getUrlFromPreference() {
-        return PreferenceManager.getDefaultSharedPreferences(mActivity).getString(
-                PREF_LAST_USED_URL, UrlConstants.BOOKMARKS_URL);
+    private void initializeToLoadingState() {
+        mContentView.showLoadingUi();
+        mDrawerListView.showLoadingUi();
+        mContentView.showLoadingUi();
+        assert mStateStack.isEmpty();
+        setState(UIState.createLoadingState());
     }
 
     /**
@@ -295,37 +254,25 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
         if (!state.isValid(mEnhancedBookmarksModel)) {
             state = UIState.createAllBookmarksState(mEnhancedBookmarksModel);
         }
-        boolean saveUrl = true;
-        if (mStateStack.isEmpty()) {
-            // When offline page feature is enabled, show offline filter view if there is offline
-            // page and there is no network connection.
-            if (mEnhancedBookmarksModel.getOfflinePageBridge() != null
-                    && !mEnhancedBookmarksModel.getOfflinePageBridge().getAllPages().isEmpty()
-                    && !OfflinePageUtils.isConnected(mActivity.getApplicationContext())
-                    && !DeviceFormFactor.isTablet(mActivity.getApplicationContext())) {
-                UIState filterState = UIState.createFilterState(
-                        EnhancedBookmarkFilter.OFFLINE_PAGES, mEnhancedBookmarksModel);
-                if (state.mState != UIState.STATE_LOADING) {
-                    state = filterState;
-                } else {
-                    state.mUrl = filterState.mUrl;
-                }
-                // Showing offline filter view is just a temporary thing and it will not be saved
-                // to the preference.
-                saveUrl = false;
-            }
-        } else {
-            if (mStateStack.peek().equals(state)) return;
-            if (mStateStack.peek().mState == UIState.STATE_LOADING) {
-                mStateStack.pop();
-            }
+
+        if (!mStateStack.isEmpty() && mStateStack.peek().equals(state)) return;
+
+        // The loading state is not persisted in history stack and once we have a valid state it
+        // shall be removed.
+        if (!mStateStack.isEmpty() && mStateStack.peek().mState == UIState.STATE_LOADING) {
+            mStateStack.pop();
         }
         mStateStack.push(state);
+
         if (state.mState != UIState.STATE_LOADING) {
             // Loading state may be pushed to the stack but should never be stored in preferences.
-            if (saveUrl) saveUrlToPreference(state.mUrl);
+            if (state.mShouldPersist) {
+                EnhancedBookmarkUtils.setLastUsedUrl(mActivity, state.mUrl);
+            }
             // If a loading state is replaced by another loading state, do not notify this change.
-            if (mUrlChangeListener != null) mUrlChangeListener.onBookmarkUIStateChange(state.mUrl);
+            if (mUrlChangeListener != null) {
+                mUrlChangeListener.onBookmarkUIStateChange(state.mUrl);
+            }
         }
 
         clearSelection();
@@ -496,8 +443,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
     }
 
     /**
-     * Internal state that represents a url. Note every state needs to have a _valid_ url. For
-     * loading state, {@link #mUrl} indicates the target to open after the bookmark model is loaded.
+     * Internal state that represents a url, except that a loading state does not have a url.
      */
     static class UIState {
         private static final int STATE_INVALID = 0;
@@ -506,20 +452,20 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
         static final int STATE_FOLDER = 3;
         static final int STATE_FILTER = 4;
 
-        private static final String TAG = "UIState";
-        private static final String URL_CHARSET = "UTF-8";
         /**
          * One of the STATE_* constants.
          */
         private int mState;
         private String mUrl;
+        /** Whether this state should be persisted as user's last location. */
+        private boolean mShouldPersist = true;
         private BookmarkId mFolder;
         private EnhancedBookmarkFilter mFilter;
 
-        private static UIState createLoadingState(String url) {
+        private static UIState createLoadingState() {
             UIState state = new UIState();
-            state.mUrl = url;
             state.mState = STATE_LOADING;
+            state.mShouldPersist = false;
             return state;
         }
 
@@ -529,14 +475,14 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
 
         private static UIState createFolderState(BookmarkId folder,
                 EnhancedBookmarksModel bookmarkModel) {
-            return createStateFromUrl(UrlConstants.BOOKMARKS_FOLDER_URL + folder.toString(),
-                    bookmarkModel);
+            return createStateFromUrl(EnhancedBookmarkUtils.encodeUrl(
+                    UrlConstants.BOOKMARKS_FOLDER_URL, folder.toString()), bookmarkModel);
         }
 
         private static UIState createFilterState(
                 EnhancedBookmarkFilter filter, EnhancedBookmarksModel bookmarkModel) {
-            return createStateFromUrl(
-                    UrlConstants.BOOKMARKS_FILTER_URL + filter.toString(), bookmarkModel);
+            return createStateFromUrl(EnhancedBookmarkUtils.encodeUrl(
+                    UrlConstants.BOOKMARKS_FILTER_URL, filter.toString()), bookmarkModel);
         }
 
         /**
@@ -547,17 +493,18 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
             UIState state = new UIState();
             state.mState = STATE_INVALID;
             state.mUrl = url;
+            state.mShouldPersist = EnhancedBookmarkUtils.getShouldPersistFromUrl(url);
 
             if (url.equals(UrlConstants.BOOKMARKS_URL)) {
                 state.mState = STATE_ALL_BOOKMARKS;
             } else if (url.startsWith(UrlConstants.BOOKMARKS_FOLDER_URL)) {
-                String suffix = decodeSuffix(url, UrlConstants.BOOKMARKS_FOLDER_URL);
+                String suffix = EnhancedBookmarkUtils.decodePath(url);
                 if (!suffix.isEmpty()) {
                     state.mFolder = BookmarkId.getBookmarkIdFromString(suffix);
                     state.mState = STATE_FOLDER;
                 }
             } else if (url.startsWith(UrlConstants.BOOKMARKS_FILTER_URL)) {
-                String suffix = decodeSuffix(url, UrlConstants.BOOKMARKS_FILTER_URL);
+                String suffix = EnhancedBookmarkUtils.decodePath(url);
                 if (!suffix.isEmpty()) {
                     state.mState = STATE_FILTER;
                     state.mFilter = EnhancedBookmarkFilter.valueOf(suffix);
@@ -579,6 +526,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
          * @return Whether this state is valid.
          */
         private boolean isValid(EnhancedBookmarksModel bookmarkModel) {
+            if (mState == STATE_LOADING) return true;
             if (mUrl == null || mState == STATE_INVALID) return false;
 
             if (mState == STATE_FOLDER) {
@@ -591,25 +539,6 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
             return true;
         }
 
-        private static String decodeSuffix(String url, String prefix) {
-            String suffix = url.substring(prefix.length());
-            try {
-                suffix = URLDecoder.decode(suffix, URL_CHARSET);
-            } catch (UnsupportedEncodingException e) {
-                Log.w(TAG, "Bookmark URL parsing failed. " + URL_CHARSET + " not supported.");
-            }
-            return suffix;
-        }
-
-        private static String encodeUrl(String prefix, String suffix) {
-            try {
-                suffix = URLEncoder.encode(suffix, URL_CHARSET);
-            } catch (UnsupportedEncodingException e) {
-                Log.w(TAG, "Bookmark URL parsing failed. " + URL_CHARSET + " not supported.");
-            }
-            return prefix + suffix;
-        }
-
         @Override
         public int hashCode() {
             return 31 * mUrl.hashCode() + mState;
@@ -619,6 +548,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
         public boolean equals(Object obj) {
             if (!(obj instanceof UIState)) return false;
             UIState other = (UIState) obj;
+            if (mState == STATE_LOADING && other.mState == STATE_LOADING) return true;
             return mState == other.mState && mUrl.equals(other.mUrl);
         }
     }