Bug 1021055 - Add Sync client device type to Fennec's tabs provider. r=rnewman
authorNick Alexander <nalexander@mozilla.com>
Wed, 11 Jun 2014 14:20:24 -0700
changeset 188321 281e6fc177c45776ee98869d9e810d45a9de26d8
parent 188320 2bd196dc1b2c631fdc80ab04ff75df077371aeb4
child 188322 8ef009362d13c3fe164c865d8b9d9766767673b1
push id26951
push useremorley@mozilla.com
push dateThu, 12 Jun 2014 14:07:43 +0000
treeherdermozilla-central@4f98802de6ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1021055
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1021055 - Add Sync client device type to Fennec's tabs provider. r=rnewman This is the expedient thing to do. The right thing to do is to deprecate Sync's clients database in favour of Fennec's clients table, but that's a good chunk of work for a small gain.
mobile/android/base/db/BrowserContract.java
mobile/android/base/db/TabsProvider.java
mobile/android/base/sync/repositories/android/FennecTabsRepository.java
mobile/android/tests/background/junit3/src/db/TestFennecTabsRepositorySession.java
--- a/mobile/android/base/db/BrowserContract.java
+++ b/mobile/android/base/db/BrowserContract.java
@@ -323,16 +323,18 @@ public class BrowserContract {
         public static final String NAME = "name";
 
         // Sync-assigned GUID for client device. NULL for local tabs.
         public static final String GUID = "guid";
 
         // Last modified time for the client's tab record. For remote records, a server
         // timestamp provided by Sync during insertion.
         public static final String LAST_MODIFIED = "last_modified";
+
+        public static final String DEVICE_TYPE = "device_type";
     }
 
     // Data storage for dynamic panels on about:home
     @RobocopTarget
     public static final class HomeItems implements CommonColumns {
         private HomeItems() {}
         public static final Uri CONTENT_FAKE_URI = Uri.withAppendedPath(HOME_AUTHORITY_URI, "items/fake");
         public static final Uri CONTENT_URI = Uri.withAppendedPath(HOME_AUTHORITY_URI, "items");
--- a/mobile/android/base/db/TabsProvider.java
+++ b/mobile/android/base/db/TabsProvider.java
@@ -21,17 +21,17 @@ import android.database.sqlite.SQLiteOpe
 import android.database.sqlite.SQLiteQueryBuilder;
 import android.net.Uri;
 import android.os.Build;
 import android.text.TextUtils;
 
 public class TabsProvider extends PerProfileDatabaseProvider<TabsProvider.TabsDatabaseHelper> {
     static final String DATABASE_NAME = "tabs.db";
 
-    static final int DATABASE_VERSION = 2;
+    static final int DATABASE_VERSION = 3;
 
     static final String TABLE_TABS = "tabs";
     static final String TABLE_CLIENTS = "clients";
 
     static final int TABS = 600;
     static final int TABS_ID = 601;
     static final int CLIENTS = 602;
     static final int CLIENTS_ID = 603;
@@ -62,22 +62,24 @@ public class TabsProvider extends PerPro
         map.put(Tabs.URL, Tabs.URL);
         map.put(Tabs.HISTORY, Tabs.HISTORY);
         map.put(Tabs.FAVICON, Tabs.FAVICON);
         map.put(Tabs.LAST_USED, Tabs.LAST_USED);
         map.put(Tabs.POSITION, Tabs.POSITION);
         map.put(Clients.GUID, Clients.GUID);
         map.put(Clients.NAME, Clients.NAME);
         map.put(Clients.LAST_MODIFIED, Clients.LAST_MODIFIED);
+        map.put(Clients.DEVICE_TYPE, Clients.DEVICE_TYPE);
         TABS_PROJECTION_MAP = Collections.unmodifiableMap(map);
 
         map = new HashMap<String, String>();
         map.put(Clients.GUID, Clients.GUID);
         map.put(Clients.NAME, Clients.NAME);
         map.put(Clients.LAST_MODIFIED, Clients.LAST_MODIFIED);
+        map.put(Clients.DEVICE_TYPE, Clients.DEVICE_TYPE);
         CLIENTS_PROJECTION_MAP = Collections.unmodifiableMap(map);
     }
 
     private static final String selectColumn(String table, String column) {
         return table + "." + column + " = ?";
     }
 
     final class TabsDatabaseHelper extends SQLiteOpenHelper {
@@ -109,17 +111,18 @@ public class TabsProvider extends PerPro
                        " ON " + TABLE_TABS + "(" + Tabs.POSITION + ")");
 
             debug("Creating " + TABLE_CLIENTS + " table");
 
             // Table for client's name-guid mapping.
             db.execSQL("CREATE TABLE " + TABLE_CLIENTS + "(" +
                        Clients.GUID + " TEXT PRIMARY KEY," +
                        Clients.NAME + " TEXT," +
-                       Clients.LAST_MODIFIED + " INTEGER" +
+                       Clients.LAST_MODIFIED + " INTEGER," +
+                       Clients.DEVICE_TYPE + " TEXT" +
                        ");");
 
             // Index on GUID.
             db.execSQL("CREATE INDEX " + INDEX_CLIENTS_GUID +
                        " ON " + TABLE_CLIENTS + "(" + Clients.GUID + ")");
 
             createLocalClient(db);
         }
