[Enhanced Bookmark]Rewrite initialization logic

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

Index: chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUIState.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUIState.java b/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUIState.java
new file mode 100644
index 0000000000000000000000000000000000000000..a198631518b795976cb895f09ed8205624e18eac
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUIState.java
@@ -0,0 +1,154 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.enhancedbookmarks;
+
+import android.net.Uri;
+import android.text.TextUtils;
+
+import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.UrlConstants;
+import org.chromium.components.bookmarks.BookmarkId;
+
+/**
+ * A class representing the UI state of the {@link EnhancedBookmarkManager}. All
+ * states can be uniquely identified by a URL.
+ */
+class EnhancedBookmarkUIState {
+    static final String URI_PERSIST_QUERY_NAME = "should_persist";

[newt (away), 2015/12/02 16:21:13]

maybe just call this "persist"

[Ian Wen, 2015/12/03 02:33:23]

Done.

+    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;
+
+    /**
+     * One of the STATE_* constants.
+     */
+    int mState;
+    String mUrl;
+    /** Whether this state should be persisted as user's last location. */
+    boolean mShouldPersist = true;
+    BookmarkId mFolder;
+    EnhancedBookmarkFilter mFilter;
+
+    static EnhancedBookmarkUIState createLoadingState() {
+        EnhancedBookmarkUIState state = new EnhancedBookmarkUIState();
+        state.mState = STATE_LOADING;
+        state.mShouldPersist = false;
+        state.mUrl = "";
+        return state;
+    }
+
+    static EnhancedBookmarkUIState createAllBookmarksState(EnhancedBookmarksModel bookmarkModel) {
+        return createStateFromUrl(UrlConstants.BOOKMARKS_URL, bookmarkModel);
+    }
+
+    static EnhancedBookmarkUIState createFolderState(BookmarkId folder,
+            EnhancedBookmarksModel bookmarkModel) {
+        return createStateFromUrl(createFolderUrl(folder), bookmarkModel);
+    }
+
+    static EnhancedBookmarkUIState createFilterState(
+            EnhancedBookmarkFilter filter, EnhancedBookmarksModel bookmarkModel) {
+        return createStateFromUrl(createFilterUrl(filter, true), bookmarkModel);
+    }
+
+    /**
+     * @return A state corresponding to the url. If the url is not valid, return all_bookmarks.
+     */
+    static EnhancedBookmarkUIState createStateFromUrl(String url,

[newt (away), 2015/12/02 16:21:12]

you could change this to accept a Uri, to eliminate duplicate Uri parsing in
this class

[Ian Wen, 2015/12/03 02:33:23]

Done.

+            EnhancedBookmarksModel bookmarkModel) {
+        EnhancedBookmarkUIState state = new EnhancedBookmarkUIState();
+        state.mState = STATE_INVALID;
+        state.mUrl = url;
+        state.mShouldPersist = shouldPersist(url);
+
+        if (url.equals(UrlConstants.BOOKMARKS_URL)) {
+            state.mState = STATE_ALL_BOOKMARKS;
+        } else if (url.startsWith(UrlConstants.BOOKMARKS_FOLDER_URL)) {
+            String path = Uri.parse(url).getLastPathSegment();
+            if (!path.isEmpty()) {
+                state.mFolder = BookmarkId.getBookmarkIdFromString(path);
+                state.mState = STATE_FOLDER;
+            }
+        } else if (url.startsWith(UrlConstants.BOOKMARKS_FILTER_URL)) {
+            String path = Uri.parse(url).getLastPathSegment();
+            if (!path.isEmpty()) {
+                state.mState = STATE_FILTER;
+                state.mFilter = EnhancedBookmarkFilter.valueOf(path);
+            }
+        }
+
+        if (!state.isValid(bookmarkModel)) {
+            state.mState = STATE_ALL_BOOKMARKS;
+            state.mUrl = UrlConstants.BOOKMARKS_URL;
+        }
+
+        return state;
+    }
+
+    private EnhancedBookmarkUIState() {}
+
+    /**
+     * @return Whether this state is valid.
+     */
+    boolean isValid(EnhancedBookmarksModel bookmarkModel) {
+        if (mUrl == null || mState == STATE_INVALID) return false;
+
+        if (mState == STATE_FOLDER) {
+            if (mFolder == null) return false;

[newt (away), 2015/12/02 16:21:13]

simpler:

  return mFolder != null && bookmarkModel.doesBookmarkExist(mFolder)
          && !mFolder.equals(bookmarkModel.getRootFolderId());

[Ian Wen, 2015/12/03 02:33:24]

Done.

+
+            return bookmarkModel.doesBookmarkExist(mFolder)
+                    && !mFolder.equals(bookmarkModel.getRootFolderId());
+        }
+
+        if (mState == STATE_FILTER && mFilter == null) return false;
+
+        return true;
+    }
+
+    private static boolean shouldPersist(String url) {

[newt (away), 2015/12/02 16:21:13]

Change the parameter to a Uri to avoid duplicate parsing in createStateFromUrl()

[Ian Wen, 2015/12/03 02:33:23]

Done.

+        Uri uri = Uri.parse(url);
+        String queryString = uri.getQueryParameter(URI_PERSIST_QUERY_NAME);
+        if (queryString == null) return true;

[newt (away), 2015/12/02 16:21:13]

don't need this line

[Ian Wen, 2015/12/03 02:33:23]

Done.

+        return !("0".equals(queryString));
+    }
+
+    @Override
+    public int hashCode() {
+        return 31 * mUrl.hashCode() + mState;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (!(obj instanceof EnhancedBookmarkUIState)) return false;
+        EnhancedBookmarkUIState other = (EnhancedBookmarkUIState) obj;
+        return mState == other.mState && TextUtils.equals(mUrl, other.mUrl);
+    }
+
+    static String createFolderUrl(BookmarkId folderId) {

[newt (away), 2015/12/02 16:21:13]

Could you reorder the methods in this class in a logical way? E.g. put all the
static methods together near the top, put the private methods at the bottom, etc

[Ian Wen, 2015/12/03 02:33:23]

Done.

+        return encodePathToUrl(UrlConstants.BOOKMARKS_FOLDER_URL, folderId.toString(), true);
+    }
+
+    static String createFilterUrl(EnhancedBookmarkFilter filter, boolean shouldPersist) {
+        return encodePathToUrl(UrlConstants.BOOKMARKS_FILTER_URL, filter.value, shouldPersist);
+    }
+
+    /**
+     * Encodes the suffix and append it to the url as path. A simple appending
+     * does not work because there might be spaces in suffix.
+     * @param shouldPersist Whether this url should be saved to preferences as
+     *            user's last location.
+     */
+    @VisibleForTesting

[newt (away), 2015/12/02 16:21:12]

remove @VisibleForTesting

[Ian Wen, 2015/12/03 02:33:24]

Done.

+    private static String encodePathToUrl(String url, String suffix, boolean shouldPersist) {

[newt (away), 2015/12/02 16:21:13]

rename this "createUrl(String baseUrl, String pathSuffix, boolean
shouldPersist)"?

[Ian Wen, 2015/12/03 02:33:23]

Done.

+        Uri.Builder builder = Uri.parse(url).buildUpon();
+        builder.appendPath(suffix);
+        if (!shouldPersist) {
+            builder.appendQueryParameter(URI_PERSIST_QUERY_NAME, "0");
+        }
+        return builder.build().toString();
+    }
+}