Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4062)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkManager.java

Issue 1440623004: [Enhanced Bookmark]Rewrite initialization logic (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Thank you, findbug Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 e0678dc87b683c2d529220d1ceeb5f740e918e9a..cd92109eb9ebca75538702478ea7a51293d8de59 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,9 +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.util.Log;
+import android.text.TextUtils;
import android.view.Gravity;
import android.view.View;
import android.view.ViewGroup;
@@ -18,21 +17,15 @@ import android.widget.ViewSwitcher;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ObserverList;
import org.chromium.chrome.R;
-import org.chromium.chrome.browser.UrlConstants;
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.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;
@@ -45,7 +38,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;
@@ -61,8 +53,9 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
private ViewSwitcher mViewSwitcher;
private DrawerLayout mDrawer;
private EnhancedBookmarkDrawerListView mDrawerListView;
- private final Stack<UIState> mStateStack = new Stack<>();
+ private final Stack<EnhancedBookmarkUIState> mStateStack = new Stack<>();
private LargeIconBridge mLargeIconBridge;
+ private String mInitialUrl;
private final BookmarkModelObserver mBookmarkModelObserver = new BookmarkModelObserver() {
@@ -71,7 +64,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
boolean isDoingExtensiveChanges) {
// If the folder is removed in folder mode, show the parent folder or falls back to all
// bookmarks mode.
- if (getCurrentState() == UIState.STATE_FOLDER
+ if (getCurrentState() == EnhancedBookmarkUIState.STATE_FOLDER
&& node.getId().equals(mStateStack.peek().mFolder)) {
if (mEnhancedBookmarksModel.getTopLevelFolderIDs(true, true).contains(
node.getId())) {
@@ -90,21 +83,29 @@ 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.
- if (getCurrentState() == UIState.STATE_FOLDER) {
+ if (getCurrentState() == EnhancedBookmarkUIState.STATE_FOLDER) {
setState(mStateStack.peek());
}
clearSelection();
}
};
+ 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(EnhancedBookmarkUIState.createStateFromUrl(mInitialUrl,
+ mEnhancedBookmarksModel));
+ }
+ }
+ };
+
/**
* Creates an instance of {@link EnhancedBookmarkManager}. It also initializes resources,
* bookmark models and jni bridges.
@@ -123,7 +124,8 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
((SnackbarManageable) activity).getSnackbarManager());
mSearchView = (EnhancedBookmarkSearchView) getView().findViewById(R.id.eb_search_view);
mEnhancedBookmarksModel.addObserver(mBookmarkModelObserver);
- initializeIfBookmarkModelLoaded();
+ initializeToLoadingState();
+ mEnhancedBookmarksModel.runAfterBookmarkModelLoaded(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
@@ -204,75 +206,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(EnhancedBookmarkUIState.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.
+ * 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 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.
- */
- 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(EnhancedBookmarkUIState.createLoadingState());
}
/**
@@ -288,41 +248,30 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
* and back button are not controlled by the manager: the tab handles back key and backstack
* navigation.
*/
- private void setState(UIState state) {
+ private void setState(EnhancedBookmarkUIState state) {
if (!state.isValid(mEnhancedBookmarksModel)) {
- state = UIState.createAllBookmarksState(mEnhancedBookmarksModel);
+ state = EnhancedBookmarkUIState.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 == EnhancedBookmarkUIState.STATE_LOADING) {
+ mStateStack.pop();
}
mStateStack.push(state);
- if (state.mState != UIState.STATE_LOADING) {
+
+ if (state.mState != EnhancedBookmarkUIState.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();
@@ -337,18 +286,18 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
@Override
public void openFolder(BookmarkId folder) {
closeSearchUI();
- setState(UIState.createFolderState(folder, mEnhancedBookmarksModel));
+ setState(EnhancedBookmarkUIState.createFolderState(folder, mEnhancedBookmarksModel));
}
@Override
public void openAllBookmarks() {
closeSearchUI();
- setState(UIState.createAllBookmarksState(mEnhancedBookmarksModel));
+ setState(EnhancedBookmarkUIState.createAllBookmarksState(mEnhancedBookmarksModel));
}
@Override
public void openFilter(EnhancedBookmarkFilter filter) {
- setState(UIState.createFilterState(filter, mEnhancedBookmarksModel));
+ setState(EnhancedBookmarkUIState.createFilterState(filter, mEnhancedBookmarksModel));
}
@Override
@@ -391,18 +340,18 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
public void notifyStateChange(EnhancedBookmarkUIObserver observer) {
int state = getCurrentState();
switch (state) {
- case UIState.STATE_ALL_BOOKMARKS:
+ case EnhancedBookmarkUIState.STATE_ALL_BOOKMARKS:
observer.onAllBookmarksStateSet();
break;
- case UIState.STATE_FOLDER:
+ case EnhancedBookmarkUIState.STATE_FOLDER:
observer.onFolderStateSet(mStateStack.peek().mFolder);
break;
- case UIState.STATE_LOADING:
+ case EnhancedBookmarkUIState.STATE_LOADING:
// In loading state, onEnhancedBookmarkDelegateInitialized() is not called for all
// UIObservers, which means that there will be no observers at the time. Do nothing.
assert mUIObservers.isEmpty();
break;
- case UIState.STATE_FILTER:
+ case EnhancedBookmarkUIState.STATE_FILTER:
observer.onFilterStateSet(mStateStack.peek().mFilter);
break;
default:
@@ -466,7 +415,7 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
@Override
public int getCurrentState() {
- if (mStateStack.isEmpty()) return UIState.STATE_LOADING;
+ if (mStateStack.isEmpty()) return EnhancedBookmarkUIState.STATE_LOADING;
return mStateStack.peek().mState;
}
@@ -479,132 +428,4 @@ public class EnhancedBookmarkManager implements EnhancedBookmarkDelegate {
public SnackbarManager getSnackbarManager() {
return ((SnackbarManageable) mActivity).getSnackbarManager();
}
-
- /**
- * 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.
- */
- static class UIState {
- private static final int STATE_INVALID = 0;
- static final int STATE_LOADING = 1;
- static final int STATE_ALL_BOOKMARKS = 2;
- 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;
- private BookmarkId mFolder;
- private EnhancedBookmarkFilter mFilter;
-
- private static UIState createLoadingState(String url) {
- UIState state = new UIState();
- state.mUrl = url;
- state.mState = STATE_LOADING;
- return state;
- }
-
- private static UIState createAllBookmarksState(EnhancedBookmarksModel bookmarkModel) {
- return createStateFromUrl(UrlConstants.BOOKMARKS_URL, bookmarkModel);
- }
-
- private static UIState createFolderState(BookmarkId folder,
- EnhancedBookmarksModel bookmarkModel) {
- return createStateFromUrl(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 A state corresponding to the url. If the url is not valid, return all_bookmarks.
- */
- private static UIState createStateFromUrl(String url,
- EnhancedBookmarksModel bookmarkModel) {
- UIState state = new UIState();
- state.mState = STATE_INVALID;
- state.mUrl = 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);
- 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);
- if (!suffix.isEmpty()) {
- state.mState = STATE_FILTER;
- state.mFilter = EnhancedBookmarkFilter.valueOf(suffix);
- }
- }
-
- if (!state.isValid(bookmarkModel)) {
- state.mState = STATE_ALL_BOOKMARKS;
- state.mUrl = UrlConstants.BOOKMARKS_URL;
- }
-
- return state;
- }
-
- private UIState() {
- }
-
- /**
- * @return Whether this state is valid.
- */
- private boolean isValid(EnhancedBookmarksModel bookmarkModel) {
- if (mUrl == null || mState == STATE_INVALID) return false;
-
- if (mState == STATE_FOLDER) {
- if (mFolder == null) return false;
-
- return bookmarkModel.doesBookmarkExist(mFolder)
- && !mFolder.equals(bookmarkModel.getRootFolderId());
- }
-
- 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;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (!(obj instanceof UIState)) return false;
- UIState other = (UIState) obj;
- return mState == other.mState && mUrl.equals(other.mUrl);
- }
- }
}

Powered by Google App Engine
This is Rietveld 408576698