@@ -128,28 +131,40 @@ public class TabsProvider extends PerPro
         private void createLocalClient(SQLiteDatabase db) {
             debug("Inserting local Fennec client into " + TABLE_CLIENTS + " table");
 
             ContentValues values = new ContentValues();
             values.put(BrowserContract.Clients.LAST_MODIFIED, System.currentTimeMillis());
             db.insertOrThrow(TABLE_CLIENTS, null, values);
         }
 
+        protected void upgradeDatabaseFrom2to3(SQLiteDatabase db) {
+            debug("Setting remote client device types to 'mobile' in " + TABLE_CLIENTS + " table");
+
+            // Add type to client, defaulting to mobile. This is correct for our
+            // local client; all remote clients will be updated by Sync.
+            db.execSQL("ALTER TABLE " + TABLE_CLIENTS + " ADD COLUMN " + BrowserContract.Clients.DEVICE_TYPE + " TEXT DEFAULT 'mobile'");
+        }
+
         @Override
         public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
             debug("Upgrading tabs.db: " + db.getPath() + " from " +
                   oldVersion + " to " + newVersion);
 
             // We have to do incremental upgrades until we reach the current
             // database schema version.
             for (int v = oldVersion + 1; v <= newVersion; v++) {
                 switch(v) {
                     case 2:
                         createLocalClient(db);
                         break;
+
+                    case 3:
+                        upgradeDatabaseFrom2to3(db);
+                        break;
                  }
              }
         }
 
         @Override
         public void onOpen(SQLiteDatabase db) {
             debug("Opening tabs.db: " + db.getPath());
             db.rawQuery("PRAGMA synchronous=OFF", null).close();
--- a/mobile/android/base/sync/repositories/android/FennecTabsRepository.java
+++ b/mobile/android/base/sync/repositories/android/FennecTabsRepository.java
@@ -4,27 +4,29 @@
 
 package org.mozilla.gecko.sync.repositories.android;
 
 import java.util.ArrayList;
 
 import org.mozilla.gecko.background.common.log.Logger;
 import org.mozilla.gecko.background.db.Tab;
 import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.db.BrowserContract.Clients;
 import org.mozilla.gecko.sync.delegates.ClientsDataDelegate;
 import org.mozilla.gecko.sync.repositories.InactiveSessionException;
 import org.mozilla.gecko.sync.repositories.NoContentProviderException;
 import org.mozilla.gecko.sync.repositories.NoStoreDelegateException;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.RepositorySession;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionGuidsSinceDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionWipeDelegate;
+import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 import org.mozilla.gecko.sync.repositories.domain.TabsRecord;
 
 import android.content.ContentProviderClient;
 import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
 import android.net.Uri;
@@ -33,46 +35,49 @@ import android.os.RemoteException;
 public class FennecTabsRepository extends Repository {
   protected final ClientsDataDelegate clientsDataDelegate;
 
   public FennecTabsRepository(ClientsDataDelegate clientsDataDelegate) {
     this.clientsDataDelegate = clientsDataDelegate;
   }
 
   /**
-   * Note that — unlike most repositories — this will only fetch Fennec's tabs,
+   * Note that -- unlike most repositories -- this will only fetch Fennec's tabs,
    * and only store tabs from other clients.
    *
    * It will never retrieve tabs from other clients, or store tabs for Fennec,
    * unless you use {@link #fetch(String[], RepositorySessionFetchRecordsDelegate)}
    * and specify an explicit GUID.
    */
   public class FennecTabsRepositorySession extends RepositorySession {
     protected static final String LOG_TAG = "FennecTabsSession";
 
     private final ContentProviderClient tabsProvider;
     private final ContentProviderClient clientsProvider;
 
     protected final RepoUtils.QueryHelper tabsHelper;
 
+    protected final ClientsDatabaseAccessor clientsDatabase;
+
     protected ContentProviderClient getContentProvider(final Context context, final Uri uri) throws NoContentProviderException {
       ContentProviderClient client = context.getContentResolver().acquireContentProviderClient(uri);
       if (client == null) {
         throw new NoContentProviderException(uri);
       }
       return client;
     }
 
     protected void releaseProviders() {
       try {
         clientsProvider.release();
       } catch (Exception e) {}
       try {
         tabsProvider.release();
       } catch (Exception e) {}
+      clientsDatabase.close();
     }
 
     public FennecTabsRepositorySession(Repository repository, Context context) throws NoContentProviderException {
       super(repository);
       clientsProvider = getContentProvider(context, BrowserContractHelpers.CLIENTS_CONTENT_URI);
       try {
         tabsProvider = getContentProvider(context, BrowserContractHelpers.TABS_CONTENT_URI);
       } catch (NoContentProviderException e) {
@@ -80,16 +85,17 @@ public class FennecTabsRepository extend
         throw e;
       } catch (Exception e) {
         clientsProvider.release();
         // Oh, Java.
         throw new RuntimeException(e);
       }
 
       tabsHelper = new RepoUtils.QueryHelper(context, BrowserContractHelpers.TABS_CONTENT_URI, LOG_TAG);
+      clientsDatabase = new ClientsDatabaseAccessor(context);
     }
 
     @Override
     public void abort() {
       releaseProviders();
       super.abort();
     }
 
@@ -235,16 +241,22 @@ public class FennecTabsRepository extend
                 delegate.onRecordStoreFailed(e, record.guid);
               }
               return;
             }
 
             // If it exists, update the client record; otherwise insert.
             final ContentValues clientsCV = tabsRecord.getClientsContentValues();
 
+            final ClientRecord clientRecord = clientsDatabase.fetchClient(tabsRecord.guid);
+            if (null != clientRecord) {
+                // Null is an acceptable device type.
+                clientsCV.put(Clients.DEVICE_TYPE, clientRecord.type);
+            }
+
             Logger.debug(LOG_TAG, "Updating clients provider.");
             final int updated = clientsProvider.update(BrowserContractHelpers.CLIENTS_CONTENT_URI,
                 clientsCV,
                 CLIENT_GUID_IS,
                 selectionArgs);
             if (0 == updated) {
               clientsProvider.insert(BrowserContractHelpers.CLIENTS_CONTENT_URI, clientsCV);
             }
--- a/mobile/android/tests/background/junit3/src/db/TestFennecTabsRepositorySession.java
+++ b/mobile/android/tests/background/junit3/src/db/TestFennecTabsRepositorySession.java
@@ -4,40 +4,47 @@
 package org.mozilla.gecko.background.db;
 
 import org.json.simple.JSONArray;
 import org.mozilla.gecko.background.helpers.AndroidSyncTestCase;
 import org.mozilla.gecko.background.sync.helpers.ExpectFetchDelegate;
 import org.mozilla.gecko.background.sync.helpers.SessionTestHelper;
 import org.mozilla.gecko.background.testhelpers.MockClientsDataDelegate;
 import org.mozilla.gecko.db.BrowserContract;
+import org.mozilla.gecko.db.BrowserContract.Clients;
 import org.mozilla.gecko.sync.repositories.NoContentProviderException;
 import org.mozilla.gecko.sync.repositories.RepositorySession;
 import org.mozilla.gecko.sync.repositories.android.BrowserContractHelpers;
+import org.mozilla.gecko.sync.repositories.android.ClientsDatabaseAccessor;
 import org.mozilla.gecko.sync.repositories.android.FennecTabsRepository;
 import org.mozilla.gecko.sync.repositories.android.FennecTabsRepository.FennecTabsRepositorySession;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionCreationDelegate;
+import org.mozilla.gecko.sync.repositories.domain.ClientRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 import org.mozilla.gecko.sync.repositories.domain.TabsRecord;
 
 import android.content.ContentProviderClient;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.database.Cursor;
 import android.os.RemoteException;
 
 public class TestFennecTabsRepositorySession extends AndroidSyncTestCase {
   public static final MockClientsDataDelegate clientsDataDelegate = new MockClientsDataDelegate();
   public static final String TEST_CLIENT_GUID = clientsDataDelegate.getAccountGUID();
   public static final String TEST_CLIENT_NAME = clientsDataDelegate.getClientName();
+  public static final String TEST_CLIENT_DEVICE_TYPE = "phablet";
 
   // Override these to test against data that is not live.
   public static final String TEST_TABS_CLIENT_GUID_IS_LOCAL_SELECTION = BrowserContract.Tabs.CLIENT_GUID + " IS ?";
   public static final String[] TEST_TABS_CLIENT_GUID_IS_LOCAL_SELECTION_ARGS = new String[] { TEST_CLIENT_GUID };
 
+  public static final String TEST_CLIENTS_GUID_IS_LOCAL_SELECTION = BrowserContract.Clients.GUID + " IS ?";
+  public static final String[] TEST_CLIENTS_GUID_IS_LOCAL_SELECTION_ARGS = new String[] { TEST_CLIENT_GUID };
+
   protected ContentProviderClient tabsClient = null;
 
   protected ContentProviderClient getTabsClient() {
     final ContentResolver cr = getApplicationContext().getContentResolver();
     return cr.acquireContentProviderClient(BrowserContractHelpers.TABS_CONTENT_URI);
   }
 
   public TestFennecTabsRepositorySession() throws NoContentProviderException {
@@ -210,9 +217,60 @@ public class TestFennecTabsRepositorySes
 
     // No tabs are modified after this, but our client name has changed, so
     // again we get a record.
     clientsDataDelegate.setClientName("new client name", System.currentTimeMillis());
     performWait(fetchSinceRunnable(session, now, new Record[] { tabsRecord }));
 
     session.abort();
   }
+
+  // Verify that storing a tabs record writes a clients record with the correct
+  // device type to the Fennec clients provider.
+  public void testStore() throws NoContentProviderException, RemoteException {
+    // Get a valid tabsRecord to write.
+    final TabsRecord tabsRecord = insertTestTabsAndExtractTabsRecord();
+    deleteAllTestTabs(tabsClient);
+
+    final ContentResolver cr = getApplicationContext().getContentResolver();
+    final ContentProviderClient clientsClient = cr.acquireContentProviderClient(BrowserContractHelpers.CLIENTS_CONTENT_URI);
+
+    try {
+      // We can't delete only our test clients due to a Fennec CP issue with guid vs. client_guid.
+      clientsClient.delete(BrowserContractHelpers.CLIENTS_CONTENT_URI, null, null);
+
+      // This clients DB is not the Fennec DB; it's Sync's own clients DB.
+      final ClientsDatabaseAccessor db = new ClientsDatabaseAccessor(getApplicationContext());
+      try {
+        ClientRecord clientRecord = new ClientRecord(TEST_CLIENT_GUID);
+        clientRecord.name = TEST_CLIENT_NAME;
+        clientRecord.type = TEST_CLIENT_DEVICE_TYPE;
+        db.store(clientRecord);
+      } finally {
+        db.close();
+      }
+
+      final FennecTabsRepositorySession session = createAndBeginSession();
+      performWait(AndroidBrowserRepositoryTestCase.storeRunnable(session, tabsRecord));
+
+      session.abort();
+
+      // This store should write Sync's idea of the client's device_type to Fennec's clients CP.
+      final Cursor cursor = clientsClient.query(BrowserContractHelpers.CLIENTS_CONTENT_URI, null,
+          TEST_CLIENTS_GUID_IS_LOCAL_SELECTION, TEST_CLIENTS_GUID_IS_LOCAL_SELECTION_ARGS, null);
+      assertNotNull(cursor);
+
+      try {
+        assertTrue(cursor.moveToFirst());
+        assertEquals(TEST_CLIENT_GUID, cursor.getString(cursor.getColumnIndex(Clients.GUID)));
+        assertEquals(TEST_CLIENT_NAME, cursor.getString(cursor.getColumnIndex(Clients.NAME)));
+        assertEquals(TEST_CLIENT_DEVICE_TYPE, cursor.getString(cursor.getColumnIndex(Clients.DEVICE_TYPE)));
+        assertTrue(cursor.isLast());
+      } finally {
+        cursor.close();
+      }
+    } finally {
+      // We can't delete only our test client due to a Fennec CP issue with guid vs. client_guid.
+      clientsClient.delete(BrowserContractHelpers.CLIENTS_CONTENT_URI, null, null);
+      clientsClient.release();
+    }
+  }
 }