Bug 1038354 - Exit editing mode before quick-switching to tab. r=lucasr

mobile/android/base/BrowserApp.java

@@ -27,7 +27,6 @@ import org.mozilla.gecko.db.BrowserContract.Combined;
 import org.mozilla.gecko.db.BrowserContract.ReadingListItems;
 import org.mozilla.gecko.db.BrowserContract.SearchHistory;
 import org.mozilla.gecko.db.BrowserDB;
-import org.mozilla.gecko.db.DBUtils;
 import org.mozilla.gecko.db.SuggestedSites;
 import org.mozilla.gecko.distribution.Distribution;
 import org.mozilla.gecko.favicons.Favicons;
@@ -47,6 +46,7 @@ import org.mozilla.gecko.health.SessionInformation;
 import org.mozilla.gecko.home.BrowserSearch;
 import org.mozilla.gecko.home.HomeBanner;
 import org.mozilla.gecko.home.HomePager;
+import org.mozilla.gecko.home.HomePager.OnUrlOpenInBackgroundListener;
 import org.mozilla.gecko.home.HomePager.OnUrlOpenListener;
 import org.mozilla.gecko.home.HomePanelsManager;
 import org.mozilla.gecko.home.SearchEngine;
@@ -74,6 +74,7 @@ import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.util.UIAsyncTask;
 import org.mozilla.gecko.widget.ButtonToast;
+import org.mozilla.gecko.widget.ButtonToast.ToastListener;
 import org.mozilla.gecko.widget.GeckoActionProvider;
 
 import android.app.Activity;
@@ -139,6 +140,7 @@ public class BrowserApp extends GeckoApp
                                    BrowserSearch.OnEditSuggestionListener,
                                    HomePager.OnNewTabsListener,
                                    OnUrlOpenListener,
+                                   OnUrlOpenInBackgroundListener,
                                    ActionModeCompat.Presenter,
                                    LayoutInflater.Factory {
     private static final String LOGTAG = "GeckoBrowserApp";
@@ -1771,7 +1773,13 @@ public class BrowserApp extends GeckoApp
 
     /**
      * Attempts to switch to an open tab with the given URL.
+     * <p>
+     * If the tab exists, this method cancels any in-progress editing as well as
+     * calling {@link Tabs#selectTab(int)}.
      *
+     * @param url of tab to switch to.
+     * @param flags to obey: if {@link OnUrlOpenListener.Flags#ALLOW_SWITCH_TO_TAB}
+     *        is not present, return false.
      * @return true if we successfully switched to a tab, false otherwise.
      */
     private boolean maybeSwitchToTab(String url, EnumSet<OnUrlOpenListener.Flags> flags) {
@@ -1792,6 +1800,26 @@ public class BrowserApp extends GeckoApp
             return false;
         }
 
+        return maybeSwitchToTab(tab.getId());
+    }
+
+    /**
+     * Attempts to switch to an open tab with the given unique tab ID.
+     * <p>
+     * If the tab exists, this method cancels any in-progress editing as well as
+     * calling {@link Tabs#selectTab(int)}.
+     *
+     * @param id of tab to switch to.
+     * @return true if we successfully switched to the tab, false otherwise.
+     */
+    private boolean maybeSwitchToTab(int id) {
+        final Tabs tabs = Tabs.getInstance();
+        final Tab tab = tabs.getTab(id);
+
+        if (tab == null) {
+            return false;
+        }
+
         // Set the target tab to null so it does not get selected (on editing
         // mode exit) in lieu of the tab we are about to select.
         mTargetTabForEditingMode = null;
@@ -3088,6 +3116,53 @@ public class BrowserApp extends GeckoApp
         }
     }
 
