Bug 977167 - "Open tabs should be filtered to exclude about:, chrome: etc. URLs prior to flushing to DB" r=rnewman
authorvivek <vivekb.balakrishnan@gmail.com>
Thu, 24 Apr 2014 14:06:00 +0200
changeset 180543 f5712360fc1a9af29e239f80fefc28e0b2e96382
parent 180542 532886a149ab30aa92dc91a1da81dbf0a30530e6
child 180544 70fdf06731b8c25729527cc09ad05910b64de0a3
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersrnewman
bugs977167
milestone31.0a1
Bug 977167 - "Open tabs should be filtered to exclude about:, chrome: etc. URLs prior to flushing to DB" r=rnewman
mobile/android/base/TabsAccessor.java
mobile/android/base/tests/robocop.ini
mobile/android/base/tests/testFilterOpenTab.java
--- a/mobile/android/base/TabsAccessor.java
+++ b/mobile/android/base/TabsAccessor.java
@@ -16,16 +16,17 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
 import android.net.Uri;
 import android.util.Log;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.regex.Pattern;
 
 public final class TabsAccessor {
     private static final String LOGTAG = "GeckoTabsAccessor";
 
     private static final String[] CLIENTS_AVAILABILITY_PROJECTION = new String[] {
                                                                         BrowserContract.Clients.GUID
                                                                     };
 
@@ -44,16 +45,17 @@ public final class TabsAccessor {
         NAME
     };
 
     private static final String CLIENTS_SELECTION = BrowserContract.Clients.GUID + " IS NOT NULL";
     private static final String TABS_SELECTION = BrowserContract.Tabs.CLIENT_GUID + " IS NOT NULL";
 
     private static final String LOCAL_CLIENT_SELECTION = BrowserContract.Clients.GUID + " IS NULL";
     private static final String LOCAL_TABS_SELECTION = BrowserContract.Tabs.CLIENT_GUID + " IS NULL";
+    private static final Pattern FILTERED_URL_PATTERN = Pattern.compile("^(about|chrome|wyciwyg|file):.*$");
 
     public static class RemoteTab {
         public String title;
         public String url;
         public String guid;
         public String name;
     }
 
@@ -144,19 +146,19 @@ public final class TabsAccessor {
      */
     private static void insertLocalTabs(final ContentResolver cr, final Iterable<Tab> tabs) {
         // Reuse this for serializing individual history URLs as JSON.
         JSONArray history = new JSONArray();
         ArrayList<ContentValues> valuesToInsert = new ArrayList<ContentValues>();
 
         int position = 0;
         for (Tab tab : tabs) {
-            // Skip this tab if it has a null URL or is in private browsing mode
+            // Skip this tab if it has a null URL or is in private browsing mode, or is a filtered URL.
             String url = tab.getURL();
-            if (url == null || tab.isPrivate())
+            if (url == null || tab.isPrivate() || isFilteredURL(url))
                 continue;
 
             ContentValues values = new ContentValues();
             values.put(BrowserContract.Tabs.URL, url);
             values.put(BrowserContract.Tabs.TITLE, tab.getTitle());
             values.put(BrowserContract.Tabs.LAST_USED, tab.getLastUsed());
 
             String favicon = tab.getFaviconURL();
@@ -187,9 +189,18 @@ public final class TabsAccessor {
     }
 
     // Deletes all local tabs and replaces them with a new list of tabs.
     public static synchronized void persistLocalTabs(final ContentResolver cr, final Iterable<Tab> tabs) {
         deleteLocalTabs(cr);
         insertLocalTabs(cr, tabs);
         updateLocalClient(cr);
     }
+
+    /**
+     * Matches the supplied URL string against the set of URLs to filter.
+     *
+     * @return true if the supplied URL should be skipped; false otherwise.
+     */
+    private static boolean isFilteredURL(String url) {
+        return FILTERED_URL_PATTERN.matcher(url).matches();
+    }
 }
--- a/mobile/android/base/tests/robocop.ini
+++ b/mobile/android/base/tests/robocop.ini
@@ -27,16 +27,17 @@ skip-if = android_version == "10"
 # disabled on 2.3; bug 979603
 skip-if = android_version == "10"
 [testBrowserSearchVisibility]
 [testClearPrivateData]
 # disabled on x86 and 2.3; bug 948591
 skip-if = android_version == "10" || processor == "x86"
 [testDistribution]
 [testDoorHanger]
+[testFilterOpenTab]
 # disabled on 2.3; bug 986172
 skip-if = android_version == "10"
 [testFindInPage]
 # disabled on Android 2.3; bug 975155
 skip-if = android_version == "10"
 [testFlingCorrectness]
 # disabled on x86 only; bug 927476
 skip-if = processor == "x86"
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/tests/testFilterOpenTab.java
@@ -0,0 +1,125 @@
+package org.mozilla.gecko.tests;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.Callable;
+
+import org.mozilla.gecko.PrivateTab;
+import org.mozilla.gecko.Tab;
+import org.mozilla.gecko.TabsAccessor;
+import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.db.TabsProvider;
+
+import android.content.ContentProvider;
+import android.content.Context;
+import android.database.Cursor;
+
+/**
+ * Tests that local tabs are filtered prior to upload.
+ * - create a set of tabs and perists them through TabsAccessor.
+ * - verifies that tabs are filtered by querying.
+ */
+public class testFilterOpenTab extends ContentProviderTest {
+    private static final String[] TABS_PROJECTION_COLUMNS = new String[] {
+                                                                BrowserContract.Tabs.TITLE,
+                                                                BrowserContract.Tabs.URL,
+                                                                BrowserContract.Clients.GUID,
+                                                                BrowserContract.Clients.NAME
+                                                            };
+
+    private static final String LOCAL_TABS_SELECTION = BrowserContract.Tabs.CLIENT_GUID + " IS NULL";
+
+    /**
+     * Factory function that makes new ContentProvider instances.
+     * <p>
+     * We want a fresh provider each test, so this should be invoked in
+     * <code>setUp</code> before each individual test.
+     */
+    protected static Callable<ContentProvider> sTabProviderCallable = new Callable<ContentProvider>() {
+        @Override
+        public ContentProvider call() {
+            return new TabsProvider();
+        }
+    };
+
+    private Cursor getTabsFromLocalClient() throws Exception {
+        return mProvider.query(BrowserContract.Tabs.CONTENT_URI,
+                               TABS_PROJECTION_COLUMNS,
+                               LOCAL_TABS_SELECTION,
+                               null,
+                               null);
+    }
+
+    private Tab createTab(int id, String url, boolean external, int parentId, String title) {
+        return new Tab((Context) getActivity(), id, url, external, parentId, title);
+    }
+
+    private Tab createPrivateTab(int id, String url, boolean external, int parentId, String title) {
+        return new PrivateTab((Context) getActivity(), id, url, external, parentId, title);
+    }
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp(sTabProviderCallable, BrowserContract.TABS_AUTHORITY, "tabs.db");
+        mTests.add(new TestInsertLocalTabs());
+    }
+
+    public void testFilterOpenTab() throws Exception {
+        for (int i = 0; i < mTests.size(); i++) {
+            Runnable test = mTests.get(i);
+
+            setTestName(test.getClass().getSimpleName());
+            test.run();
+        }
+    }
+
+    private class TestInsertLocalTabs extends TestCase  {
+        @Override
+        public void test() throws Exception {
+            final String TITLE1 = "Google";
+            final String URL1 = "http://www.google.com/";
+            final String TITLE2 = "Mozilla Start Page";
+            final String URL2 = "about:home";
+            final String TITLE3 = "Chrome Weave URL";
+            final String URL3 = "chrome://weave/";
+            final String TITLE4 = "What You Cache Is What You Get";
+            final String URL4 = "wyciwyg://1/test.com";
+            final String TITLE5 = "Root Folder";
+            final String URL5 = "file:///";
+
+            // Create a list of local tabs.
+            List<Tab> tabs = new ArrayList<Tab>(6);
+            Tab tab1 = createTab(1, URL1, false, 0, TITLE1);
+            Tab tab2 = createTab(2, URL2, false, 0, TITLE2);
+            Tab tab3 = createTab(3, URL3, false, 0, TITLE3);
+            Tab tab4 = createTab(4, URL4, false, 0, TITLE4);
+            Tab tab5 = createTab(5, URL5, false, 0, TITLE5);
+            Tab tab6 = createPrivateTab(6, URL1, false, 0, TITLE1);
+            tabs.add(tab1);
+            tabs.add(tab2);
+            tabs.add(tab3);
+            tabs.add(tab4);
+            tabs.add(tab5);
+            tabs.add(tab6);
+
+            // Persist the created tabs.
+            TabsAccessor.persistLocalTabs(mResolver, tabs);
+
+            // Get the persisted tab and check if urls are filtered.
+            Cursor c = getTabsFromLocalClient();
+            assertCountIsAndClose(c, 1, 1 + " tabs entries found");
+        }
+    }
+
+    /**
+     * Assert that the provided cursor has the expected number of rows,
+     * closing the cursor afterwards.
+     */
+    private void assertCountIsAndClose(Cursor c, int expectedCount, String message) {
+        try {
+            mAsserter.is(c.getCount(), expectedCount, message);
+        } finally {
+            c.close();
+        }
+    }
+}