+    // HomePager.OnUrlOpenInBackgroundListener
+    @Override
+    public void onUrlOpenInBackground(final String url, EnumSet<OnUrlOpenInBackgroundListener.Flags> flags) {
+        if (url == null) {
+            throw new IllegalArgumentException("url must not be null");
+        }
+        if (flags == null) {
+            throw new IllegalArgumentException("flags must not be null");
+        }
+
+        final boolean isPrivate = flags.contains(OnUrlOpenInBackgroundListener.Flags.PRIVATE);
+
+        int loadFlags = Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_BACKGROUND;
+        if (isPrivate) {
+            loadFlags |= Tabs.LOADURL_PRIVATE;
+        }
+
+        final Tab newTab = Tabs.getInstance().loadUrl(url, loadFlags);
+
+        // We switch to the desired tab by unique ID, which closes any window
+        // for a race between opening the tab and closing it, and switching to
+        // it. We could also switch to the Tab explicitly, but we don't want to
+        // hold a reference to the Tab itself in the anonymous listener class.
+        final int newTabId = newTab.getId();
+
+        final ToastListener listener = new ButtonToast.ToastListener() {
+            @Override
+            public void onButtonClicked() {
+                maybeSwitchToTab(newTabId);
+            }
+
+            @Override
+            public void onToastHidden(ButtonToast.ReasonHidden reason) { }
+        };
+
+        final String message = isPrivate ?
+                getResources().getString(R.string.new_private_tab_opened) :
+                getResources().getString(R.string.new_tab_opened);
+        final String buttonMessage = getResources().getString(R.string.switch_button_message);
+        getButtonToast().show(false,
+                              message,
+                              ButtonToast.LENGTH_SHORT,
+                              buttonMessage,
+                              R.drawable.switch_button_icon,
+                              listener);
+    }
+
     // BrowserSearch.OnSearchListener
     @Override
     public void onSearch(SearchEngine engine, String text) {

mobile/android/base/home/HomeFragment.java

@@ -5,30 +5,29 @@
 
 package org.mozilla.gecko.home;
 
+import java.util.EnumSet;
+
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.EditBookmarkDialog;
-import org.mozilla.gecko.GeckoApp;
 import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.GeckoEvent;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.ReaderModeUtils;
-import org.mozilla.gecko.Tab;
-import org.mozilla.gecko.Tabs;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.db.BrowserContract.SuggestedSites;
 import org.mozilla.gecko.db.BrowserDB;
 import org.mozilla.gecko.favicons.Favicons;
 import org.mozilla.gecko.home.HomeContextMenuInfo.RemoveItemType;
+import org.mozilla.gecko.home.HomePager.OnUrlOpenInBackgroundListener;
 import org.mozilla.gecko.home.HomePager.OnUrlOpenListener;
 import org.mozilla.gecko.home.TopSitesGridView.TopSitesGridContextMenuInfo;
 import org.mozilla.gecko.util.Clipboard;
 import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.gecko.util.UIAsyncTask;
-import org.mozilla.gecko.widget.ButtonToast;
 
 import android.app.Activity;
 import android.content.ContentResolver;
@@ -72,6 +71,9 @@ public abstract class HomeFragment extends Fragment {
     // On URL open listener
     protected OnUrlOpenListener mUrlOpenListener;
 
+    // Helper for opening a tab in the background.
+    private OnUrlOpenInBackgroundListener mUrlOpenInBackgroundListener;
+
     @Override
     public void onAttach(Activity activity) {
         super.onAttach(activity);
@@ -82,12 +84,20 @@ public abstract class HomeFragment extends Fragment {
             throw new ClassCastException(activity.toString()
                     + " must implement HomePager.OnUrlOpenListener");
         }
+
+        try {
+            mUrlOpenInBackgroundListener = (OnUrlOpenInBackgroundListener) activity;
+        } catch (ClassCastException e) {
+            throw new ClassCastException(activity.toString()
+                    + " must implement HomePager.OnUrlOpenInBackgroundListener");
+        }
     }
 
     @Override
     public void onDetach() {
         super.onDetach();
         mUrlOpenListener = null;
+        mUrlOpenInBackgroundListener = null;
     }
 
     @Override
@@ -205,40 +215,23 @@ public abstract class HomeFragment extends Fragment {
                 return false;
             }
 
-            int flags = Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_BACKGROUND;
-            final boolean isPrivate = (item.getItemId() == R.id.home_open_private_tab);
-            if (isPrivate) {
-                flags |= Tabs.LOADURL_PRIVATE;
+            // Some pinned site items have "user-entered" urls. URLs entered in
+            // the PinSiteDialog are wrapped in a special URI until we can get a
+            // valid URL. If the url is a user-entered url, decode the URL
+            // before loading it.
+            final String url = StringUtils.decodeUserEnteredUrl(info.isInReadingList()
+                    ? ReaderModeUtils.getAboutReaderForUrl(info.url)
+                    : info.url);
+
+            final EnumSet<OnUrlOpenInBackgroundListener.Flags> flags = EnumSet.noneOf(OnUrlOpenInBackgroundListener.Flags.class);
+            if (item.getItemId() == R.id.home_open_private_tab) {
+                flags.add(OnUrlOpenInBackgroundListener.Flags.PRIVATE);
             }
 
+            mUrlOpenInBackgroundListener.onUrlOpenInBackground(url, flags);
+
             Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.CONTEXT_MENU);
 
-            final String url = (info.isInReadingList() ? ReaderModeUtils.getAboutReaderForUrl(info.url) : info.url);
-
-            // Some pinned site items have "user-entered" urls. URLs entered in the PinSiteDialog are wrapped in
-            // a special URI until we can get a valid URL. If the url is a user-entered url, decode the URL before loading it.
-            final Tab newTab = Tabs.getInstance().loadUrl(StringUtils.decodeUserEnteredUrl(url), flags);
-            final int newTabId = newTab.getId(); // We don't want to hold a reference to the Tab.
-
-            final String message = isPrivate ?
-                    getResources().getString(R.string.new_private_tab_opened) :
-                    getResources().getString(R.string.new_tab_opened);
-            final String buttonMessage = getResources().getString(R.string.switch_button_message);
-            final GeckoApp geckoApp = (GeckoApp) context;
-            geckoApp.getButtonToast().show(false,
-                    message,
-                    ButtonToast.LENGTH_SHORT,
-                    buttonMessage,
-                    R.drawable.switch_button_icon,
-                    new ButtonToast.ToastListener() {
-                        @Override
-                        public void onButtonClicked() {
-                            Tabs.getInstance().selectTab(newTabId);
-                        }
-
-                        @Override
-                        public void onToastHidden(ButtonToast.ReasonHidden reason) { }
-                    });
             return true;
         }
 

mobile/android/base/home/HomePager.java

@@ -81,6 +81,27 @@ public class HomePager extends ViewPager {
         public void onUrlOpen(String url, EnumSet<Flags> flags);
     }
 
+    /**
+     * Interface for requesting a new tab be opened in the background.
+     * <p>
+     * This is the <code>HomeFragment</code> equivalent of opening a new tab by
+     * long clicking a link and selecting the "Open new [private] tab" context
+     * menu option.
+     */
+    public interface OnUrlOpenInBackgroundListener {
+        public enum Flags {
+            PRIVATE,
+        }
+
+        /**
+         * Open a new tab with the given URL
+         *
+         * @param url to open.
+         * @param flags to open new tab with.
+         */
+        public void onUrlOpenInBackground(String url, EnumSet<Flags> flags);
+    }
+
     public interface OnNewTabsListener {
         public void onNewTabs(List<String> urls);
     }