From ea61419989e0fe98cd1d9022d395c5c4b5600e72 Mon Sep 17 00:00:00 2001 From: Sriram Ramasubramanian Date: Fri, 3 Feb 2012 11:22:36 -0800 Subject: [PATCH 01/16] Bug 723899: There is no "empty" resource in ICS. [r=mfinkle] --- mobile/android/base/AwesomeBarTabs.java | 4 ++-- mobile/android/base/resources/layout/awesomebar_row.xml | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/mobile/android/base/AwesomeBarTabs.java b/mobile/android/base/AwesomeBarTabs.java index 5b86807d2ee..c7a442acd24 100644 --- a/mobile/android/base/AwesomeBarTabs.java +++ b/mobile/android/base/AwesomeBarTabs.java @@ -134,7 +134,7 @@ public class AwesomeBarTabs extends TabHost { ImageView favicon = (ImageView) childView.findViewById(R.id.favicon); if (b == null) { - favicon.setImageResource(android.R.id.empty); + favicon.setImageDrawable(null); } else { Bitmap bitmap = BitmapFactory.decodeByteArray(b, 0, b.length); favicon.setImageBitmap(bitmap); @@ -150,7 +150,7 @@ public class AwesomeBarTabs extends TabHost { ImageView favicon = (ImageView) view; if (b == null) { - favicon.setImageResource(android.R.id.empty); + favicon.setImageDrawable(null); } else { Bitmap bitmap = BitmapFactory.decodeByteArray(b, 0, b.length); favicon.setImageBitmap(bitmap); diff --git a/mobile/android/base/resources/layout/awesomebar_row.xml b/mobile/android/base/resources/layout/awesomebar_row.xml index 45b01aed03e..3f7bf7db9f6 100644 --- a/mobile/android/base/resources/layout/awesomebar_row.xml +++ b/mobile/android/base/resources/layout/awesomebar_row.xml @@ -11,7 +11,6 @@ android:layout_centerVertical="true" android:minWidth="32dip" android:minHeight="32dip" - android:src="@android:id/empty" android:scaleType="fitCenter"/> Date: Fri, 3 Feb 2012 15:05:49 -0500 Subject: [PATCH 02/16] Bug 719560 - [resize screen] Can't publish split native and xul builds under the same product on android market. r=blassey --- embedding/android/AndroidManifest.xml.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/embedding/android/AndroidManifest.xml.in b/embedding/android/AndroidManifest.xml.in index ec386218c19..dbd64a30d9b 100644 --- a/embedding/android/AndroidManifest.xml.in +++ b/embedding/android/AndroidManifest.xml.in @@ -17,7 +17,8 @@ + android:xlargeScreens="true" + android:resizeable=true /> #endif From 3bba375c887d7ac30fa13d1c0cda4901281fb765 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Fri, 3 Feb 2012 15:55:09 -0500 Subject: [PATCH 03/16] Bug 723550 - Lots of base64 decode errors in logcat [r=blassey] --- mobile/android/base/GeckoAppShell.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java index 9c8a53608f6..511b4ebb52a 100644 --- a/mobile/android/base/GeckoAppShell.java +++ b/mobile/android/base/GeckoAppShell.java @@ -1900,7 +1900,7 @@ public class GeckoAppShell */ public static byte[] decodeBase64(byte[] in) { if (Build.VERSION.SDK_INT >=Build.VERSION_CODES.FROYO) - return Base64.decode(in, GUID_ENCODE_FLAGS); + return Base64.decode(in, Base64.DEFAULT); int iOff = 0; int iLen = in.length; if (iLen%4 != 0) throw new IllegalArgumentException ("Length of Base64 encoded input string is not a multiple of 4."); From d8aa1a479b84b4d326b9cc2539015f4cb3b3303f Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 04/16] Bug 718703 - Part 0: Makefile fix for preprocessing multiple files. r=ted --- mobile/android/base/Makefile.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in index ba6e706926b..116c6cb0c11 100644 --- a/mobile/android/base/Makefile.in +++ b/mobile/android/base/Makefile.in @@ -597,9 +597,10 @@ classes.dex: $(FENNEC_JAVA_FILES) $(FENNEC_PP_JAVA_FILES) $(SYNC_JAVA_FILES) $(S PP_RES_XML=$(SYNC_PP_RES_XML) -$(PP_RES_XML): $(subst res/,$(srcdir)/resources/, $(PP_RES_XML).in) +# This is kinda awful; if any of the source files change, we remake them all. +$(PP_RES_XML): $(patsubst res/%,$(srcdir)/resources/%.in,$(PP_RES_XML)) $(PYTHON) $(topsrcdir)/config/Preprocessor.py \ - $(AUTOMATION_PPARGS) $(DEFINES) $(ACDEFINES) $< > $@ + $(AUTOMATION_PPARGS) $(DEFINES) $(ACDEFINES) $(subst res,$(srcdir)/resources,$@).in > $@ # AndroidManifest.xml includes these files, so they need to be marked as dependencies. SYNC_MANIFEST_FRAGMENTS = $(wildcard $(topsrcdir)/mobile/android/sync/manifests/*.in) From d043fc698a6907d16349d61af794dc8eb166ca3e Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 05/16] Bug 720934 - Part 1: Collected Android Sync 0.4 code drop (Bug 721305, Bug 709348, Bug 719693, Bug 709660, Bug 718703). a=mobile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Includes: 3ad57f4 Bug 718703 - Pair a Device crashes. r=rnewman 3c530d1 Bug 709660 - Fixed blurry icons used in SyncAdapter. r=rnewman bde287a Bug 719693 - Compatibility for JDK < 6. 91c5e23 Bug 719693 - Effect review comments on 6ffb7d72f. 6ffb7d7 Bug 719693 - Handle date-format HTTP Retry-After headers. 6df289e Bug 709348 - Trace logging and fixed logging of visit equality. 71580df Bug 709348 - Include Fennec visits and counts in HistoryRecord androidID equivalence. 8f6a106 Bug 709348 - Equality work. f40bf4a Bug 709348 - Part 5: Rework handling of certain cases of incoming deletion, including history visits. 0dd004d Bug 709348 - Part 4: Reworking equality tests between records. 2210717 Bug 709348 - Part 3: Better commenting for ignored bookmark records. c05236b Bug 709348 - Part 2: Introduce trace log method. a34f4aa Bug 709348 - Part 1: Introduce copyWithIDs to clone records. 796b15f Bug 721305 - sync.link.advancedsetup.label should use …, not .... adf7586 Bump AndroidSync User-Agent version. --- .../base/locales/en-US/sync_strings.dtd | 2 +- mobile/android/base/sync/CryptoRecord.java | 11 ++ mobile/android/base/sync/GlobalSession.java | 22 +-- .../base/sync/SynchronizerConfigurations.java | 9 - .../android/base/sync/net/SyncResponse.java | 47 +++++- .../base/sync/net/SyncStorageRequest.java | 2 +- ...roidBrowserBookmarksRepositorySession.java | 1 - .../AndroidBrowserRepositorySession.java | 33 +++- .../repositories/domain/BookmarkRecord.java | 66 +++++++- .../repositories/domain/HistoryRecord.java | 82 +++++++-- .../repositories/domain/PasswordRecord.java | 51 +++++- .../base/sync/repositories/domain/Record.java | 155 +++++++++++++++++- 12 files changed, 406 insertions(+), 75 deletions(-) diff --git a/mobile/android/base/locales/en-US/sync_strings.dtd b/mobile/android/base/locales/en-US/sync_strings.dtd index 0f0e592fd42..b40c4a20e68 100644 --- a/mobile/android/base/locales/en-US/sync_strings.dtd +++ b/mobile/android/base/locales/en-US/sync_strings.dtd @@ -17,7 +17,7 @@ - + diff --git a/mobile/android/base/sync/CryptoRecord.java b/mobile/android/base/sync/CryptoRecord.java index 2a77b90c7ab..912fb77417f 100644 --- a/mobile/android/base/sync/CryptoRecord.java +++ b/mobile/android/base/sync/CryptoRecord.java @@ -130,6 +130,17 @@ public class CryptoRecord extends Record { super(source.guid, source.collection, source.lastModified, source.deleted); } + @Override + public Record copyWithIDs(String guid, long androidID) { + CryptoRecord out = new CryptoRecord(this); + out.guid = guid; + out.androidID = androidID; + out.sortIndex = this.sortIndex; + out.payload = (this.payload == null) ? null : new ExtendedJSONObject(this.payload.object); + out.keyBundle = this.keyBundle; // TODO: copy me? + return out; + } + /** * Take a whole record as JSON -- i.e., something like * diff --git a/mobile/android/base/sync/GlobalSession.java b/mobile/android/base/sync/GlobalSession.java index 2f8a69c686f..c9ea8028aaf 100644 --- a/mobile/android/base/sync/GlobalSession.java +++ b/mobile/android/base/sync/GlobalSession.java @@ -185,8 +185,7 @@ public class GlobalSession implements CredentialsSource, PrefsSource { config.syncKeyBundle = syncKeyBundle; // clusterURL and syncID are set through `persisted`, or fetched from the server. - // TODO: populate saved configurations. We'll amend these after processing meta/global. - this.synchronizerConfigurations = new SynchronizerConfigurations(persisted); + assert(null == persisted); prepareStages(); } @@ -696,23 +695,4 @@ public class GlobalSession implements CredentialsSource, PrefsSource { } return this.config.metaGlobal.engines.get(engineName) != null; } - - /** - * Return enough information to be able to reconstruct a Synchronizer. - * - * @param engineName - * @return - */ - public SynchronizerConfiguration configForEngine(String engineName) { - // TODO: we need an altogether better way of handling empty configs. - SynchronizerConfiguration stored = this.getSynchronizerConfigurations().forEngine(engineName); - if (stored == null) { - return new SynchronizerConfiguration(engineName, new RepositorySessionBundle(0), new RepositorySessionBundle(0)); - } - return stored; - } - private SynchronizerConfigurations synchronizerConfigurations; - private SynchronizerConfigurations getSynchronizerConfigurations() { - return this.synchronizerConfigurations; - } } diff --git a/mobile/android/base/sync/SynchronizerConfigurations.java b/mobile/android/base/sync/SynchronizerConfigurations.java index aff8595b89b..d571fd1e5cd 100644 --- a/mobile/android/base/sync/SynchronizerConfigurations.java +++ b/mobile/android/base/sync/SynchronizerConfigurations.java @@ -91,15 +91,6 @@ public class SynchronizerConfigurations { engines = new HashMap(); } - public void fillBundle(Bundle bundle) { - Bundle contents = new Bundle(); - for (Entry entry : engines.entrySet()) { - contents.putStringArray(entry.getKey(), entry.getValue().toStringValues()); - } - contents.putInt("version", CONFIGURATION_VERSION); - bundle.putBundle("engines", contents); - } - public SynchronizerConfiguration forEngine(String engineName) { return engines.get(engineName); } diff --git a/mobile/android/base/sync/net/SyncResponse.java b/mobile/android/base/sync/net/SyncResponse.java index 0036680272e..5a6a8ea2ffa 100644 --- a/mobile/android/base/sync/net/SyncResponse.java +++ b/mobile/android/base/sync/net/SyncResponse.java @@ -20,6 +20,7 @@ * * Contributor(s): * Richard Newman + * Nick Alexander * * Alternatively, the contents of this file may be used under the terms of * either the GNU General Public License Version 2 or later (the "GPL"), or @@ -47,11 +48,16 @@ import org.mozilla.gecko.sync.ExtendedJSONObject; import org.mozilla.gecko.sync.NonObjectJSONException; import org.mozilla.gecko.sync.Utils; +import android.util.Log; import ch.boye.httpclientandroidlib.Header; import ch.boye.httpclientandroidlib.HttpEntity; import ch.boye.httpclientandroidlib.HttpResponse; +import ch.boye.httpclientandroidlib.impl.cookie.DateParseException; +import ch.boye.httpclientandroidlib.impl.cookie.DateUtils; public class SyncResponse { + private static final String HEADER_RETRY_AFTER = "retry-after"; + private static final String LOG_TAG = "SyncResponse"; protected HttpResponse response; @@ -123,10 +129,20 @@ public class SyncResponse { return this.response.containsHeader(h); } - private int getIntegerHeader(String h) { + private static boolean missingHeader(String value) { + return value == null || + value.trim().length() == 0; + } + + private int getIntegerHeader(String h) throws NumberFormatException { if (this.hasHeader(h)) { Header header = this.response.getFirstHeader(h); - return Integer.parseInt(header.getValue(), 10); + String value = header.getValue(); + if (missingHeader(value)) { + Log.w(LOG_TAG, h + " header present but empty."); + return -1; + } + return Integer.parseInt(value, 10); } return -1; } @@ -135,7 +151,31 @@ public class SyncResponse { * @return A number of seconds, or -1 if the header was not present. */ public int retryAfter() throws NumberFormatException { - return this.getIntegerHeader("retry-after"); + if (!this.hasHeader(HEADER_RETRY_AFTER)) { + return -1; + } + + Header header = this.response.getFirstHeader(HEADER_RETRY_AFTER); + String retryAfter = header.getValue(); + if (missingHeader(retryAfter)) { + Log.w(LOG_TAG, "Retry-After header present but empty."); + return -1; + } + + try { + return Integer.parseInt(retryAfter, 10); + } catch (NumberFormatException e) { + // Fall through to try date format. + } + + try { + final long then = DateUtils.parseDate(retryAfter).getTime(); + final long now = System.currentTimeMillis(); + return (int)((then - now) / 1000); // Convert milliseconds to seconds. + } catch (DateParseException e) { + Log.w(LOG_TAG, "Retry-After header neither integer nor date: " + retryAfter); + return -1; + } } public int weaveBackoff() throws NumberFormatException { @@ -174,5 +214,4 @@ public class SyncResponse { } return null; } - } \ No newline at end of file diff --git a/mobile/android/base/sync/net/SyncStorageRequest.java b/mobile/android/base/sync/net/SyncStorageRequest.java index 991dbe9c174..7c9f626bcbd 100644 --- a/mobile/android/base/sync/net/SyncStorageRequest.java +++ b/mobile/android/base/sync/net/SyncStorageRequest.java @@ -173,7 +173,7 @@ public class SyncStorageRequest implements Resource { } } - public static String USER_AGENT = "Firefox AndroidSync 0.3"; + public static String USER_AGENT = "Firefox AndroidSync 0.4"; protected SyncResourceDelegate resourceDelegate; public SyncStorageRequestDelegate delegate; protected BaseResource resource; diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java index ec46b9ab9e1..010966bffe1 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java @@ -56,7 +56,6 @@ import android.content.Context; import android.database.Cursor; import android.util.Log; - public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepositorySession { // TODO: synchronization for these. diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java index b845b7d89f7..83de5ff6169 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java @@ -41,6 +41,7 @@ package org.mozilla.gecko.sync.repositories.android; import java.util.ArrayList; import java.util.HashMap; +import org.mozilla.gecko.sync.Utils; import org.mozilla.gecko.sync.repositories.InactiveSessionException; import org.mozilla.gecko.sync.repositories.InvalidRequestException; import org.mozilla.gecko.sync.repositories.InvalidSessionTransitionException; @@ -342,6 +343,15 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession this.fetchSince(0, delegate); } + private void trace(String m) { + if (Utils.ENABLE_TRACE_LOGGING) { + if (Utils.LOG_TO_STDOUT) { + System.out.println(LOG_TAG + "::TRACE " + m); + } + Log.d(LOG_TAG, m); + } + } + @Override public void store(final Record record) throws NoStoreDelegateException { if (delegate == null) { @@ -363,9 +373,11 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession return; } - // Check that the record is a valid type - // TODO Currently for bookmarks we only take care of folders - // and bookmarks, all other types are ignored and thrown away + // Check that the record is a valid type. + // Fennec only supports bookmarks and folders. All other types of records, + // including livemarks and queries, are simply ignored. + // See Bug 708149. This might be resolved by Fennec changing its database + // schema, or by Sync storing non-applied records in its own private database. if (!checkRecordType(record)) { Log.d(LOG_TAG, "Ignoring record " + record.guid + " due to unknown record type."); @@ -386,11 +398,18 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession record.androidID = insert(record); } else if (existingRecord != null) { + // If the incoming record is marked deleted and + // our existing record has a newer timestamp, then + // discard the incoming record. + if (record.deleted && existingRecord.lastModified > record.lastModified) { + delegate.onRecordStoreSucceeded(record); + return; + } + // Now's a great time to do expensive additions. + existingRecord = transformRecord(existingRecord); dbHelper.delete(existingRecord); - // Or clause: We won't store a remotely deleted record ever, but if it is marked deleted - // and our existing record has a newer timestamp, we will restore the existing record - if (!record.deleted || (record.deleted && existingRecord.lastModified > record.lastModified)) { - // Record exists already, need to figure out what to store + if (!record.deleted) { + // Record exists already, need to figure out what to store. Record store = reconcileRecords(existingRecord, record); record.androidID = insert(store); } diff --git a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java index c215ae00202..bedbe6cc527 100644 --- a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java +++ b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java @@ -95,6 +95,51 @@ public class BookmarkRecord extends Record { parentID + "/" + androidParentID + "/" + parentName + ">"; } + // Oh God, this is terribly thread-unsafe. These record objects should be immutable. + @SuppressWarnings("unchecked") + protected JSONArray copyChildren() { + if (this.children == null) { + return null; + } + JSONArray children = new JSONArray(); + children.addAll(this.children); + return children; + } + + @SuppressWarnings("unchecked") + protected JSONArray copyTags() { + if (this.tags == null) { + return null; + } + JSONArray tags = new JSONArray(); + tags.addAll(this.tags); + return tags; + } + + @Override + public Record copyWithIDs(String guid, long androidID) { + BookmarkRecord out = new BookmarkRecord(guid, this.collection, this.lastModified, this.deleted); + out.androidID = androidID; + out.sortIndex = this.sortIndex; + + // Copy BookmarkRecord fields. + out.title = this.title; + out.bookmarkURI = this.bookmarkURI; + out.description = this.description; + out.keyword = this.keyword; + out.parentID = this.parentID; + out.parentName = this.parentName; + out.androidParentID = this.androidParentID; + out.type = this.type; + out.pos = this.pos; + out.androidPosition = this.androidPosition; + + out.children = this.copyChildren(); + out.tags = this.copyTags(); + + return out; + } + @Override public void initFromPayload(CryptoRecord payload) { ExtendedJSONObject p = payload.payload; @@ -185,15 +230,14 @@ public class BookmarkRecord extends Record { } @Override - public boolean equals(Object o) { - trace("Calling BookmarkRecord.equals."); - if (!(o instanceof BookmarkRecord)) { + public boolean equalPayloads(Object o) { + trace("Calling BookmarkRecord.equalPayloads."); + if (o == null || !(o instanceof BookmarkRecord)) { return false; } BookmarkRecord other = (BookmarkRecord) o; - - if (!super.equals(other)) { + if (!super.equalPayloads(other)) { return false; } @@ -236,8 +280,15 @@ public class BookmarkRecord extends Record { && RepoUtils.stringsEqual(this.keyword, other.keyword) && jsonArrayStringsEqual(this.tags, other.tags); } - - // Converts to JSONArrays to strings and checks if they are the same. + + // TODO: two records can be congruent if their child lists are different. + @Override + public boolean congruentWith(Object o) { + return this.equalPayloads(o) && + super.congruentWith(o); + } + + // Converts two JSONArrays to strings and checks if they are the same. // This is only useful for stuff like tags where we aren't actually // touching the data there (and therefore ordering won't change) private boolean jsonArrayStringsEqual(JSONArray a, JSONArray b) { @@ -247,7 +298,6 @@ public class BookmarkRecord extends Record { if (a != null && b == null) return false; return RepoUtils.stringsEqual(a.toJSONString(), b.toJSONString()); } - } diff --git a/mobile/android/base/sync/repositories/domain/HistoryRecord.java b/mobile/android/base/sync/repositories/domain/HistoryRecord.java index 35c0327e1e6..4cd32fcc86b 100644 --- a/mobile/android/base/sync/repositories/domain/HistoryRecord.java +++ b/mobile/android/base/sync/repositories/domain/HistoryRecord.java @@ -83,6 +83,32 @@ public class HistoryRecord extends Record { public long fennecDateVisited; public long fennecVisitCount; + @SuppressWarnings("unchecked") + private JSONArray copyVisits() { + if (this.visits == null) { + return null; + } + JSONArray out = new JSONArray(); + out.addAll(this.visits); + return out; + } + + @Override + public Record copyWithIDs(String guid, long androidID) { + HistoryRecord out = new HistoryRecord(guid, this.collection, this.lastModified, this.deleted); + out.androidID = androidID; + out.sortIndex = this.sortIndex; + + // Copy HistoryRecord fields. + out.title = this.title; + out.histURI = this.histURI; + out.fennecDateVisited = this.fennecDateVisited; + out.fennecVisitCount = this.fennecVisitCount; + out.visits = this.copyVisits(); + + return out; + } + @Override public void initFromPayload(CryptoRecord payload) { ExtendedJSONObject p = payload.payload; @@ -115,32 +141,62 @@ public class HistoryRecord extends Record { return rec; } - public boolean equalsExceptVisits(Object o) { - if (!(o instanceof HistoryRecord)) { + + /** + * We consider two history records to be congruent if they represent the + * same history record regardless of visits. + */ + @Override + public boolean congruentWith(Object o) { + if (o == null || !(o instanceof HistoryRecord)) { return false; } HistoryRecord other = (HistoryRecord) o; - return super.equals(other) && - RepoUtils.stringsEqual(this.title, other.title) && + if (!super.congruentWith(other)) { + return false; + } + return RepoUtils.stringsEqual(this.title, other.title) && RepoUtils.stringsEqual(this.histURI, other.histURI); } - public boolean equalsIncludingVisits(Object o) { + @Override + public boolean equalPayloads(Object o) { + if (o == null || !(o instanceof HistoryRecord)) { + Log.d(LOG_TAG, "Not a HistoryRecord: " + o); + return false; + } HistoryRecord other = (HistoryRecord) o; - return equalsExceptVisits(other) && this.checkVisitsEquals(other); + if (!super.equalPayloads(other)) { + Log.d(LOG_TAG, "super.equalPayloads returned false."); + return false; + } + return RepoUtils.stringsEqual(this.title, other.title) && + RepoUtils.stringsEqual(this.histURI, other.histURI) && + checkVisitsEquals(other); } @Override - /** - * We consider two history records to be equal if they represent the - * same history record regardless of visits. - */ - public boolean equals(Object o) { - return equalsExceptVisits(o); + public boolean equalAndroidIDs(Record other) { + return super.equalAndroidIDs(other) && + this.equalFennecVisits(other); + } + + private boolean equalFennecVisits(Record other) { + if (!(other instanceof HistoryRecord)) { + return false; + } + HistoryRecord h = (HistoryRecord) other; + return this.fennecDateVisited == h.fennecDateVisited && + this.fennecVisitCount == h.fennecVisitCount; } private boolean checkVisitsEquals(HistoryRecord other) { - + Log.d(LOG_TAG, "Checking visits."); + if (Utils.ENABLE_TRACE_LOGGING) { + Log.d(LOG_TAG, ">> Mine: " + ((this.visits == null) ? "null" : this.visits.toJSONString())); + Log.d(LOG_TAG, ">> Theirs: " + ((other.visits == null) ? "null" : other.visits.toJSONString())); + } + // Handle nulls. if (this.visits == other.visits) { return true; diff --git a/mobile/android/base/sync/repositories/domain/PasswordRecord.java b/mobile/android/base/sync/repositories/domain/PasswordRecord.java index e4c6d49ed57..aa978f572df 100644 --- a/mobile/android/base/sync/repositories/domain/PasswordRecord.java +++ b/mobile/android/base/sync/repositories/domain/PasswordRecord.java @@ -75,6 +75,28 @@ public class PasswordRecord extends Record { public long timeLastUsed; public long timesUsed; + + @Override + public Record copyWithIDs(String guid, long androidID) { + PasswordRecord out = new PasswordRecord(guid, this.collection, this.lastModified, this.deleted); + out.androidID = androidID; + out.sortIndex = this.sortIndex; + + // Copy HistoryRecord fields. + out.hostname = this.hostname; + out.formSubmitURL = this.formSubmitURL; + out.httpRealm = this.httpRealm; + out.username = this.username; + out.password = this.password; + out.usernameField = this.usernameField; + out.passwordField = this.passwordField; + out.encType = this.encType; + out.timeLastUsed = this.timeLastUsed; + out.timesUsed = this.timesUsed; + + return out; + } + @Override public void initFromPayload(CryptoRecord payload) { // TODO Auto-generated method stub @@ -88,10 +110,33 @@ public class PasswordRecord extends Record { } @Override - public boolean equals(Object o) { - if (!o.getClass().equals(PasswordRecord.class)) return false; + public boolean congruentWith(Object o) { + if (o == null || !(o instanceof PasswordRecord)) { + return false; + } PasswordRecord other = (PasswordRecord) o; - if (!super.equals(other)) return false; + if (!super.congruentWith(other)) { + return false; + } + return RepoUtils.stringsEqual(this.hostname, other.hostname) + && RepoUtils.stringsEqual(this.formSubmitURL, other.formSubmitURL) + && RepoUtils.stringsEqual(this.httpRealm, other.httpRealm) + && RepoUtils.stringsEqual(this.username, other.username) + && RepoUtils.stringsEqual(this.password, other.password) + && RepoUtils.stringsEqual(this.usernameField, other.usernameField) + && RepoUtils.stringsEqual(this.passwordField, other.passwordField) + && RepoUtils.stringsEqual(this.encType, other.encType); + } + + @Override + public boolean equalPayloads(Object o) { + if (o == null || !(o instanceof PasswordRecord)) { + return false; + } + PasswordRecord other = (PasswordRecord) o; + if (!super.equalPayloads(other)) { + return false; + } return RepoUtils.stringsEqual(this.hostname, other.hostname) && RepoUtils.stringsEqual(this.formSubmitURL, other.formSubmitURL) && RepoUtils.stringsEqual(this.httpRealm, other.httpRealm) diff --git a/mobile/android/base/sync/repositories/domain/Record.java b/mobile/android/base/sync/repositories/domain/Record.java index c1b82acebe0..0fc050b0602 100644 --- a/mobile/android/base/sync/repositories/domain/Record.java +++ b/mobile/android/base/sync/repositories/domain/Record.java @@ -43,8 +43,62 @@ import java.io.UnsupportedEncodingException; import org.mozilla.gecko.sync.CryptoRecord; import org.mozilla.gecko.sync.ExtendedJSONObject; +/** + * Record is the abstract base class for all entries that Sync processes: + * bookmarks, passwords, history, and such. + * + * A Record can be initialized from or serialized to a CryptoRecord for + * submission to an encrypted store. + * + * Records should be considered to be conventionally immutable: modifications + * should be completed before the new record object escapes its constructing + * scope. Note that this is a critically important part of equality. As Rich + * Hickey notes: + * + * … the only things you can really compare for equality are immutable things, + * because if you compare two things for equality that are mutable, and ever + * say true, and they're ever not the same thing, you are wrong. Or you will + * become wrong at some point in the future. + * + * Records have a layered definition of equality. Two records can be said to be + * "equal" if: + * + * * They have the same GUID and collection. Two crypto/keys records are in some + * way "the same". + * This is `equalIdentifiers`. + * + * * Their most significant fields are the same. That is to say, they share a + * GUID, a collection, deletion, and domain-specific fields. Two copies of + * crypto/keys, neither deleted, with the same encrypted data but different + * modified times and sortIndex are in a stronger way "the same". + * This is `equalPayloads`. + * + * * Their most significant fields are the same, and their local fields (e.g., + * the androidID to which we have decided that this record maps) are congruent. + * A record with the same androidID, or one whose androidID has not been set, + * can be considered "the same". + * This concept can be extended by Record subclasses. The key point is that + * reconciling should be applied to the contents of these records. For example, + * two history records with the same URI and GUID, but different visit arrays, + * can be said to be congruent. + * This is `congruentWith`. + * + * * They are strictly identical. Every field that is persisted, including + * lastModified and androidID, is equal. + * This is `equals`. + * + * Different parts of the codebase have use for different layers of this + * comparison hierarchy. For instance, lastModified times change every time a + * record is stored; a store followed by a retrieval will return a Record that + * shares its most significant fields with the input, but has a later + * lastModified time and might not yet have values set for others. Reconciling + * will thus ignore the modification time of a record. + * + * @author rnewman + * + */ public abstract class Record { - // TODO: consider immutability, effective immutability, and thread-safety. + public String guid; public String collection; public long lastModified; @@ -58,16 +112,22 @@ public abstract class Record { this.lastModified = lastModified; this.deleted = deleted; this.sortIndex = 0; + this.androidID = -1; } - @Override - public boolean equals(Object o) { - if (o == null) { + /** + * Return true iff the input is a Record and has the same + * collection and guid as this object. + * + * @param o + * @return + */ + public boolean equalIdentifiers(Object o) { + if (o == null || !(o instanceof Record)) { return false; } Record other = (Record) o; - if (this.guid == null) { if (other.guid != null) { return false; @@ -77,7 +137,6 @@ public abstract class Record { return false; } } - if (this.collection == null) { if (other.collection != null) { return false; @@ -87,13 +146,84 @@ public abstract class Record { return false; } } + return true; + } - if (this.deleted != other.deleted) { + /** + * Return true iff the input is a Record which is substantially the + * same as this object. + * + * @param o + * @return + */ + public boolean equalPayloads(Object o) { + if (!this.equalIdentifiers(o)) { + return false; + } + Record other = (Record) o; + return this.deleted == other.deleted; + } + + /** + * Return true iff the input is a Record which is substantially the + * same as this object, considering the ability and desire two + * reconcile the two objects if possible. + * + * @param o + * @return + */ + public boolean congruentWith(Object o) { + if (!this.equalIdentifiers(o)) { + return false; + } + Record other = (Record) o; + return congruentAndroidIDs(other) && + (this.deleted == other.deleted); + } + + public boolean congruentAndroidIDs(Record other) { + // We treat -1 as "unset", and treat this as + // congruent with any other value. + if (this.androidID != -1 && + other.androidID != -1 && + this.androidID != other.androidID) { return false; } return true; } + /** + * Return true iff the input is both equal in terms of payload, + * and also shares transient values such as timestamps. + */ + @Override + public boolean equals(Object o) { + if (o == null || !(o instanceof Record)) { + return false; + } + + Record other = (Record) o; + return equalTimestamps(other) && + equalSortIndices(other) && + equalAndroidIDs(other) && + equalPayloads(o); + } + + public boolean equalAndroidIDs(Record other) { + return this.androidID == other.androidID; + } + + public boolean equalSortIndices(Record other) { + return this.sortIndex == other.sortIndex; + } + + public boolean equalTimestamps(Object o) { + if (o == null || !(o instanceof Record)) { + return false; + } + return ((Record) o).lastModified == this.lastModified; + } + public abstract void initFromPayload(CryptoRecord payload); public abstract CryptoRecord getPayload(); @@ -122,4 +252,15 @@ public abstract class Record { throw new IllegalStateException(detailMessage); } } + + /** + * Return an identical copy of this record with the provided two values. + * + * Oh for persistent data structures. + * + * @param guid + * @param androidID + * @return + */ + public abstract Record copyWithIDs(String guid, long androidID); } From 74192e01f55733fe85a8a9de85226ce3827e896b Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 06/16] Bug 722434 - check for failure after JSON parsing. r=rnewman --- .../android/base/sync/cryptographer/SyncCryptographer.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mobile/android/base/sync/cryptographer/SyncCryptographer.java b/mobile/android/base/sync/cryptographer/SyncCryptographer.java index 93838e658b9..81ea8a9d536 100644 --- a/mobile/android/base/sync/cryptographer/SyncCryptographer.java +++ b/mobile/android/base/sync/cryptographer/SyncCryptographer.java @@ -57,6 +57,7 @@ import org.mozilla.gecko.sync.crypto.CryptoInfo; import org.mozilla.gecko.sync.crypto.Cryptographer; import org.mozilla.gecko.sync.crypto.KeyBundle; import org.mozilla.gecko.sync.cryptographer.CryptoStatusBundle.CryptoStatus; +import java.security.GeneralSecurityException; /* * This class acts as a wrapper for the Cryptographer class. @@ -192,6 +193,10 @@ public class SyncCryptographer { e.printStackTrace(); } + if (json == null) { + throw new CryptoException(new GeneralSecurityException("Could not decrypt JSON payload")); + } + // Verify that this is indeed the crypto/keys bundle and that // decryption worked. String id = (String) json.get(KEY_ID); From d7526bc2e2cad75c94f02f9cdb6fab54659e2328 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 07/16] Bug 722426 - Remove WRITE_SECURE_SETTINGS from the Android manifest, since this permission is only granted to system/firmware applications. r=rnewman --- .../sync/manifests/SyncAndroidManifest_permissions.xml.in | 1 - 1 file changed, 1 deletion(-) diff --git a/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in b/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in index e8515d88a3c..a5dc0e353dd 100644 --- a/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in +++ b/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in @@ -5,6 +5,5 @@ - From 689aa6206c4cfdccbdbf425781bb070fce4582df Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 08/16] Bug 722896. r=dchan --- .../sync/manifests/SyncAndroidManifest_services.xml.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/android/sync/manifests/SyncAndroidManifest_services.xml.in b/mobile/android/sync/manifests/SyncAndroidManifest_services.xml.in index 97deec0dc2a..f32e687dc77 100644 --- a/mobile/android/sync/manifests/SyncAndroidManifest_services.xml.in +++ b/mobile/android/sync/manifests/SyncAndroidManifest_services.xml.in @@ -1,5 +1,5 @@ @@ -10,7 +10,7 @@ android:resource="@xml/sync_authenticator" /> From 745e70c9b4baa4dac85201a5a83b11df34197416 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 09/16] Bug 722430 - Propagate NoSuchAlgorithmException and InvalidKeyException; wrap in CryptoException higher up the call stack. r=rnewman --- .../android/base/sync/crypto/CryptoInfo.java | 130 ++++------- .../base/sync/crypto/Cryptographer.java | 24 +- mobile/android/base/sync/crypto/HKDF.java | 205 +++++++----------- .../android/base/sync/crypto/KeyBundle.java | 14 +- .../sync/cryptographer/SyncCryptographer.java | 2 +- .../android/base/sync/jpake/JPakeClient.java | 10 + .../android/base/sync/jpake/JPakeCrypto.java | 7 +- .../android/base/sync/net/BaseResource.java | 2 +- 8 files changed, 176 insertions(+), 218 deletions(-) diff --git a/mobile/android/base/sync/crypto/CryptoInfo.java b/mobile/android/base/sync/crypto/CryptoInfo.java index 2b0056b4cfe..eb38568dcb5 100644 --- a/mobile/android/base/sync/crypto/CryptoInfo.java +++ b/mobile/android/base/sync/crypto/CryptoInfo.java @@ -1,102 +1,66 @@ -/* ***** BEGIN LICENSE BLOCK ***** - * Version: MPL 1.1/GPL 2.0/LGPL 2.1 - * - * The contents of this file are subject to the Mozilla Public License Version - * 1.1 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * http://www.mozilla.org/MPL/ - * - * Software distributed under the License is distributed on an "AS IS" basis, - * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License - * for the specific language governing rights and limitations under the - * License. - * - * The Original Code is Android Sync Client. - * - * The Initial Developer of the Original Code is - * the Mozilla Foundation. - * Portions created by the Initial Developer are Copyright (C) 2011 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Jason Voll - * - * Alternatively, the contents of this file may be used under the terms of - * either the GNU General Public License Version 2 or later (the "GPL"), or - * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), - * in which case the provisions of the GPL or the LGPL are applicable instead - * of those above. If you wish to allow use of your version of this file only - * under the terms of either the GPL or the LGPL, and not to allow others to - * use your version of this file under the terms of the MPL, indicate your - * decision by deleting the provisions above and replace them with the notice - * and other provisions required by the GPL or the LGPL. If you do not delete - * the provisions above, a recipient may use your version of this file under - * the terms of any one of the MPL, the GPL or the LGPL. - * - * ***** END LICENSE BLOCK ***** */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ package org.mozilla.gecko.sync.crypto; - - /* * All info in these objects should be decoded (i.e. not BaseXX encoded). */ public class CryptoInfo { - private byte[] message; - private byte[] iv; - private byte[] hmac; - private KeyBundle keys; + private byte[] message; + private byte[] iv; + private byte[] hmac; + private KeyBundle keys; - /* - * Constructor typically used when encrypting - */ - public CryptoInfo(byte[] message, KeyBundle keys) { - this.setMessage(message); - this.setKeys(keys); - } + /* + * Constructor typically used when encrypting. + */ + public CryptoInfo(byte[] message, KeyBundle keys) { + this.setMessage(message); + this.setKeys(keys); + } - /* - * Constructor typically used when decrypting - */ - public CryptoInfo(byte[] message, byte[] iv, byte[] hmac, KeyBundle keys) { - this.setMessage(message); - this.setIV(iv); - this.setHMAC(hmac); - this.setKeys(keys); - } + /* + * Constructor typically used when decrypting. + */ + public CryptoInfo(byte[] message, byte[] iv, byte[] hmac, KeyBundle keys) { + this.setMessage(message); + this.setIV(iv); + this.setHMAC(hmac); + this.setKeys(keys); + } - public byte[] getMessage() { - return message; - } + public byte[] getMessage() { + return message; + } - public void setMessage(byte[] message) { - this.message = message; - } + public void setMessage(byte[] message) { + this.message = message; + } - public byte[] getIV() { - return iv; - } + public byte[] getIV() { + return iv; + } - public void setIV(byte[] iv) { - this.iv = iv; - } + public void setIV(byte[] iv) { + this.iv = iv; + } - public byte[] getHMAC() { - return hmac; - } + public byte[] getHMAC() { + return hmac; + } - public void setHMAC(byte[] hmac) { - this.hmac = hmac; - } + public void setHMAC(byte[] hmac) { + this.hmac = hmac; + } - public KeyBundle getKeys() { - return keys; - } - - public void setKeys(KeyBundle keys) { - this.keys = keys; - } + public KeyBundle getKeys() { + return keys; + } + public void setKeys(KeyBundle keys) { + this.keys = keys; + } } diff --git a/mobile/android/base/sync/crypto/Cryptographer.java b/mobile/android/base/sync/crypto/Cryptographer.java index 48e326eafdf..1594a8abfc1 100644 --- a/mobile/android/base/sync/crypto/Cryptographer.java +++ b/mobile/android/base/sync/crypto/Cryptographer.java @@ -56,6 +56,7 @@ import javax.crypto.spec.SecretKeySpec; import org.mozilla.apache.commons.codec.binary.Base32; import org.mozilla.apache.commons.codec.binary.Base64; import org.mozilla.gecko.sync.Utils; +import java.security.InvalidKeyException; /* * Implements the basic required cryptography options. @@ -82,7 +83,6 @@ public class Cryptographer { cipher.init(Cipher.ENCRYPT_MODE, spec, new IvParameterSpec(info.getIV())); } } catch (GeneralSecurityException ex) { - ex.printStackTrace(); throw new CryptoException(ex); } @@ -94,7 +94,13 @@ public class Cryptographer { info.setIV(cipher.getIV()); // Generate HMAC. - info.setHMAC(generateHMAC(info)); + try { + info.setHMAC(generateHMAC(info)); + } catch (NoSuchAlgorithmException e) { + throw new CryptoException(e); + } catch (InvalidKeyException e) { + throw new CryptoException(e); + } return info; @@ -112,8 +118,14 @@ public class Cryptographer { public static byte[] decrypt(CryptoInfo info) throws CryptoException { // Check HMAC. - if (!verifyHMAC(info)) { - throw new HMACVerificationException(); + try { + if (!verifyHMAC(info)) { + throw new HMACVerificationException(); + } + } catch (NoSuchAlgorithmException e) { + throw new CryptoException(e); + } catch (InvalidKeyException e) { + throw new CryptoException(e); } Cipher cipher = getCipher(); @@ -190,7 +202,7 @@ public class Cryptographer { /* * Helper to verify HMAC Input: CryptoInfo Output: true if HMAC is correct */ - private static boolean verifyHMAC(CryptoInfo bundle) { + private static boolean verifyHMAC(CryptoInfo bundle) throws NoSuchAlgorithmException, InvalidKeyException { byte[] generatedHMAC = generateHMAC(bundle); byte[] expectedHMAC = bundle.getHMAC(); boolean eq = Arrays.equals(generatedHMAC, expectedHMAC); @@ -206,7 +218,7 @@ public class Cryptographer { * Helper to generate HMAC Input: CryptoInfo Output: a generated HMAC for * given cipher text */ - private static byte[] generateHMAC(CryptoInfo bundle) { + private static byte[] generateHMAC(CryptoInfo bundle) throws NoSuchAlgorithmException, InvalidKeyException { Mac hmacHasher = HKDF.makeHMACHasher(bundle.getKeys().getHMACKey()); return hmacHasher.doFinal(Base64.encodeBase64(bundle.getMessage())); } diff --git a/mobile/android/base/sync/crypto/HKDF.java b/mobile/android/base/sync/crypto/HKDF.java index d289a1a63ad..f68cfb6eeda 100644 --- a/mobile/android/base/sync/crypto/HKDF.java +++ b/mobile/android/base/sync/crypto/HKDF.java @@ -1,39 +1,6 @@ -/* ***** BEGIN LICENSE BLOCK ***** - * Version: MPL 1.1/GPL 2.0/LGPL 2.1 - * - * The contents of this file are subject to the Mozilla Public License Version - * 1.1 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * http://www.mozilla.org/MPL/ - * - * Software distributed under the License is distributed on an "AS IS" basis, - * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License - * for the specific language governing rights and limitations under the - * License. - * - * The Original Code is Android Sync Client. - * - * The Initial Developer of the Original Code is - * the Mozilla Foundation. - * Portions created by the Initial Developer are Copyright (C) 2011 - * the Initial Developer. All Rights Reserved. - * - * Contributor(s): - * Jason Voll - * - * Alternatively, the contents of this file may be used under the terms of - * either the GNU General Public License Version 2 or later (the "GPL"), or - * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), - * in which case the provisions of the GPL or the LGPL are applicable instead - * of those above. If you wish to allow use of your version of this file only - * under the terms of either the GPL or the LGPL, and not to allow others to - * use your version of this file under the terms of the MPL, indicate your - * decision by deleting the provisions above and replace them with the notice - * and other provisions required by the GPL or the LGPL. If you do not delete - * the provisions above, a recipient may use your version of this file under - * the terms of any one of the MPL, the GPL or the LGPL. - * - * ***** END LICENSE BLOCK ***** */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ package org.mozilla.gecko.sync.crypto; @@ -46,105 +13,99 @@ import javax.crypto.spec.SecretKeySpec; import org.mozilla.gecko.sync.Utils; - /* * A standards-compliant implementation of RFC 5869 * for HMAC-based Key Derivation Function. * HMAC uses HMAC SHA256 standard. */ public class HKDF { + public static String HMAC_ALGORITHM = "hmacSHA256"; - /** - * Used for conversion in cases in which you *know* the encoding exists. - */ - public static final byte[] bytes(String in) { - try { - return in.getBytes("UTF-8"); - } catch (java.io.UnsupportedEncodingException e) { - return null; - } + /** + * Used for conversion in cases in which you *know* the encoding exists. + */ + public static final byte[] bytes(String in) { + try { + return in.getBytes("UTF-8"); + } catch (java.io.UnsupportedEncodingException e) { + return null; + } + } + + public static final int BLOCKSIZE = 256 / 8; + public static final byte[] HMAC_INPUT = bytes("Sync-AES_256_CBC-HMAC256"); + + /* + * Step 1 of RFC 5869 + * Get sha256HMAC Bytes + * Input: salt (message), IKM (input keyring material) + * Output: PRK (pseudorandom key) + */ + public static byte[] hkdfExtract(byte[] salt, byte[] IKM) throws NoSuchAlgorithmException, InvalidKeyException { + return digestBytes(IKM, makeHMACHasher(salt)); + } + + /* + * Step 2 of RFC 5869. + * Input: PRK from step 1, info, length. + * Output: OKM (output keyring material). + */ + public static byte[] hkdfExpand(byte[] prk, byte[] info, int len) throws NoSuchAlgorithmException, InvalidKeyException { + Mac hmacHasher = makeHMACHasher(prk); + + byte[] T = {}; + byte[] Tn = {}; + + int iterations = (int) Math.ceil(((double)len) / ((double)BLOCKSIZE)); + for (int i = 0; i < iterations; i++) { + Tn = digestBytes(Utils.concatAll(Tn, info, Utils.hex2Byte(Integer.toHexString(i + 1))), + hmacHasher); + T = Utils.concatAll(T, Tn); } - public static final int BLOCKSIZE = 256 / 8; - public static final byte[] HMAC_INPUT = bytes("Sync-AES_256_CBC-HMAC256"); + byte[] result = new byte[len]; + System.arraycopy(T, 0, result, 0, len); + return result; + } - /* - * Step 1 of RFC 5869 - * Get sha256HMAC Bytes - * Input: salt (message), IKM (input keyring material) - * Output: PRK (pseudorandom key) - */ - public static byte[] hkdfExtract(byte[] salt, byte[] IKM) { - return digestBytes(IKM, makeHMACHasher(salt)); + /* + * Make HMAC key + * Input: key (salt) + * Output: Key HMAC-Key + */ + public static Key makeHMACKey(byte[] key) { + if (key.length == 0) { + key = new byte[BLOCKSIZE]; } + return new SecretKeySpec(key, HMAC_ALGORITHM); + } - /* - * Step 2 of RFC 5869. - * Input: PRK from step 1, info, length. - * Output: OKM (output keyring material). - */ - public static byte[] hkdfExpand(byte[] prk, byte[] info, int len) { + /* + * Make an HMAC hasher + * Input: Key hmacKey + * Ouput: An HMAC Hasher + */ + public static Mac makeHMACHasher(byte[] key) throws NoSuchAlgorithmException, InvalidKeyException { + Mac hmacHasher = null; + hmacHasher = Mac.getInstance(HMAC_ALGORITHM); - Mac hmacHasher = makeHMACHasher(prk); + // If Mac.getInstance doesn't throw NoSuchAlgorithmException, hmacHasher is + // non-null. + assert(hmacHasher != null); - byte[] T = {}; - byte[] Tn = {}; + hmacHasher.init(makeHMACKey(key)); + return hmacHasher; + } - int iterations = (int) Math.ceil(((double)len) / ((double)BLOCKSIZE)); - for (int i = 0; i < iterations; i++) { - Tn = digestBytes(Utils.concatAll - (Tn, info, Utils.hex2Byte(Integer.toHexString(i + 1))), hmacHasher); - T = Utils.concatAll(T, Tn); - } - - byte[] result = new byte[len]; - System.arraycopy(T, 0, result, 0, len); - return result; - } - - /* - * Make HMAC key - * Input: key (salt) - * Output: Key HMAC-Key - */ - public static Key makeHMACKey(byte[] key) { - if (key.length == 0) { - key = new byte[BLOCKSIZE]; - } - return new SecretKeySpec(key, "HmacSHA256"); - } - - /* - * Make an HMAC hasher - * Input: Key hmacKey - * Ouput: An HMAC Hasher - */ - public static Mac makeHMACHasher(byte[] key) { - Mac hmacHasher = null; - try { - hmacHasher = Mac.getInstance("hmacSHA256"); - } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - } - - try { - hmacHasher.init(makeHMACKey(key)); - } catch (InvalidKeyException e) { - e.printStackTrace(); - } - - return hmacHasher; - } - - /* - * Hash bytes with given hasher - * Input: message to hash, HMAC hasher - * Output: hashed byte[]. - */ - public static byte[] digestBytes(byte[] message, Mac hasher) { - hasher.update(message); - byte[] ret = hasher.doFinal(); - hasher.reset(); - return ret; - } + /* + * Hash bytes with given hasher + * Input: message to hash, HMAC hasher + * Output: hashed byte[]. + */ + public static byte[] digestBytes(byte[] message, Mac hasher) { + hasher.update(message); + byte[] ret = hasher.doFinal(); + hasher.reset(); + return ret; + } } diff --git a/mobile/android/base/sync/crypto/KeyBundle.java b/mobile/android/base/sync/crypto/KeyBundle.java index e8007e4effa..ded6372bf30 100644 --- a/mobile/android/base/sync/crypto/KeyBundle.java +++ b/mobile/android/base/sync/crypto/KeyBundle.java @@ -45,6 +45,8 @@ import javax.crypto.Mac; import org.mozilla.apache.commons.codec.binary.Base64; import org.mozilla.gecko.sync.Utils; +import org.mozilla.gecko.sync.crypto.CryptoException; +import java.security.InvalidKeyException; public class KeyBundle { @@ -86,7 +88,7 @@ public class KeyBundle { * encryption key and the second iteration the HMAC key. * */ - public KeyBundle(String username, String base32SyncKey) { + public KeyBundle(String username, String base32SyncKey) throws CryptoException { if (base32SyncKey == null) { throw new IllegalArgumentException("No sync key provided."); } @@ -105,7 +107,15 @@ public class KeyBundle { byte[] syncKey = Utils.decodeFriendlyBase32(base32SyncKey); byte[] user = username.getBytes(); - Mac hmacHasher = HKDF.makeHMACHasher(syncKey); + Mac hmacHasher; + try { + hmacHasher = HKDF.makeHMACHasher(syncKey); + } catch (NoSuchAlgorithmException e) { + throw new CryptoException(e); + } catch (InvalidKeyException e) { + throw new CryptoException(e); + } + assert(hmacHasher != null); // If makeHMACHasher doesn't throw, then hmacHasher is non-null. byte[] encrBytes = Utils.concatAll(EMPTY_BYTES, HKDF.HMAC_INPUT, user, ENCR_INPUT_BYTES); byte[] encrKey = HKDF.digestBytes(encrBytes, hmacHasher); diff --git a/mobile/android/base/sync/cryptographer/SyncCryptographer.java b/mobile/android/base/sync/cryptographer/SyncCryptographer.java index 81ea8a9d536..b9530549ae8 100644 --- a/mobile/android/base/sync/cryptographer/SyncCryptographer.java +++ b/mobile/android/base/sync/cryptographer/SyncCryptographer.java @@ -344,7 +344,7 @@ public class SyncCryptographer { /* * Get the keys needed to encrypt the crypto/keys bundle. */ - public KeyBundle getCryptoKeysBundleKeys() { + public KeyBundle getCryptoKeysBundleKeys() throws CryptoException { return new KeyBundle(username, syncKey); } diff --git a/mobile/android/base/sync/jpake/JPakeClient.java b/mobile/android/base/sync/jpake/JPakeClient.java index ae392c3002d..ed2296cb891 100644 --- a/mobile/android/base/sync/jpake/JPakeClient.java +++ b/mobile/android/base/sync/jpake/JPakeClient.java @@ -74,6 +74,8 @@ import ch.boye.httpclientandroidlib.client.methods.HttpRequestBase; import ch.boye.httpclientandroidlib.entity.StringEntity; import ch.boye.httpclientandroidlib.impl.client.DefaultHttpClient; import ch.boye.httpclientandroidlib.message.BasicHeader; +import java.security.NoSuchAlgorithmException; +import java.security.InvalidKeyException; public class JPakeClient implements JPakeRequestDelegate { private static String LOG_TAG = "JPakeClient"; @@ -474,6 +476,14 @@ public class JPakeClient implements JPakeRequestDelegate { Log.e(LOG_TAG, "ZKP mismatch"); abort(Constants.JPAKE_ERROR_WRONGMESSAGE); e.printStackTrace(); + } catch (NoSuchAlgorithmException e) { + Log.e(LOG_TAG, "NoSuchAlgorithmException", e); + abort(Constants.JPAKE_ERROR_INTERNAL); + e.printStackTrace(); + } catch (InvalidKeyException e) { + Log.e(LOG_TAG, "InvalidKeyException", e); + abort(Constants.JPAKE_ERROR_INTERNAL); + e.printStackTrace(); } if (pairWithPin) { // Wait for other device to send verification of keys. diff --git a/mobile/android/base/sync/jpake/JPakeCrypto.java b/mobile/android/base/sync/jpake/JPakeCrypto.java index 0c13af7c541..ea37a57a2cd 100644 --- a/mobile/android/base/sync/jpake/JPakeCrypto.java +++ b/mobile/android/base/sync/jpake/JPakeCrypto.java @@ -51,6 +51,7 @@ import org.mozilla.gecko.sync.crypto.HKDF; import org.mozilla.gecko.sync.crypto.KeyBundle; import android.util.Log; +import java.security.InvalidKeyException; public class JPakeCrypto { private static final String LOG_TAG = "JPakeCrypto"; @@ -174,7 +175,7 @@ public class JPakeCrypto { * @throws IncorrectZkpException */ public static KeyBundle finalRound(String secret, JPakeParty jp) - throws IncorrectZkpException { + throws IncorrectZkpException, NoSuchAlgorithmException, InvalidKeyException { Log.d(LOG_TAG, "Final round started."); BigInteger gb = jp.gx1.multiply(jp.gx2).mod(P).multiply(jp.gx3) .mod(P); @@ -321,12 +322,12 @@ public class JPakeCrypto { /* * Helper function to generate encryption key and HMAC from a byte array. */ - public static void generateKeyAndHmac(BigInteger k, byte[] encOut, byte[] hmacOut) { + public static void generateKeyAndHmac(BigInteger k, byte[] encOut, byte[] hmacOut) throws NoSuchAlgorithmException, InvalidKeyException { // Generate HMAC and Encryption keys from synckey. byte[] zerokey = new byte[32]; byte[] prk = HMACSHA256(BigIntegerHelper.BigIntegerToByteArrayWithoutSign(k), zerokey); - byte[] okm = HKDF.hkdfExpand(prk, HKDF.HMAC_INPUT, 32 * 2); + byte[] okm = HKDF.hkdfExpand(prk, HKDF.HMAC_INPUT, 32 * 2); System.arraycopy(okm, 0, encOut, 0, 32); System.arraycopy(okm, 32, hmacOut, 0, 32); } diff --git a/mobile/android/base/sync/net/BaseResource.java b/mobile/android/base/sync/net/BaseResource.java index 614ec51b114..3d333b9ee23 100644 --- a/mobile/android/base/sync/net/BaseResource.java +++ b/mobile/android/base/sync/net/BaseResource.java @@ -101,7 +101,7 @@ public class BaseResource implements Resource { } public BaseResource(String uri, boolean rewrite) throws URISyntaxException { - this(new URI(uri), rewrite); + this(new URI(uri), rewrite); } public BaseResource(URI uri, boolean rewrite) { From 0985211e31751a90f1fc047361365ea543120ad8 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 10/16] Bug 718703 - Part 1: Sync crashes from Pair a Device link in sync settings. r=rnewman --- .../{sync_icon.png => sync_ic_launcher.png} | Bin .../base/resources/xml/sync_authenticator.xml | 6 +++--- .../xml/{sync_options.xml => sync_options.xml.in} | 5 +++-- mobile/android/sync/android-drawable-resources.mn | 2 +- mobile/android/sync/android-xml-resources.mn | 1 - .../manifests/SyncAndroidManifest_activities.xml.in | 8 ++------ mobile/android/sync/strings.xml.in | 3 ++- 7 files changed, 11 insertions(+), 14 deletions(-) rename mobile/android/base/resources/drawable/{sync_icon.png => sync_ic_launcher.png} (100%) rename mobile/android/base/resources/xml/{sync_options.xml => sync_options.xml.in} (86%) diff --git a/mobile/android/base/resources/drawable/sync_icon.png b/mobile/android/base/resources/drawable/sync_ic_launcher.png similarity index 100% rename from mobile/android/base/resources/drawable/sync_icon.png rename to mobile/android/base/resources/drawable/sync_ic_launcher.png diff --git a/mobile/android/base/resources/xml/sync_authenticator.xml b/mobile/android/base/resources/xml/sync_authenticator.xml index 634ef2af4f0..bbf4b7a87a7 100644 --- a/mobile/android/base/resources/xml/sync_authenticator.xml +++ b/mobile/android/base/resources/xml/sync_authenticator.xml @@ -1,7 +1,7 @@ \ No newline at end of file + android:accountPreferences="@xml/sync_options" /> diff --git a/mobile/android/base/resources/xml/sync_options.xml b/mobile/android/base/resources/xml/sync_options.xml.in similarity index 86% rename from mobile/android/base/resources/xml/sync_options.xml rename to mobile/android/base/resources/xml/sync_options.xml.in index b0480d010a0..159eca9f571 100644 --- a/mobile/android/base/resources/xml/sync_options.xml +++ b/mobile/android/base/resources/xml/sync_options.xml.in @@ -1,3 +1,4 @@ +#filter substitution - \ No newline at end of file + diff --git a/mobile/android/sync/android-drawable-resources.mn b/mobile/android/sync/android-drawable-resources.mn index ab2b76c5d5d..35931c8c4d3 100644 --- a/mobile/android/sync/android-drawable-resources.mn +++ b/mobile/android/sync/android-drawable-resources.mn @@ -1,2 +1,2 @@ mobile/android/base/resources/drawable/pin_background.xml -mobile/android/base/resources/drawable/sync_icon.png +mobile/android/base/resources/drawable/sync_ic_launcher.png diff --git a/mobile/android/sync/android-xml-resources.mn b/mobile/android/sync/android-xml-resources.mn index 746b3ff5bc1..73ac49ada82 100644 --- a/mobile/android/sync/android-xml-resources.mn +++ b/mobile/android/sync/android-xml-resources.mn @@ -1,3 +1,2 @@ mobile/android/base/resources/xml/sync_authenticator.xml -mobile/android/base/resources/xml/sync_options.xml mobile/android/base/resources/xml/sync_syncadapter.xml diff --git a/mobile/android/sync/manifests/SyncAndroidManifest_activities.xml.in b/mobile/android/sync/manifests/SyncAndroidManifest_activities.xml.in index 733106faedb..64a2b633ac1 100644 --- a/mobile/android/sync/manifests/SyncAndroidManifest_activities.xml.in +++ b/mobile/android/sync/manifests/SyncAndroidManifest_activities.xml.in @@ -1,7 +1,6 @@ - - + \ No newline at end of file + android:name="org.mozilla.gecko.sync.setup.activities.SetupSuccessActivity" /> \ No newline at end of file diff --git a/mobile/android/sync/strings.xml.in b/mobile/android/sync/strings.xml.in index 6c83c33a0b8..17c04a93f91 100644 --- a/mobile/android/sync/strings.xml.in +++ b/mobile/android/sync/strings.xml.in @@ -59,4 +59,5 @@ &bookmarks.folder.mobile.label; - &sync.notification.oneaccount.label; \ No newline at end of file + &sync.notification.oneaccount.label; + From c06569629dc60eea3ba88b75ecbe8203af26e8b9 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:28 -0800 Subject: [PATCH 11/16] Bug 718703 - Part 2: makefile change to preprocess sync_options.xml. r=ted --- mobile/android/base/Makefile.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in index 116c6cb0c11..ae744bd52bf 100644 --- a/mobile/android/base/Makefile.in +++ b/mobile/android/base/Makefile.in @@ -57,8 +57,8 @@ SYNC_RES_DRAWABLE_MDPI=$(shell cat $(topsrcdir)/mobile/android/sync/android-draw SYNC_RES_DRAWABLE_HDPI=$(shell cat $(topsrcdir)/mobile/android/sync/android-drawable-hdpi-resources.mn | tr '\n' ' ';) SYNC_RES_LAYOUT=$(shell cat $(topsrcdir)/mobile/android/sync/android-layout-resources.mn | tr '\n' ' ';) SYNC_RES_VALUES=$(shell cat $(topsrcdir)/mobile/android/sync/android-values-resources.mn | tr '\n' ' ';) -SYNC_RES_XML=res/xml/sync_authenticator.xml res/xml/sync_options.xml -SYNC_PP_RES_XML=res/xml/sync_syncadapter.xml +SYNC_RES_XML=res/xml/sync_authenticator.xml +SYNC_PP_RES_XML=res/xml/sync_syncadapter.xml res/xml/sync_options.xml FENNEC_JAVA_FILES = \ AboutHomeContent.java \ @@ -636,10 +636,10 @@ $(RESOURCES): $(RES_DIRS) $(subst res/,$(srcdir)/resources/,$(RESOURCES)) $(NSINSTALL) $(subst res/,$(srcdir)/resources/,$@) $(dir $@) -R.java: $(MOZ_APP_ICON) $(RESOURCES) $(RES_DRAWABLE) $(RES_DRAWABLE_LDPI) $(RES_DRAWABLE_MDPI) $(RES_DRAWABLE_HDPI) $(PP_RES_XML) res/values/defaults.xml res/drawable/sync_icon.png res/drawable/icon.png res/drawable-hdpi/icon.png res/values/strings.xml AndroidManifest.xml FORCE +R.java: $(MOZ_APP_ICON) $(RESOURCES) $(RES_DRAWABLE) $(RES_DRAWABLE_LDPI) $(RES_DRAWABLE_MDPI) $(RES_DRAWABLE_HDPI) $(PP_RES_XML) res/values/defaults.xml res/drawable/sync_ic_launcher.png res/drawable/icon.png res/drawable-hdpi/icon.png res/values/strings.xml AndroidManifest.xml FORCE $(AAPT) package -f -M AndroidManifest.xml -I $(ANDROID_SDK)/android.jar -S res -J . --custom-package org.mozilla.gecko -gecko.ap_: AndroidManifest.xml res/drawable/sync_icon.png res/drawable/icon.png res/drawable-hdpi/icon.png $(RESOURCES) $(RES_DRAWABLE) $(RES_DRAWABLE_LDPI) $(RES_DRAWABLE_MDPI) $(RES_DRAWABLE_HDPI) $(PP_RES_XML) res/values/defaults.xml res/values/strings.xml FORCE +gecko.ap_: AndroidManifest.xml res/drawable/sync_ic_launcher.png res/drawable/icon.png res/drawable-hdpi/icon.png $(RESOURCES) $(RES_DRAWABLE) $(RES_DRAWABLE_LDPI) $(RES_DRAWABLE_MDPI) $(RES_DRAWABLE_HDPI) $(PP_RES_XML) res/values/defaults.xml res/values/strings.xml FORCE $(AAPT) package -f -M AndroidManifest.xml -I $(ANDROID_SDK)/android.jar -S res -F $@ libs:: classes.dex package-name.txt From ccffd1a626dc55af44384c25013678c998c3d6e6 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:29 -0800 Subject: [PATCH 12/16] Bug 720934 - Part 2: Collected Android Sync 0.4 code drop (Bug 722945, Bug 709348). a=mobile Includes: Bug 722945 - Transactional behavior for store (don't return stored records in subsequent fetch). Bug 709348 - Refactor and improve reconciling and storing of records. --- mobile/android/base/sync/Utils.java | 23 ++ .../net/SyncStorageCollectionRequest.java | 6 + .../sync/net/SyncStorageRequestDelegate.java | 4 + .../repositories/HashSetStoreTracker.java | 55 ++++ .../base/sync/repositories/RecordFilter.java | 11 + .../sync/repositories/RepositorySession.java | 102 ++++++- .../base/sync/repositories/StoreTracker.java | 78 ++++++ .../StoreTrackingRepositorySession.java | 62 +++++ .../AndroidBrowserBookmarksRepository.java | 1 + ...roidBrowserBookmarksRepositorySession.java | 31 +-- .../AndroidBrowserHistoryDataAccessor.java | 14 +- ...ndroidBrowserHistoryRepositorySession.java | 5 + ...roidBrowserPasswordsRepositorySession.java | 4 + .../AndroidBrowserRepositoryDataAccessor.java | 11 + .../AndroidBrowserRepositorySession.java | 258 ++++++++++++------ .../repositories/domain/BookmarkRecord.java | 4 +- .../ConcurrentRecordConsumer.java | 20 +- mobile/android/sync/android-xml-resources.mn | 2 - mobile/android/sync/java-sources.mn | 2 +- 19 files changed, 568 insertions(+), 125 deletions(-) create mode 100644 mobile/android/base/sync/repositories/HashSetStoreTracker.java create mode 100644 mobile/android/base/sync/repositories/RecordFilter.java create mode 100644 mobile/android/base/sync/repositories/StoreTracker.java create mode 100644 mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java delete mode 100644 mobile/android/sync/android-xml-resources.mn diff --git a/mobile/android/base/sync/Utils.java b/mobile/android/base/sync/Utils.java index 7c12cf475fa..0f5be6122bf 100644 --- a/mobile/android/base/sync/Utils.java +++ b/mobile/android/base/sync/Utils.java @@ -76,6 +76,29 @@ public class Utils { } } + public static void error(String logTag, String message) { + logToStdout(logTag, " :: ERROR: ", message); + Log.i(logTag, message); + } + + public static void info(String logTag, String message) { + logToStdout(logTag, " :: INFO: ", message); + Log.i(logTag, message); + } + + public static void debug(String logTag, String message) { + logToStdout(logTag, " :: DEBUG: ", message); + Log.d(logTag, message); + } + + public static void trace(String logTag, String message) { + if (!ENABLE_TRACE_LOGGING) { + return; + } + logToStdout(logTag, " :: TRACE: ", message); + Log.d(logTag, message); + } + public static String generateGuid() { byte[] encodedBytes = Base64.encodeBase64(generateRandomBytes(9), false); return new String(encodedBytes).replace("+", "-").replace("/", "_"); diff --git a/mobile/android/base/sync/net/SyncStorageCollectionRequest.java b/mobile/android/base/sync/net/SyncStorageCollectionRequest.java index af971bf470f..da008fdab45 100644 --- a/mobile/android/base/sync/net/SyncStorageCollectionRequest.java +++ b/mobile/android/base/sync/net/SyncStorageCollectionRequest.java @@ -98,6 +98,12 @@ public class SyncStorageCollectionRequest extends SyncStorageRequest { return; } + // TODO: at this point we can access X-Weave-Timestamp, compare + // that to our local timestamp, and compute an estimate of clock + // skew. We can provide this to the incremental delegate, which + // will allow it to seamlessly correct timestamps on the records + // it processes. Bug 721887. + // Line-by-line processing, then invoke success. SyncStorageCollectionRequestDelegate delegate = (SyncStorageCollectionRequestDelegate) this.request.delegate; InputStream content = null; diff --git a/mobile/android/base/sync/net/SyncStorageRequestDelegate.java b/mobile/android/base/sync/net/SyncStorageRequestDelegate.java index a2d856a543f..237e374674c 100644 --- a/mobile/android/base/sync/net/SyncStorageRequestDelegate.java +++ b/mobile/android/base/sync/net/SyncStorageRequestDelegate.java @@ -40,6 +40,10 @@ package org.mozilla.gecko.sync.net; public interface SyncStorageRequestDelegate { String credentials(); String ifUnmodifiedSince(); + + // TODO: at this point we can access X-Weave-Timestamp, compare + // that to our local timestamp, and compute an estimate of clock + // skew. Bug 721887. void handleRequestSuccess(SyncStorageResponse response); void handleRequestFailure(SyncStorageResponse response); void handleRequestError(Exception ex); diff --git a/mobile/android/base/sync/repositories/HashSetStoreTracker.java b/mobile/android/base/sync/repositories/HashSetStoreTracker.java new file mode 100644 index 00000000000..0045d206328 --- /dev/null +++ b/mobile/android/base/sync/repositories/HashSetStoreTracker.java @@ -0,0 +1,55 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.sync.repositories; + +import java.util.HashSet; + +import org.mozilla.gecko.sync.repositories.domain.Record; + +public class HashSetStoreTracker implements StoreTracker { + + // Guarded by `this`. + // Used to store GUIDs that were not locally modified but + // have been modified by a call to `store`, and thus + // should not be returned by a subsequent fetch. + private HashSet guids; + + public HashSetStoreTracker() { + guids = new HashSet(); + } + + @Override + public String toString() { + return "#"; + } + + @Override + public synchronized boolean trackRecordForExclusion(String guid) { + return (guid != null) && guids.add(guid); + } + + @Override + public synchronized boolean isTrackedForExclusion(String guid) { + return (guid != null) && guids.contains(guid); + } + + @Override + public synchronized boolean untrackStoredForExclusion(String guid) { + return (guid != null) && guids.remove(guid); + } + + @Override + public RecordFilter getFilter() { + if (guids.size() == 0) { + return null; + } + return new RecordFilter() { + @Override + public boolean excludeRecord(Record r) { + return isTrackedForExclusion(r.guid); + } + }; + } +} diff --git a/mobile/android/base/sync/repositories/RecordFilter.java b/mobile/android/base/sync/repositories/RecordFilter.java new file mode 100644 index 00000000000..7f699b27b0f --- /dev/null +++ b/mobile/android/base/sync/repositories/RecordFilter.java @@ -0,0 +1,11 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.sync.repositories; + +import org.mozilla.gecko.sync.repositories.domain.Record; + +public interface RecordFilter { + public boolean excludeRecord(Record r); +} diff --git a/mobile/android/base/sync/repositories/RepositorySession.java b/mobile/android/base/sync/repositories/RepositorySession.java index 87904500cb8..d2626c084db 100644 --- a/mobile/android/base/sync/repositories/RepositorySession.java +++ b/mobile/android/base/sync/repositories/RepositorySession.java @@ -41,6 +41,7 @@ package org.mozilla.gecko.sync.repositories; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import org.mozilla.gecko.sync.Utils; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate; @@ -74,6 +75,15 @@ public abstract class RepositorySession { } private static final String LOG_TAG = "RepositorySession"; + + private static void error(String message) { + Utils.error(LOG_TAG, message); + } + + protected static void trace(String message) { + Utils.trace(LOG_TAG, message); + } + protected SessionStatus status = SessionStatus.UNSTARTED; protected Repository repository; protected RepositorySessionStoreDelegate delegate; @@ -163,11 +173,6 @@ public abstract class RepositorySession { } } - private static void error(String msg) { - System.err.println("ERROR: " + msg); - Log.e(LOG_TAG, msg); - } - /** * Synchronously perform the shared work of beginning. Throws on failure. * @throws InvalidSessionTransitionException @@ -251,4 +256,91 @@ public abstract class RepositorySession { storeWorkQueue.shutdown(); delegateQueue.shutdown(); } + + /** + * Produce a record that is some combination of the remote and local records + * provided. + * + * The returned record must be produced without mutating either remoteRecord + * or localRecord. It is acceptable to return either remoteRecord or localRecord + * if no modifications are to be propagated. + * + * The returned record *should* have the local androidID and the remote GUID, + * and some optional merge of data from the two records. + * + * This method can be called with records that are identical, or differ in + * any regard. + * + * This method will not be called if: + * + * * either record is marked as deleted, or + * * there is no local mapping for a new remote record. + * + * Otherwise, it will be called precisely once. + * + * Side-effects (e.g., for transactional storage) can be hooked in here. + * + * @param remoteRecord + * The record retrieved from upstream, already adjusted for clock skew. + * @param localRecord + * The record retrieved from local storage. + * @param lastRemoteRetrieval + * The timestamp of the last retrieved set of remote records, adjusted for + * clock skew. + * @param lastLocalRetrieval + * The timestamp of the last retrieved set of local records. + * @return + * A Record instance to apply, or null to apply nothing. + */ + protected Record reconcileRecords(final Record remoteRecord, + final Record localRecord, + final long lastRemoteRetrieval, + final long lastLocalRetrieval) { + Log.d(LOG_TAG, "Reconciling remote " + remoteRecord.guid + " against local " + localRecord.guid); + + if (localRecord.equalPayloads(remoteRecord)) { + if (remoteRecord.lastModified > localRecord.lastModified) { + Log.d(LOG_TAG, "Records are equal. No record application needed."); + return null; + } + + // Local wins. + return null; + } + + // TODO: Decide what to do based on: + // * Which of the two records is modified; + // * Whether they are equal or congruent; + // * The modified times of each record (interpreted through the lens of clock skew); + // * ... + boolean localIsMoreRecent = localRecord.lastModified > remoteRecord.lastModified; + Log.d(LOG_TAG, "Local record is more recent? " + localIsMoreRecent); + Record donor = localIsMoreRecent ? localRecord : remoteRecord; + + // Modify the local record to match the remote record's GUID and values. + // Preserve the local Android ID, and merge data where possible. + // It sure would be nice if copyWithIDs didn't give a shit about androidID, mm? + Record out = donor.copyWithIDs(remoteRecord.guid, localRecord.androidID); + + // We don't want to upload the record if the remote record was + // applied without changes. + // This logic will become more complicated as reconciling becomes smarter. + if (!localIsMoreRecent) { + trackRecord(out); + } + return out; + } + + /** + * Depending on the RepositorySession implementation, track + * that a record — most likely a brand-new record that has been + * applied unmodified — should be tracked so as to not be uploaded + * redundantly. + * + * The default implementation does nothing. + * + * @param record + */ + protected synchronized void trackRecord(Record record) { + } } diff --git a/mobile/android/base/sync/repositories/StoreTracker.java b/mobile/android/base/sync/repositories/StoreTracker.java new file mode 100644 index 00000000000..94017a2fc80 --- /dev/null +++ b/mobile/android/base/sync/repositories/StoreTracker.java @@ -0,0 +1,78 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.sync.repositories; + +/** + * Our hacky version of transactional semantics. The goal is to prevent + * the following situation: + * + * * AAA is not modified locally. + * * A modified AAA is downloaded during the storing phase. Its local + * timestamp is advanced. + * * The direction of syncing changes, and AAA is now uploaded to the server. + * + * The following situation should still be supported: + * + * * AAA is not modified locally. + * * A modified AAA is downloaded and merged with the local AAA. + * * The merged AAA is uploaded to the server. + * + * As should: + * + * * AAA is modified locally. + * * A modified AAA is downloaded, and discarded or merged. + * * The current version of AAA is uploaded to the server. + * + * We achieve this by tracking GUIDs during the storing phase. If we + * apply a record such that the local copy is substantially the same + * as the record we just downloaded, we add it to a list of records + * to avoid uploading. The definition of "substantially the same" + * depends on the particular repository. The only consideration is "do we + * want to upload this record in this sync?". + * + * Note that items are removed from this list when a fetch that + * considers them for upload completes successfully. The entire list + * is discarded when the session is completed. + * + * This interface exposes methods to: + * + * * During a store, recording that a record has been stored, and should + * thus not be returned in subsequent fetches; + * * During a fetch, checking whether a record should be returned. + * + * In the future this might also grow self-persistence. + * + * See also RepositorySession.trackRecord. + * + * @author rnewman + * + */ +public interface StoreTracker { + + /** + * @param guid + * The GUID of the item to track. + * @return + * Whether the GUID was a newly tracked value. + */ + public boolean trackRecordForExclusion(String guid); + + /** + * @param guid + * The GUID of the item to check. + * @return + * true if the item is already tracked. + */ + public boolean isTrackedForExclusion(String guid); + + /** + * + * @param guid + * @return true if the specified GUID was removed from the tracked set. + */ + public boolean untrackStoredForExclusion(String guid); + + public RecordFilter getFilter(); +} diff --git a/mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java b/mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java new file mode 100644 index 00000000000..9574652062d --- /dev/null +++ b/mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java @@ -0,0 +1,62 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.sync.repositories; + +import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate; +import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate; +import org.mozilla.gecko.sync.repositories.domain.Record; + +import android.util.Log; + +public abstract class StoreTrackingRepositorySession extends RepositorySession { + private static final String LOG_TAG = "StoreTrackingRepositorySession"; + protected StoreTracker storeTracker; + + protected static StoreTracker createStoreTracker() { + return new HashSetStoreTracker(); + } + + public StoreTrackingRepositorySession(Repository repository) { + super(repository); + } + + @Override + public void begin(RepositorySessionBeginDelegate delegate) { + RepositorySessionBeginDelegate deferredDelegate = delegate.deferredBeginDelegate(delegateQueue); + try { + super.sharedBegin(); + } catch (InvalidSessionTransitionException e) { + deferredDelegate.onBeginFailed(e); + return; + } + // Or do this in your own subclass. + storeTracker = createStoreTracker(); + deferredDelegate.onBeginSucceeded(this); + } + + @Override + protected synchronized void trackRecord(Record record) { + if (this.storeTracker == null) { + throw new IllegalStateException("Store tracker not yet initialized!"); + } + + Log.d(LOG_TAG, "Tracking record " + record.guid + + " (" + record.lastModified + ") to avoid re-upload."); + // Future: we care about the timestamp… + this.storeTracker.trackRecordForExclusion(record.guid); + } + + @Override + public void abort(RepositorySessionFinishDelegate delegate) { + this.storeTracker = null; + super.abort(delegate); + } + + @Override + public void finish(RepositorySessionFinishDelegate delegate) { + this.storeTracker = null; + super.finish(delegate); + } +} \ No newline at end of file diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepository.java b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepository.java index dceb5d18bc7..6de0ac72fc3 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepository.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepository.java @@ -45,6 +45,7 @@ import android.content.Context; public class AndroidBrowserBookmarksRepository extends AndroidBrowserRepository implements BookmarksRepository { + @Override protected void sessionCreator(RepositorySessionCreationDelegate delegate, Context context) { AndroidBrowserBookmarksRepositorySession session = new AndroidBrowserBookmarksRepositorySession(AndroidBrowserBookmarksRepository.this, context); delegate.onSessionCreated(session); diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java index 010966bffe1..1e0d525c4d0 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java @@ -67,12 +67,6 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo private AndroidBrowserBookmarksDataAccessor dataAccessor; private int needsReparenting = 0; - private static void trace(String string) { - if (Utils.ENABLE_TRACE_LOGGING) { - Log.d(LOG_TAG, string); - } - } - /** * Return true if the provided record GUID should be skipped * in child lists or fetch results. @@ -313,10 +307,9 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo super.finish(delegate); }; - // TODO this code is yucky, cleanup or comment or something - @SuppressWarnings("unchecked") @Override - protected long insert(Record record) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + @SuppressWarnings("unchecked") + protected Record prepareRecord(Record record) { BookmarkRecord bmk = (BookmarkRecord) record; // Check if parent exists @@ -355,16 +348,21 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo " (" + bmk.parentID + ", " + bmk.parentName + ", " + bmk.pos + ")"); } - long id = RepoUtils.getAndroidIdFromUri(dbHelper.insert(bmk)); - Log.d(LOG_TAG, "Inserted as " + id); + return bmk; + } - putRecordToGuidMap(buildRecordString(bmk), bmk.guid); - bmk.androidID = id; + @Override + @SuppressWarnings("unchecked") + protected void updateBookkeeping(Record record) throws NoGuidForIdException, + NullCursorException, + ParentNotFoundException { + super.updateBookkeeping(record); + BookmarkRecord bmk = (BookmarkRecord) record; // If record is folder, update maps and re-parent children if necessary if (bmk.type.equalsIgnoreCase(AndroidBrowserBookmarksDataAccessor.TYPE_FOLDER)) { - guidToID.put(bmk.guid, id); - idToGuid.put(id, bmk.guid); + guidToID.put(bmk.guid, bmk.androidID); + idToGuid.put(bmk.androidID, bmk.guid); JSONArray childArray = bmk.children; @@ -376,14 +374,13 @@ public class AndroidBrowserBookmarksRepositorySession extends AndroidBrowserRepo childArray.add(child); } position = childArray.indexOf(child); - dataAccessor.updateParentAndPosition(child, id, position); + dataAccessor.updateParentAndPosition(child, bmk.androidID, position); needsReparenting--; } missingParentToChildren.remove(bmk.guid); } parentToChildArray.put(bmk.guid, childArray); } - return id; } @Override diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java b/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java index 59358b3e255..40e8cc08760 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryDataAccessor.java @@ -102,8 +102,18 @@ public class AndroidBrowserHistoryDataAccessor extends AndroidBrowserRepositoryD dataExtender.store(record.guid, rec.visits); Log.d(LOG_TAG, "Storing record " + record.guid); return super.insert(record); - } - + } + + @Override + public void update(String oldGUID, Record newRecord) { + HistoryRecord rec = (HistoryRecord) newRecord; + String newGUID = newRecord.guid; + Log.d(LOG_TAG, "Storing visits for " + newGUID + ", replacing " + oldGUID); + dataExtender.delete(oldGUID); + dataExtender.store(newGUID, rec.visits); + super.update(oldGUID, newRecord); + } + @Override protected void delete(String guid) { Log.d(LOG_TAG, "Deleting record " + guid); diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java index 7a97e32db21..b4d07ecbfdd 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java @@ -135,4 +135,9 @@ public class AndroidBrowserHistoryRepositorySession extends AndroidBrowserReposi hist.visits = visitsArray; return hist; } + + @Override + protected Record prepareRecord(Record record) { + return record; + } } diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java index e898b502d4d..c53416c949c 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java @@ -63,4 +63,8 @@ public class AndroidBrowserPasswordsRepositorySession extends return rec.hostname + rec.formSubmitURL + rec.httpRealm + rec.username; } + @Override + protected Record prepareRecord(Record record) { + return record; + } } diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java index 6e50f082796..25f85fa99b2 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java @@ -101,8 +101,19 @@ public abstract class AndroidBrowserRepositoryDataAccessor { Log.w(LOG_TAG, "Unexpectedly deleted " + deleted + " rows for guid " + guid); } + public void update(String guid, Record newRecord) { + String where = BrowserContract.SyncColumns.GUID + " = ?"; + String[] args = new String[] { guid }; + ContentValues cv = getContentValues(newRecord); + int updated = context.getContentResolver().update(getUri(), cv, where, args); + if (updated != 1) { + Log.w(LOG_TAG, "Unexpectedly updated " + updated + " rows for guid " + guid); + } + } + public Uri insert(Record record) { ContentValues cv = getContentValues(record); + Log.d(LOG_TAG, "INSERTING: " + cv.getAsString("guid")); return context.getContentResolver().insert(getUri(), cv); } diff --git a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java index 83de5ff6169..3ac08dfcb8a 100644 --- a/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java +++ b/mobile/android/base/sync/repositories/android/AndroidBrowserRepositorySession.java @@ -51,8 +51,9 @@ import org.mozilla.gecko.sync.repositories.NoStoreDelegateException; import org.mozilla.gecko.sync.repositories.NullCursorException; import org.mozilla.gecko.sync.repositories.ParentNotFoundException; import org.mozilla.gecko.sync.repositories.ProfileDatabaseException; +import org.mozilla.gecko.sync.repositories.RecordFilter; import org.mozilla.gecko.sync.repositories.Repository; -import org.mozilla.gecko.sync.repositories.RepositorySession; +import org.mozilla.gecko.sync.repositories.StoreTrackingRepositorySession; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate; import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionGuidsSinceDelegate; @@ -60,6 +61,7 @@ import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionWipeDelega import org.mozilla.gecko.sync.repositories.domain.Record; import android.database.Cursor; +import android.net.Uri; import android.util.Log; /** @@ -84,10 +86,10 @@ import android.util.Log; * @author rnewman * */ -public abstract class AndroidBrowserRepositorySession extends RepositorySession { +public abstract class AndroidBrowserRepositorySession extends StoreTrackingRepositorySession { protected AndroidBrowserRepositoryDataAccessor dbHelper; - protected static final String LOG_TAG = "AndroidBrowserRepositorySession"; + public static final String LOG_TAG = "AndroidBrowserRepositorySession"; private HashMap recordToGuid; public AndroidBrowserRepositorySession(Repository repository) { @@ -150,15 +152,17 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession deferredDelegate.onBeginFailed(e); return; } + storeTracker = createStoreTracker(); deferredDelegate.onBeginSucceeded(this); } protected abstract String buildRecordString(Record record); protected void checkDatabase() throws ProfileDatabaseException, NullCursorException { - Log.i(LOG_TAG, "Checking database."); + Utils.info(LOG_TAG, "BEGIN: checking database."); try { dbHelper.fetch(new String[] { "none" }).close(); + Utils.info(LOG_TAG, "END: checking database."); } catch (NullPointerException e) { throw new ProfileDatabaseException(e); } @@ -224,7 +228,7 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession @Override public void fetch(String[] guids, RepositorySessionFetchRecordsDelegate delegate) { - FetchRunnable command = new FetchRunnable(guids, now(), delegate); + FetchRunnable command = new FetchRunnable(guids, now(), null, delegate); delegateQueue.execute(command); } @@ -235,7 +239,7 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession this.delegate = delegate; } - protected void fetchFromCursor(Cursor cursor, long end) { + protected void fetchFromCursor(Cursor cursor, RecordFilter filter, long end) { Log.d(LOG_TAG, "Fetch from cursor:"); try { try { @@ -245,9 +249,13 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession } while (!cursor.isAfterLast()) { Log.d(LOG_TAG, "... one more record."); - Record r = transformRecord(recordFromMirrorCursor(cursor)); + Record r = recordFromMirrorCursor(cursor); if (r != null) { - delegate.onFetchedRecord(r); + if (filter == null || !filter.excludeRecord(r)) { + delegate.onFetchedRecord(transformRecord(r)); + } else { + Log.d(LOG_TAG, "Filter says to skip record."); + } } cursor.moveToNext(); } @@ -270,13 +278,16 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession class FetchRunnable extends FetchingRunnable { private String[] guids; private long end; + private RecordFilter filter; public FetchRunnable(String[] guids, long end, + RecordFilter filter, RepositorySessionFetchRecordsDelegate delegate) { super(delegate); - this.guids = guids; - this.end = end; + this.guids = guids; + this.end = end; + this.filter = filter; } @Override @@ -294,7 +305,7 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession try { Cursor cursor = dbHelper.fetch(guids); - this.fetchFromCursor(cursor, end); + this.fetchFromCursor(cursor, filter, end); } catch (NullCursorException e) { delegate.onFetchFailed(e, null); } @@ -304,21 +315,28 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession @Override public void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) { + if (this.storeTracker == null) { + throw new IllegalStateException("Store tracker not yet initialized!"); + } + Log.i(LOG_TAG, "Running fetchSince(" + timestamp + ")."); - FetchSinceRunnable command = new FetchSinceRunnable(timestamp, now(), delegate); + FetchSinceRunnable command = new FetchSinceRunnable(timestamp, now(), this.storeTracker.getFilter(), delegate); delegateQueue.execute(command); } class FetchSinceRunnable extends FetchingRunnable { private long since; private long end; + private RecordFilter filter; public FetchSinceRunnable(long since, long end, + RecordFilter filter, RepositorySessionFetchRecordsDelegate delegate) { super(delegate); - this.since = since; - this.end = end; + this.since = since; + this.end = end; + this.filter = filter; } @Override @@ -330,7 +348,7 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession try { Cursor cursor = dbHelper.fetchSince(since); - this.fetchFromCursor(cursor, end); + this.fetchFromCursor(cursor, filter, end); } catch (NullCursorException e) { delegate.onFetchFailed(e, null); return; @@ -343,15 +361,6 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession this.fetchSince(0, delegate); } - private void trace(String m) { - if (Utils.ENABLE_TRACE_LOGGING) { - if (Utils.LOG_TO_STDOUT) { - System.out.println(LOG_TAG + "::TRACE " + m); - } - Log.d(LOG_TAG, m); - } - } - @Override public void store(final Record record) throws NoStoreDelegateException { if (delegate == null) { @@ -386,34 +395,101 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession return; } - // TODO: - // TODO: rnewman 2012-01-13: read and improve this code. - // TODO: + + // TODO: lift these into the session. + // Temporary: this matches prior syncing semantics, in which only + // the relationship between the local and remote record is considered. + // In the future we'll track these two timestamps and use them to + // determine which records have changed, and thus process incoming + // records more efficiently. + long lastLocalRetrieval = 0; // lastSyncTimestamp? + long lastRemoteRetrieval = 0; // TODO: adjust for clock skew. + boolean remotelyModified = record.lastModified > lastRemoteRetrieval; + Record existingRecord; try { - existingRecord = findExistingRecord(record); - - // If the record is new and not deleted, store it - if (existingRecord == null && !record.deleted) { - record.androidID = insert(record); - } else if (existingRecord != null) { - - // If the incoming record is marked deleted and - // our existing record has a newer timestamp, then - // discard the incoming record. - if (record.deleted && existingRecord.lastModified > record.lastModified) { - delegate.onRecordStoreSucceeded(record); + // GUID matching only: deleted records don't have a payload with which to search. + existingRecord = recordForGUID(record.guid); + if (record.deleted) { + if (existingRecord == null) { + // We're done. Don't bother with a callback. That can change later + // if we want it to. + trace("Incoming record " + record.guid + " is deleted, and no local version. Bye!"); return; } - // Now's a great time to do expensive additions. - existingRecord = transformRecord(existingRecord); - dbHelper.delete(existingRecord); - if (!record.deleted) { - // Record exists already, need to figure out what to store. - Record store = reconcileRecords(existingRecord, record); - record.androidID = insert(store); + + if (existingRecord.deleted) { + trace("Local record already deleted. Bye!"); + return; } + + // Which one wins? + if (!remotelyModified) { + trace("Ignoring deleted record from the past."); + return; + } + + boolean locallyModified = existingRecord.lastModified > lastLocalRetrieval; + if (!locallyModified) { + trace("Remote modified, local not. Deleting."); + storeRecordDeletion(record); + return; + } + + trace("Both local and remote records have been modified."); + if (record.lastModified > existingRecord.lastModified) { + trace("Remote is newer, and deleted. Deleting local."); + storeRecordDeletion(record); + return; + } + + trace("Remote is older, local is not deleted. Ignoring."); + if (!locallyModified) { + Log.w(LOG_TAG, "Inconsistency: old remote record is deleted, but local record not modified!"); + // Ensure that this is tracked for upload. + } + return; } + // End deletion logic. + + // Now we're processing a non-deleted incoming record. + if (existingRecord == null) { + trace("Looking up match for record " + record.guid); + existingRecord = findExistingRecord(record); + } + + if (existingRecord == null) { + // The record is new. + trace("No match. Inserting."); + Record inserted = insert(record); + trackRecord(inserted); + delegate.onRecordStoreSucceeded(inserted); + return; + } + + // We found a local dupe. + trace("Incoming record " + record.guid + " dupes to local record " + existingRecord.guid); + + // Populate more expensive fields prior to reconciling. + existingRecord = transformRecord(existingRecord); + Record toStore = reconcileRecords(record, existingRecord, lastRemoteRetrieval, lastLocalRetrieval); + + if (toStore == null) { + Log.d(LOG_TAG, "Reconciling returned null. Not inserting a record."); + return; + } + + // TODO: pass in timestamps? + Log.d(LOG_TAG, "Replacing " + existingRecord.guid + " with record " + toStore.guid); + Record replaced = replace(toStore, existingRecord); + + // Note that we don't track records here; deciding that is the job + // of reconcileRecords. + Log.d(LOG_TAG, "Calling delegate callback with guid " + replaced.guid + + "(" + replaced.androidID + ")"); + delegate.onRecordStoreSucceeded(replaced); + return; + } catch (MultipleRecordsForGuidException e) { Log.e(LOG_TAG, "Multiple records returned for given guid: " + record.guid); delegate.onRecordStoreFailed(e); @@ -431,17 +507,38 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession delegate.onRecordStoreFailed(e); return; } - - // Invoke callback with result. - delegate.onRecordStoreSucceeded(record); } }; storeWorkQueue.execute(command); } - - protected long insert(Record record) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { - putRecordToGuidMap(buildRecordString(record), record.guid); - return RepoUtils.getAndroidIdFromUri(dbHelper.insert(record)); + + protected void storeRecordDeletion(final Record record) { + // TODO: we ought to mark the record as deleted rather than deleting it, + // in order to support syncing to multiple destinations. Bug 722607. + dbHelper.delete(record); // TODO: mm? + delegate.onRecordStoreSucceeded(record); + } + + protected Record insert(Record record) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + Record toStore = prepareRecord(record); + Uri recordURI = dbHelper.insert(toStore); + long id = RepoUtils.getAndroidIdFromUri(recordURI); + Log.d(LOG_TAG, "Inserted as " + id); + + toStore.androidID = id; + updateBookkeeping(toStore); + Log.d(LOG_TAG, "insert() returning record " + toStore.guid); + return toStore; + } + + protected Record replace(Record newRecord, Record existingRecord) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + Record toStore = prepareRecord(newRecord); + + // newRecord should already have suitable androidID and guid. + dbHelper.update(existingRecord.guid, toStore); + updateBookkeeping(toStore); + Log.d(LOG_TAG, "replace() returning record " + toStore.guid); + return toStore; } protected Record recordForGUID(String guid) throws @@ -470,21 +567,23 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession } } - // Check if record already exists locally. + /** + * Attempt to find an equivalent record through some means other than GUID. + * + * @param record + * The record for which to search. + * @return + * An equivalent Record object, or null if none is found. + * + * @throws MultipleRecordsForGuidException + * @throws NoGuidForIdException + * @throws NullCursorException + * @throws ParentNotFoundException + */ protected Record findExistingRecord(Record record) throws MultipleRecordsForGuidException, NoGuidForIdException, NullCursorException, ParentNotFoundException { - Log.d(LOG_TAG, "Finding existing record for GUID " + record.guid); - Record r = recordForGUID(record.guid); - - // One result. (Multiple throws an exception.) - if (r != null) { - Log.d(LOG_TAG, "Found one by GUID."); - return r; - } - - // Empty result. - // Check to see if record exists but with a different guid. + Log.d(LOG_TAG, "Finding existing record for incoming record with GUID " + record.guid); String recordString = buildRecordString(record); Log.d(LOG_TAG, "Searching with record string " + recordString); String guid = getRecordToGuidMap().get(recordString); @@ -504,6 +603,7 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession } private void createRecordToGuidMap() throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + Utils.info(LOG_TAG, "BEGIN: creating record -> GUID map."); recordToGuid = new HashMap(); Cursor cur = dbHelper.fetchAll(); try { @@ -520,33 +620,21 @@ public abstract class AndroidBrowserRepositorySession extends RepositorySession } finally { cur.close(); } + Utils.info(LOG_TAG, "END: creating record -> GUID map."); } - public void putRecordToGuidMap(String guid, String recordString) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { + public void putRecordToGuidMap(String recordString, String guid) throws NoGuidForIdException, NullCursorException, ParentNotFoundException { if (recordToGuid == null) { createRecordToGuidMap(); } - recordToGuid.put(guid, recordString); + recordToGuid.put(recordString, guid); } - protected Record reconcileRecords(Record local, Record remote) { - Log.i(LOG_TAG, "Reconciling " + local.guid + " against " + remote.guid); - - // Determine which record is newer since this is the one we will take in case of conflict. - // Yes, clock drift. *sigh* - Record newer; - if (local.lastModified > remote.lastModified) { - newer = local; - } else { - newer = remote; - } - - if (newer.guid != remote.guid) { - newer.guid = remote.guid; - } - newer.androidID = local.androidID; - - return newer; + protected abstract Record prepareRecord(Record record); + protected void updateBookkeeping(Record record) throws NoGuidForIdException, + NullCursorException, + ParentNotFoundException { + putRecordToGuidMap(buildRecordString(record), record.guid); } // Wipe method and thread. diff --git a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java index bedbe6cc527..00d3fa4233c 100644 --- a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java +++ b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java @@ -224,9 +224,7 @@ public class BookmarkRecord extends Record { } private void trace(String s) { - if (Utils.ENABLE_TRACE_LOGGING) { - Log.d(LOG_TAG, s); - } + Utils.trace(LOG_TAG, s); } @Override diff --git a/mobile/android/base/sync/synchronizer/ConcurrentRecordConsumer.java b/mobile/android/base/sync/synchronizer/ConcurrentRecordConsumer.java index 0b8d7ba868e..0bc02316f94 100644 --- a/mobile/android/base/sync/synchronizer/ConcurrentRecordConsumer.java +++ b/mobile/android/base/sync/synchronizer/ConcurrentRecordConsumer.java @@ -65,21 +65,15 @@ class ConcurrentRecordConsumer extends RecordConsumer { } private static void info(String message) { - Utils.logToStdout(LOG_TAG, "::INFO: ", message); - Log.i(LOG_TAG, message); + Utils.info(LOG_TAG, message); } private static void debug(String message) { - Utils.logToStdout(LOG_TAG, ":: DEBUG: ", message); - Log.d(LOG_TAG, message); + Utils.debug(LOG_TAG, message); } private static void trace(String message) { - if (!Utils.ENABLE_TRACE_LOGGING) { - return; - } - Utils.logToStdout(LOG_TAG, ":: TRACE: ", message); - Log.d(LOG_TAG, message); + Utils.trace(LOG_TAG, message); } private Object monitor = new Object(); @@ -139,7 +133,13 @@ class ConcurrentRecordConsumer extends RecordConsumer { while (!delegate.getQueue().isEmpty()) { trace("Grabbing record..."); Record record = delegate.getQueue().remove(); - delegate.store(record); + trace("Storing record... " + delegate); + try { + delegate.store(record); + } catch (Exception e) { + // TODO: Bug 709371: track records that failed to apply. + Log.e(LOG_TAG, "Caught error in store.", e); + } trace("Done with record."); } synchronized (monitor) { diff --git a/mobile/android/sync/android-xml-resources.mn b/mobile/android/sync/android-xml-resources.mn deleted file mode 100644 index 73ac49ada82..00000000000 --- a/mobile/android/sync/android-xml-resources.mn +++ /dev/null @@ -1,2 +0,0 @@ -mobile/android/base/resources/xml/sync_authenticator.xml -mobile/android/base/resources/xml/sync_syncadapter.xml diff --git a/mobile/android/sync/java-sources.mn b/mobile/android/sync/java-sources.mn index 9651087230b..929c08f3060 100644 --- a/mobile/android/sync/java-sources.mn +++ b/mobile/android/sync/java-sources.mn @@ -1 +1 @@ -sync/AlreadySyncingException.java sync/CollectionKeys.java sync/CredentialsSource.java sync/crypto/CryptoException.java sync/crypto/Cryptographer.java sync/crypto/CryptoInfo.java sync/crypto/HKDF.java sync/crypto/HMACVerificationException.java sync/crypto/KeyBundle.java sync/crypto/MissingCryptoInputException.java sync/crypto/NoKeyBundleException.java sync/cryptographer/CryptoStatusBundle.java sync/cryptographer/SyncCryptographer.java sync/CryptoRecord.java sync/DelayedWorkTracker.java sync/delegates/FreshStartDelegate.java sync/delegates/GlobalSessionCallback.java sync/delegates/InfoCollectionsDelegate.java sync/delegates/KeyUploadDelegate.java sync/delegates/MetaGlobalDelegate.java sync/delegates/WipeServerDelegate.java sync/ExtendedJSONObject.java sync/GlobalSession.java sync/HTTPFailureException.java sync/InfoCollections.java sync/jpake/BigIntegerHelper.java sync/jpake/Gx4IsOneException.java sync/jpake/IncorrectZkpException.java sync/jpake/JPakeClient.java sync/jpake/JPakeCrypto.java sync/jpake/JPakeNoActivePairingException.java sync/jpake/JPakeNumGenerator.java sync/jpake/JPakeNumGeneratorRandom.java sync/jpake/JPakeParty.java sync/jpake/JPakeRequest.java sync/jpake/JPakeRequestDelegate.java sync/jpake/JPakeResponse.java sync/jpake/JPakeUtils.java sync/jpake/Zkp.java sync/MetaGlobal.java sync/MetaGlobalException.java sync/MetaGlobalMissingEnginesException.java sync/MetaGlobalNotSetException.java sync/middleware/Crypto5MiddlewareRepository.java sync/middleware/Crypto5MiddlewareRepositorySession.java sync/middleware/MiddlewareRepository.java sync/net/BaseResource.java sync/net/CompletedEntity.java sync/net/HandleProgressException.java sync/net/Resource.java sync/net/ResourceDelegate.java sync/net/SyncResourceDelegate.java sync/net/SyncResponse.java sync/net/SyncStorageCollectionRequest.java sync/net/SyncStorageCollectionRequestDelegate.java sync/net/SyncStorageRecordRequest.java sync/net/SyncStorageRequest.java sync/net/SyncStorageRequestDelegate.java sync/net/SyncStorageRequestIncrementalDelegate.java sync/net/SyncStorageResponse.java sync/net/TLSSocketFactory.java sync/net/WBOCollectionRequestDelegate.java sync/NoCollectionKeysSetException.java sync/NonArrayJSONException.java sync/NonObjectJSONException.java sync/PrefsSource.java sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java sync/repositories/android/AndroidBrowserBookmarksRepository.java sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java sync/repositories/android/AndroidBrowserHistoryDataAccessor.java sync/repositories/android/AndroidBrowserHistoryDataExtender.java sync/repositories/android/AndroidBrowserHistoryRepository.java sync/repositories/android/AndroidBrowserHistoryRepositorySession.java sync/repositories/android/AndroidBrowserPasswordsDataAccessor.java sync/repositories/android/AndroidBrowserPasswordsRepository.java sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java sync/repositories/android/AndroidBrowserRepository.java sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java sync/repositories/android/AndroidBrowserRepositorySession.java sync/repositories/android/BrowserContract.java sync/repositories/android/PasswordColumns.java sync/repositories/android/RepoUtils.java sync/repositories/BookmarkNeedsReparentingException.java sync/repositories/BookmarksRepository.java sync/repositories/delegates/DeferrableRepositorySessionCreationDelegate.java sync/repositories/delegates/DeferredRepositorySessionBeginDelegate.java sync/repositories/delegates/DeferredRepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/DeferredRepositorySessionFinishDelegate.java sync/repositories/delegates/DeferredRepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionBeginDelegate.java sync/repositories/delegates/RepositorySessionCleanDelegate.java sync/repositories/delegates/RepositorySessionCreationDelegate.java sync/repositories/delegates/RepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/RepositorySessionFinishDelegate.java sync/repositories/delegates/RepositorySessionGuidsSinceDelegate.java sync/repositories/delegates/RepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionWipeDelegate.java sync/repositories/domain/BookmarkRecord.java sync/repositories/domain/BookmarkRecordFactory.java sync/repositories/domain/HistoryRecord.java sync/repositories/domain/HistoryRecordFactory.java sync/repositories/domain/PasswordRecord.java sync/repositories/domain/Record.java sync/repositories/HistoryRepository.java sync/repositories/IdentityRecordFactory.java sync/repositories/InactiveSessionException.java sync/repositories/InvalidBookmarkTypeException.java sync/repositories/InvalidRequestException.java sync/repositories/InvalidSessionTransitionException.java sync/repositories/MultipleRecordsForGuidException.java sync/repositories/NoGuidForIdException.java sync/repositories/NoStoreDelegateException.java sync/repositories/NullCursorException.java sync/repositories/ParentNotFoundException.java sync/repositories/ProfileDatabaseException.java sync/repositories/RecordFactory.java sync/repositories/Repository.java sync/repositories/RepositorySession.java sync/repositories/RepositorySessionBundle.java sync/repositories/Server11Repository.java sync/repositories/Server11RepositorySession.java sync/setup/activities/AccountActivity.java sync/setup/activities/SetupFailureActivity.java sync/setup/activities/SetupSuccessActivity.java sync/setup/activities/SetupSyncActivity.java sync/setup/Constants.java sync/setup/SyncAuthenticatorService.java sync/stage/AndroidBrowserBookmarksServerSyncStage.java sync/stage/AndroidBrowserHistoryServerSyncStage.java sync/stage/CheckPreconditionsStage.java sync/stage/CompletedStage.java sync/stage/EnsureClusterURLStage.java sync/stage/EnsureKeysStage.java sync/stage/FetchInfoCollectionsStage.java sync/stage/FetchMetaGlobalStage.java sync/stage/GlobalSyncStage.java sync/stage/NoSuchStageException.java sync/stage/NoSyncIDException.java sync/stage/ServerSyncStage.java sync/StubActivity.java sync/syncadapter/SyncAdapter.java sync/syncadapter/SyncService.java sync/SyncConfiguration.java sync/SyncConfigurationException.java sync/SyncException.java sync/synchronizer/ConcurrentRecordConsumer.java sync/synchronizer/RecordConsumer.java sync/synchronizer/RecordsChannel.java sync/synchronizer/RecordsChannelDelegate.java sync/synchronizer/RecordsConsumerDelegate.java sync/synchronizer/SerialRecordConsumer.java sync/synchronizer/SessionNotBegunException.java sync/synchronizer/Synchronizer.java sync/synchronizer/SynchronizerDelegate.java sync/synchronizer/SynchronizerSession.java sync/synchronizer/SynchronizerSessionDelegate.java sync/synchronizer/UnbundleError.java sync/synchronizer/UnexpectedSessionException.java sync/SynchronizerConfiguration.java sync/SynchronizerConfigurations.java sync/ThreadPool.java sync/UnexpectedJSONException.java sync/UnknownSynchronizerConfigurationVersionException.java sync/Utils.java +sync/AlreadySyncingException.java sync/CollectionKeys.java sync/CredentialsSource.java sync/crypto/CryptoException.java sync/crypto/Cryptographer.java sync/crypto/CryptoInfo.java sync/crypto/HKDF.java sync/crypto/HMACVerificationException.java sync/crypto/KeyBundle.java sync/crypto/MissingCryptoInputException.java sync/crypto/NoKeyBundleException.java sync/cryptographer/CryptoStatusBundle.java sync/cryptographer/SyncCryptographer.java sync/CryptoRecord.java sync/DelayedWorkTracker.java sync/delegates/FreshStartDelegate.java sync/delegates/GlobalSessionCallback.java sync/delegates/InfoCollectionsDelegate.java sync/delegates/KeyUploadDelegate.java sync/delegates/MetaGlobalDelegate.java sync/delegates/WipeServerDelegate.java sync/ExtendedJSONObject.java sync/GlobalSession.java sync/HTTPFailureException.java sync/InfoCollections.java sync/jpake/BigIntegerHelper.java sync/jpake/Gx4IsOneException.java sync/jpake/IncorrectZkpException.java sync/jpake/JPakeClient.java sync/jpake/JPakeCrypto.java sync/jpake/JPakeNoActivePairingException.java sync/jpake/JPakeNumGenerator.java sync/jpake/JPakeNumGeneratorRandom.java sync/jpake/JPakeParty.java sync/jpake/JPakeRequest.java sync/jpake/JPakeRequestDelegate.java sync/jpake/JPakeResponse.java sync/jpake/JPakeUtils.java sync/jpake/Zkp.java sync/MetaGlobal.java sync/MetaGlobalException.java sync/MetaGlobalMissingEnginesException.java sync/MetaGlobalNotSetException.java sync/middleware/Crypto5MiddlewareRepository.java sync/middleware/Crypto5MiddlewareRepositorySession.java sync/middleware/MiddlewareRepository.java sync/net/BaseResource.java sync/net/CompletedEntity.java sync/net/HandleProgressException.java sync/net/Resource.java sync/net/ResourceDelegate.java sync/net/SyncResourceDelegate.java sync/net/SyncResponse.java sync/net/SyncStorageCollectionRequest.java sync/net/SyncStorageCollectionRequestDelegate.java sync/net/SyncStorageRecordRequest.java sync/net/SyncStorageRequest.java sync/net/SyncStorageRequestDelegate.java sync/net/SyncStorageRequestIncrementalDelegate.java sync/net/SyncStorageResponse.java sync/net/TLSSocketFactory.java sync/net/WBOCollectionRequestDelegate.java sync/NoCollectionKeysSetException.java sync/NonArrayJSONException.java sync/NonObjectJSONException.java sync/PrefsSource.java sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java sync/repositories/android/AndroidBrowserBookmarksRepository.java sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java sync/repositories/android/AndroidBrowserHistoryDataAccessor.java sync/repositories/android/AndroidBrowserHistoryDataExtender.java sync/repositories/android/AndroidBrowserHistoryRepository.java sync/repositories/android/AndroidBrowserHistoryRepositorySession.java sync/repositories/android/AndroidBrowserPasswordsDataAccessor.java sync/repositories/android/AndroidBrowserPasswordsRepository.java sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java sync/repositories/android/AndroidBrowserRepository.java sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java sync/repositories/android/AndroidBrowserRepositorySession.java sync/repositories/android/BrowserContract.java sync/repositories/android/PasswordColumns.java sync/repositories/android/RepoUtils.java sync/repositories/BookmarkNeedsReparentingException.java sync/repositories/BookmarksRepository.java sync/repositories/delegates/DeferrableRepositorySessionCreationDelegate.java sync/repositories/delegates/DeferredRepositorySessionBeginDelegate.java sync/repositories/delegates/DeferredRepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/DeferredRepositorySessionFinishDelegate.java sync/repositories/delegates/DeferredRepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionBeginDelegate.java sync/repositories/delegates/RepositorySessionCleanDelegate.java sync/repositories/delegates/RepositorySessionCreationDelegate.java sync/repositories/delegates/RepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/RepositorySessionFinishDelegate.java sync/repositories/delegates/RepositorySessionGuidsSinceDelegate.java sync/repositories/delegates/RepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionWipeDelegate.java sync/repositories/domain/BookmarkRecord.java sync/repositories/domain/BookmarkRecordFactory.java sync/repositories/domain/HistoryRecord.java sync/repositories/domain/HistoryRecordFactory.java sync/repositories/domain/PasswordRecord.java sync/repositories/domain/Record.java sync/repositories/HashSetStoreTracker.java sync/repositories/HistoryRepository.java sync/repositories/IdentityRecordFactory.java sync/repositories/InactiveSessionException.java sync/repositories/InvalidBookmarkTypeException.java sync/repositories/InvalidRequestException.java sync/repositories/InvalidSessionTransitionException.java sync/repositories/MultipleRecordsForGuidException.java sync/repositories/NoGuidForIdException.java sync/repositories/NoStoreDelegateException.java sync/repositories/NullCursorException.java sync/repositories/ParentNotFoundException.java sync/repositories/ProfileDatabaseException.java sync/repositories/RecordFactory.java sync/repositories/RecordFilter.java sync/repositories/Repository.java sync/repositories/RepositorySession.java sync/repositories/RepositorySessionBundle.java sync/repositories/Server11Repository.java sync/repositories/Server11RepositorySession.java sync/repositories/StoreTracker.java sync/repositories/StoreTrackingRepositorySession.java sync/setup/activities/AccountActivity.java sync/setup/activities/SetupFailureActivity.java sync/setup/activities/SetupSuccessActivity.java sync/setup/activities/SetupSyncActivity.java sync/setup/Constants.java sync/setup/SyncAuthenticatorService.java sync/stage/AndroidBrowserBookmarksServerSyncStage.java sync/stage/AndroidBrowserHistoryServerSyncStage.java sync/stage/CheckPreconditionsStage.java sync/stage/CompletedStage.java sync/stage/EnsureClusterURLStage.java sync/stage/EnsureKeysStage.java sync/stage/FetchInfoCollectionsStage.java sync/stage/FetchMetaGlobalStage.java sync/stage/GlobalSyncStage.java sync/stage/NoSuchStageException.java sync/stage/NoSyncIDException.java sync/stage/ServerSyncStage.java sync/StubActivity.java sync/syncadapter/SyncAdapter.java sync/syncadapter/SyncService.java sync/SyncConfiguration.java sync/SyncConfigurationException.java sync/SyncException.java sync/synchronizer/ConcurrentRecordConsumer.java sync/synchronizer/RecordConsumer.java sync/synchronizer/RecordsChannel.java sync/synchronizer/RecordsChannelDelegate.java sync/synchronizer/RecordsConsumerDelegate.java sync/synchronizer/SerialRecordConsumer.java sync/synchronizer/SessionNotBegunException.java sync/synchronizer/Synchronizer.java sync/synchronizer/SynchronizerDelegate.java sync/synchronizer/SynchronizerSession.java sync/synchronizer/SynchronizerSessionDelegate.java sync/synchronizer/UnbundleError.java sync/synchronizer/UnexpectedSessionException.java sync/SynchronizerConfiguration.java sync/SynchronizerConfigurations.java sync/ThreadPool.java sync/UnexpectedJSONException.java sync/UnknownSynchronizerConfigurationVersionException.java sync/Utils.java From f77bb49caf60d8d894eeb9d16c255ebeed52464f Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:29 -0800 Subject: [PATCH 13/16] Bug 723235, Bug 720304, Bug 723240 - Request bookmarks and history items sorted by sortindex/frecency, with limits. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Bug 720304 - Limit number of processed history records, with a hard limit of 500 history items per request. • Bug 723235 - Request history items sorted by sortindex/frecency. • Bug 723240 - Request bookmark items sorted by sortindex/frecency. And imposes a sanity limit of 5,000 bookmarks per fetch. (Note that batching is outside the scope of these bugs.) --- .../ConstrainedServer11Repository.java | 38 ++++++++++++ .../sync/repositories/Server11Repository.java | 61 ++++++++++++------- .../Server11RepositorySession.java | 21 +++++-- ...ndroidBrowserBookmarksServerSyncStage.java | 19 ++++++ .../AndroidBrowserHistoryServerSyncStage.java | 19 ++++++ .../base/sync/stage/ServerSyncStage.java | 14 +++-- mobile/android/sync/java-sources.mn | 2 +- 7 files changed, 143 insertions(+), 31 deletions(-) create mode 100644 mobile/android/base/sync/repositories/ConstrainedServer11Repository.java diff --git a/mobile/android/base/sync/repositories/ConstrainedServer11Repository.java b/mobile/android/base/sync/repositories/ConstrainedServer11Repository.java new file mode 100644 index 00000000000..69c3b3cd51b --- /dev/null +++ b/mobile/android/base/sync/repositories/ConstrainedServer11Repository.java @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.gecko.sync.repositories; + +import java.net.URISyntaxException; + +import org.mozilla.gecko.sync.CredentialsSource; + +/** + * A kind of Server11Repository that supports explicit setting of limit and sort on operations. + * + * @author rnewman + * + */ +public class ConstrainedServer11Repository extends Server11Repository { + + private String sort = null; + private long limit = -1; + + public ConstrainedServer11Repository(String serverURI, String username, String collection, CredentialsSource credentialsSource, long limit, String sort) throws URISyntaxException { + super(serverURI, username, collection, credentialsSource); + + this.limit = limit; + this.sort = sort; + } + + @Override + protected String getDefaultSort() { + return sort; + } + + @Override + protected long getDefaultFetchLimit() { + return limit; + } +} diff --git a/mobile/android/base/sync/repositories/Server11Repository.java b/mobile/android/base/sync/repositories/Server11Repository.java index fe723b7c582..141d71ac6a8 100644 --- a/mobile/android/base/sync/repositories/Server11Repository.java +++ b/mobile/android/base/sync/repositories/Server11Repository.java @@ -39,6 +39,7 @@ package org.mozilla.gecko.sync.repositories; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import org.mozilla.gecko.sync.CredentialsSource; import org.mozilla.gecko.sync.Utils; @@ -92,32 +93,50 @@ public class Server11Repository extends Repository { return this.collectionPathURI; } - public URI collectionURI(boolean full, long newer, String ids) throws URISyntaxException { - // Do it this way to make it easier to add more params later. - // It's pretty ugly, I'll grant. - // I can't believe Java doesn't have a good way to do this. - boolean anyParams = full; - String uriParams = ""; - if (anyParams) { - StringBuilder params = new StringBuilder("?"); - if (full) { - params.append("full=1"); - } - if (newer >= 0) { - // Translate local millisecond timestamps into server decimal seconds. - String newerString = Utils.millisecondsToDecimalSecondsString(newer); - params.append((full ? "&newer=" : "newer=") + newerString); - } - if (ids != null) { - params.append(((full || newer >= 0) ? "&ids=" : "ids=") + ids); - } - uriParams = params.toString(); + public URI collectionURI(boolean full, long newer, long limit, String sort, String ids) throws URISyntaxException { + ArrayList params = new ArrayList(); + if (full) { + params.add("full=1"); } - String uri = this.collectionPath + uriParams; + if (newer >= 0) { + // Translate local millisecond timestamps into server decimal seconds. + String newerString = Utils.millisecondsToDecimalSecondsString(newer); + params.add("newer=" + newerString); + } + if (limit > 0) { + params.add("limit=" + limit); + } + if (sort != null) { + params.add("sort=" + sort); // We trust these values. + } + if (ids != null) { + params.add("ids=" + ids); // We trust these values. + } + + if (params.size() == 0) { + return this.collectionPathURI; + } + + StringBuilder out = new StringBuilder(); + char indicator = '?'; + for (String param : params) { + out.append(indicator); + indicator = '&'; + out.append(param); + } + String uri = this.collectionPath + out.toString(); return new URI(uri); } public URI wboURI(String id) throws URISyntaxException { return new URI(this.collectionPath + "/" + id); } + + // Override these. + protected long getDefaultFetchLimit() { + return -1; + } + protected String getDefaultSort() { + return null; + } } diff --git a/mobile/android/base/sync/repositories/Server11RepositorySession.java b/mobile/android/base/sync/repositories/Server11RepositorySession.java index ddc4bea62e8..e92a354bb47 100644 --- a/mobile/android/base/sync/repositories/Server11RepositorySession.java +++ b/mobile/android/base/sync/repositories/Server11RepositorySession.java @@ -214,21 +214,34 @@ public class Server11RepositorySession extends RepositorySession { } protected void fetchWithParameters(long newer, + long limit, boolean full, + String sort, String ids, - SyncStorageRequestDelegate delegate) throws URISyntaxException { + SyncStorageRequestDelegate delegate) + throws URISyntaxException { - URI collectionURI = serverRepository.collectionURI(full, newer, ids); + URI collectionURI = serverRepository.collectionURI(full, newer, limit, sort, ids); SyncStorageCollectionRequest request = new SyncStorageCollectionRequest(collectionURI); request.delegate = delegate; request.get(); } + public void fetchSince(long timestamp, long limit, String sort, RepositorySessionFetchRecordsDelegate delegate) { + try { + this.fetchWithParameters(timestamp, limit, true, sort, null, new RequestFetchDelegateAdapter(delegate)); + } catch (URISyntaxException e) { + delegate.onFetchFailed(e, null); + } + } + @Override public void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) { try { - this.fetchWithParameters(timestamp, true, null, new RequestFetchDelegateAdapter(delegate)); + long limit = serverRepository.getDefaultFetchLimit(); + String sort = serverRepository.getDefaultSort(); + this.fetchWithParameters(timestamp, limit, true, sort, null, new RequestFetchDelegateAdapter(delegate)); } catch (URISyntaxException e) { delegate.onFetchFailed(e, null); } @@ -245,7 +258,7 @@ public class Server11RepositorySession extends RepositorySession { // TODO: watch out for URL length limits! try { String ids = flattenIDs(guids); - this.fetchWithParameters(-1, true, ids, new RequestFetchDelegateAdapter(delegate)); + this.fetchWithParameters(-1, -1, true, "index", ids, new RequestFetchDelegateAdapter(delegate)); } catch (URISyntaxException e) { delegate.onFetchFailed(e, null); } diff --git a/mobile/android/base/sync/stage/AndroidBrowserBookmarksServerSyncStage.java b/mobile/android/base/sync/stage/AndroidBrowserBookmarksServerSyncStage.java index d5be4ddf192..4ab79d634c7 100644 --- a/mobile/android/base/sync/stage/AndroidBrowserBookmarksServerSyncStage.java +++ b/mobile/android/base/sync/stage/AndroidBrowserBookmarksServerSyncStage.java @@ -37,12 +37,21 @@ package org.mozilla.gecko.sync.stage; +import java.net.URISyntaxException; + +import org.mozilla.gecko.sync.repositories.ConstrainedServer11Repository; import org.mozilla.gecko.sync.repositories.RecordFactory; import org.mozilla.gecko.sync.repositories.Repository; import org.mozilla.gecko.sync.repositories.android.AndroidBrowserBookmarksRepository; import org.mozilla.gecko.sync.repositories.domain.BookmarkRecordFactory; public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage { + + // Eventually this kind of sync stage will be data-driven, + // and all this hard-coding can go away. + private static final String BOOKMARKS_SORT = "index"; + private static final long BOOKMARKS_REQUEST_LIMIT = 5000; // Sanity limit. + @Override public void execute(org.mozilla.gecko.sync.GlobalSession session) throws NoSuchStageException { super.execute(session); @@ -57,6 +66,16 @@ public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage { return "bookmarks"; } + @Override + protected Repository getRemoteRepository() throws URISyntaxException { + return new ConstrainedServer11Repository(session.config.getClusterURLString(), + session.config.username, + getCollection(), + session, + BOOKMARKS_REQUEST_LIMIT, + BOOKMARKS_SORT); + } + @Override protected Repository getLocalRepository() { return new AndroidBrowserBookmarksRepository(); diff --git a/mobile/android/base/sync/stage/AndroidBrowserHistoryServerSyncStage.java b/mobile/android/base/sync/stage/AndroidBrowserHistoryServerSyncStage.java index 0da9fdcb328..d745591df1b 100644 --- a/mobile/android/base/sync/stage/AndroidBrowserHistoryServerSyncStage.java +++ b/mobile/android/base/sync/stage/AndroidBrowserHistoryServerSyncStage.java @@ -37,12 +37,21 @@ package org.mozilla.gecko.sync.stage; +import java.net.URISyntaxException; + +import org.mozilla.gecko.sync.repositories.ConstrainedServer11Repository; import org.mozilla.gecko.sync.repositories.RecordFactory; import org.mozilla.gecko.sync.repositories.Repository; import org.mozilla.gecko.sync.repositories.android.AndroidBrowserHistoryRepository; import org.mozilla.gecko.sync.repositories.domain.HistoryRecordFactory; public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage { + + // Eventually this kind of sync stage will be data-driven, + // and all this hard-coding can go away. + private static final String HISTORY_SORT = "index"; + private static final long HISTORY_REQUEST_LIMIT = 500; + @Override public void execute(org.mozilla.gecko.sync.GlobalSession session) throws NoSuchStageException { super.execute(session); @@ -62,6 +71,16 @@ public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage { return new AndroidBrowserHistoryRepository(); } + @Override + protected Repository getRemoteRepository() throws URISyntaxException { + return new ConstrainedServer11Repository(session.config.getClusterURLString(), + session.config.username, + getCollection(), + session, + HISTORY_REQUEST_LIMIT, + HISTORY_SORT); + } + @Override protected RecordFactory getRecordFactory() { return new HistoryRecordFactory(); diff --git a/mobile/android/base/sync/stage/ServerSyncStage.java b/mobile/android/base/sync/stage/ServerSyncStage.java index 0d93887aa8a..c591b00e2d6 100644 --- a/mobile/android/base/sync/stage/ServerSyncStage.java +++ b/mobile/android/base/sync/stage/ServerSyncStage.java @@ -84,6 +84,14 @@ public abstract class ServerSyncStage implements protected abstract Repository getLocalRepository(); protected abstract RecordFactory getRecordFactory(); + // Override this in subclasses. + protected Repository getRemoteRepository() throws URISyntaxException { + return new Server11Repository(session.config.getClusterURLString(), + session.config.username, + getCollection(), + session); + } + /** * Return a Crypto5Middleware-wrapped Server11Repository. * @@ -97,11 +105,7 @@ public abstract class ServerSyncStage implements protected Repository wrappedServerRepo() throws NoCollectionKeysSetException, URISyntaxException { String collection = this.getCollection(); KeyBundle collectionKey = session.keyForCollection(collection); - Server11Repository serverRepo = new Server11Repository(session.config.getClusterURLString(), - session.config.username, - collection, - session); - Crypto5MiddlewareRepository cryptoRepo = new Crypto5MiddlewareRepository(serverRepo, collectionKey); + Crypto5MiddlewareRepository cryptoRepo = new Crypto5MiddlewareRepository(getRemoteRepository(), collectionKey); cryptoRepo.recordFactory = getRecordFactory(); return cryptoRepo; } diff --git a/mobile/android/sync/java-sources.mn b/mobile/android/sync/java-sources.mn index 929c08f3060..0511919e52a 100644 --- a/mobile/android/sync/java-sources.mn +++ b/mobile/android/sync/java-sources.mn @@ -1 +1 @@ -sync/AlreadySyncingException.java sync/CollectionKeys.java sync/CredentialsSource.java sync/crypto/CryptoException.java sync/crypto/Cryptographer.java sync/crypto/CryptoInfo.java sync/crypto/HKDF.java sync/crypto/HMACVerificationException.java sync/crypto/KeyBundle.java sync/crypto/MissingCryptoInputException.java sync/crypto/NoKeyBundleException.java sync/cryptographer/CryptoStatusBundle.java sync/cryptographer/SyncCryptographer.java sync/CryptoRecord.java sync/DelayedWorkTracker.java sync/delegates/FreshStartDelegate.java sync/delegates/GlobalSessionCallback.java sync/delegates/InfoCollectionsDelegate.java sync/delegates/KeyUploadDelegate.java sync/delegates/MetaGlobalDelegate.java sync/delegates/WipeServerDelegate.java sync/ExtendedJSONObject.java sync/GlobalSession.java sync/HTTPFailureException.java sync/InfoCollections.java sync/jpake/BigIntegerHelper.java sync/jpake/Gx4IsOneException.java sync/jpake/IncorrectZkpException.java sync/jpake/JPakeClient.java sync/jpake/JPakeCrypto.java sync/jpake/JPakeNoActivePairingException.java sync/jpake/JPakeNumGenerator.java sync/jpake/JPakeNumGeneratorRandom.java sync/jpake/JPakeParty.java sync/jpake/JPakeRequest.java sync/jpake/JPakeRequestDelegate.java sync/jpake/JPakeResponse.java sync/jpake/JPakeUtils.java sync/jpake/Zkp.java sync/MetaGlobal.java sync/MetaGlobalException.java sync/MetaGlobalMissingEnginesException.java sync/MetaGlobalNotSetException.java sync/middleware/Crypto5MiddlewareRepository.java sync/middleware/Crypto5MiddlewareRepositorySession.java sync/middleware/MiddlewareRepository.java sync/net/BaseResource.java sync/net/CompletedEntity.java sync/net/HandleProgressException.java sync/net/Resource.java sync/net/ResourceDelegate.java sync/net/SyncResourceDelegate.java sync/net/SyncResponse.java sync/net/SyncStorageCollectionRequest.java sync/net/SyncStorageCollectionRequestDelegate.java sync/net/SyncStorageRecordRequest.java sync/net/SyncStorageRequest.java sync/net/SyncStorageRequestDelegate.java sync/net/SyncStorageRequestIncrementalDelegate.java sync/net/SyncStorageResponse.java sync/net/TLSSocketFactory.java sync/net/WBOCollectionRequestDelegate.java sync/NoCollectionKeysSetException.java sync/NonArrayJSONException.java sync/NonObjectJSONException.java sync/PrefsSource.java sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java sync/repositories/android/AndroidBrowserBookmarksRepository.java sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java sync/repositories/android/AndroidBrowserHistoryDataAccessor.java sync/repositories/android/AndroidBrowserHistoryDataExtender.java sync/repositories/android/AndroidBrowserHistoryRepository.java sync/repositories/android/AndroidBrowserHistoryRepositorySession.java sync/repositories/android/AndroidBrowserPasswordsDataAccessor.java sync/repositories/android/AndroidBrowserPasswordsRepository.java sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java sync/repositories/android/AndroidBrowserRepository.java sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java sync/repositories/android/AndroidBrowserRepositorySession.java sync/repositories/android/BrowserContract.java sync/repositories/android/PasswordColumns.java sync/repositories/android/RepoUtils.java sync/repositories/BookmarkNeedsReparentingException.java sync/repositories/BookmarksRepository.java sync/repositories/delegates/DeferrableRepositorySessionCreationDelegate.java sync/repositories/delegates/DeferredRepositorySessionBeginDelegate.java sync/repositories/delegates/DeferredRepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/DeferredRepositorySessionFinishDelegate.java sync/repositories/delegates/DeferredRepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionBeginDelegate.java sync/repositories/delegates/RepositorySessionCleanDelegate.java sync/repositories/delegates/RepositorySessionCreationDelegate.java sync/repositories/delegates/RepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/RepositorySessionFinishDelegate.java sync/repositories/delegates/RepositorySessionGuidsSinceDelegate.java sync/repositories/delegates/RepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionWipeDelegate.java sync/repositories/domain/BookmarkRecord.java sync/repositories/domain/BookmarkRecordFactory.java sync/repositories/domain/HistoryRecord.java sync/repositories/domain/HistoryRecordFactory.java sync/repositories/domain/PasswordRecord.java sync/repositories/domain/Record.java sync/repositories/HashSetStoreTracker.java sync/repositories/HistoryRepository.java sync/repositories/IdentityRecordFactory.java sync/repositories/InactiveSessionException.java sync/repositories/InvalidBookmarkTypeException.java sync/repositories/InvalidRequestException.java sync/repositories/InvalidSessionTransitionException.java sync/repositories/MultipleRecordsForGuidException.java sync/repositories/NoGuidForIdException.java sync/repositories/NoStoreDelegateException.java sync/repositories/NullCursorException.java sync/repositories/ParentNotFoundException.java sync/repositories/ProfileDatabaseException.java sync/repositories/RecordFactory.java sync/repositories/RecordFilter.java sync/repositories/Repository.java sync/repositories/RepositorySession.java sync/repositories/RepositorySessionBundle.java sync/repositories/Server11Repository.java sync/repositories/Server11RepositorySession.java sync/repositories/StoreTracker.java sync/repositories/StoreTrackingRepositorySession.java sync/setup/activities/AccountActivity.java sync/setup/activities/SetupFailureActivity.java sync/setup/activities/SetupSuccessActivity.java sync/setup/activities/SetupSyncActivity.java sync/setup/Constants.java sync/setup/SyncAuthenticatorService.java sync/stage/AndroidBrowserBookmarksServerSyncStage.java sync/stage/AndroidBrowserHistoryServerSyncStage.java sync/stage/CheckPreconditionsStage.java sync/stage/CompletedStage.java sync/stage/EnsureClusterURLStage.java sync/stage/EnsureKeysStage.java sync/stage/FetchInfoCollectionsStage.java sync/stage/FetchMetaGlobalStage.java sync/stage/GlobalSyncStage.java sync/stage/NoSuchStageException.java sync/stage/NoSyncIDException.java sync/stage/ServerSyncStage.java sync/StubActivity.java sync/syncadapter/SyncAdapter.java sync/syncadapter/SyncService.java sync/SyncConfiguration.java sync/SyncConfigurationException.java sync/SyncException.java sync/synchronizer/ConcurrentRecordConsumer.java sync/synchronizer/RecordConsumer.java sync/synchronizer/RecordsChannel.java sync/synchronizer/RecordsChannelDelegate.java sync/synchronizer/RecordsConsumerDelegate.java sync/synchronizer/SerialRecordConsumer.java sync/synchronizer/SessionNotBegunException.java sync/synchronizer/Synchronizer.java sync/synchronizer/SynchronizerDelegate.java sync/synchronizer/SynchronizerSession.java sync/synchronizer/SynchronizerSessionDelegate.java sync/synchronizer/UnbundleError.java sync/synchronizer/UnexpectedSessionException.java sync/SynchronizerConfiguration.java sync/SynchronizerConfigurations.java sync/ThreadPool.java sync/UnexpectedJSONException.java sync/UnknownSynchronizerConfigurationVersionException.java sync/Utils.java +sync/AlreadySyncingException.java sync/CollectionKeys.java sync/CredentialsSource.java sync/crypto/CryptoException.java sync/crypto/Cryptographer.java sync/crypto/CryptoInfo.java sync/crypto/HKDF.java sync/crypto/HMACVerificationException.java sync/crypto/KeyBundle.java sync/crypto/MissingCryptoInputException.java sync/crypto/NoKeyBundleException.java sync/cryptographer/CryptoStatusBundle.java sync/cryptographer/SyncCryptographer.java sync/CryptoRecord.java sync/DelayedWorkTracker.java sync/delegates/FreshStartDelegate.java sync/delegates/GlobalSessionCallback.java sync/delegates/InfoCollectionsDelegate.java sync/delegates/KeyUploadDelegate.java sync/delegates/MetaGlobalDelegate.java sync/delegates/WipeServerDelegate.java sync/ExtendedJSONObject.java sync/GlobalSession.java sync/HTTPFailureException.java sync/InfoCollections.java sync/jpake/BigIntegerHelper.java sync/jpake/Gx4IsOneException.java sync/jpake/IncorrectZkpException.java sync/jpake/JPakeClient.java sync/jpake/JPakeCrypto.java sync/jpake/JPakeNoActivePairingException.java sync/jpake/JPakeNumGenerator.java sync/jpake/JPakeNumGeneratorRandom.java sync/jpake/JPakeParty.java sync/jpake/JPakeRequest.java sync/jpake/JPakeRequestDelegate.java sync/jpake/JPakeResponse.java sync/jpake/JPakeUtils.java sync/jpake/Zkp.java sync/MetaGlobal.java sync/MetaGlobalException.java sync/MetaGlobalMissingEnginesException.java sync/MetaGlobalNotSetException.java sync/middleware/Crypto5MiddlewareRepository.java sync/middleware/Crypto5MiddlewareRepositorySession.java sync/middleware/MiddlewareRepository.java sync/net/BaseResource.java sync/net/CompletedEntity.java sync/net/HandleProgressException.java sync/net/Resource.java sync/net/ResourceDelegate.java sync/net/SyncResourceDelegate.java sync/net/SyncResponse.java sync/net/SyncStorageCollectionRequest.java sync/net/SyncStorageCollectionRequestDelegate.java sync/net/SyncStorageRecordRequest.java sync/net/SyncStorageRequest.java sync/net/SyncStorageRequestDelegate.java sync/net/SyncStorageRequestIncrementalDelegate.java sync/net/SyncStorageResponse.java sync/net/TLSSocketFactory.java sync/net/WBOCollectionRequestDelegate.java sync/NoCollectionKeysSetException.java sync/NonArrayJSONException.java sync/NonObjectJSONException.java sync/PrefsSource.java sync/repositories/android/AndroidBrowserBookmarksDataAccessor.java sync/repositories/android/AndroidBrowserBookmarksRepository.java sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java sync/repositories/android/AndroidBrowserHistoryDataAccessor.java sync/repositories/android/AndroidBrowserHistoryDataExtender.java sync/repositories/android/AndroidBrowserHistoryRepository.java sync/repositories/android/AndroidBrowserHistoryRepositorySession.java sync/repositories/android/AndroidBrowserPasswordsDataAccessor.java sync/repositories/android/AndroidBrowserPasswordsRepository.java sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java sync/repositories/android/AndroidBrowserRepository.java sync/repositories/android/AndroidBrowserRepositoryDataAccessor.java sync/repositories/android/AndroidBrowserRepositorySession.java sync/repositories/android/BrowserContract.java sync/repositories/android/PasswordColumns.java sync/repositories/android/RepoUtils.java sync/repositories/BookmarkNeedsReparentingException.java sync/repositories/BookmarksRepository.java sync/repositories/ConstrainedServer11Repository.java sync/repositories/delegates/DeferrableRepositorySessionCreationDelegate.java sync/repositories/delegates/DeferredRepositorySessionBeginDelegate.java sync/repositories/delegates/DeferredRepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/DeferredRepositorySessionFinishDelegate.java sync/repositories/delegates/DeferredRepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionBeginDelegate.java sync/repositories/delegates/RepositorySessionCleanDelegate.java sync/repositories/delegates/RepositorySessionCreationDelegate.java sync/repositories/delegates/RepositorySessionFetchRecordsDelegate.java sync/repositories/delegates/RepositorySessionFinishDelegate.java sync/repositories/delegates/RepositorySessionGuidsSinceDelegate.java sync/repositories/delegates/RepositorySessionStoreDelegate.java sync/repositories/delegates/RepositorySessionWipeDelegate.java sync/repositories/domain/BookmarkRecord.java sync/repositories/domain/BookmarkRecordFactory.java sync/repositories/domain/HistoryRecord.java sync/repositories/domain/HistoryRecordFactory.java sync/repositories/domain/PasswordRecord.java sync/repositories/domain/Record.java sync/repositories/HashSetStoreTracker.java sync/repositories/HistoryRepository.java sync/repositories/IdentityRecordFactory.java sync/repositories/InactiveSessionException.java sync/repositories/InvalidBookmarkTypeException.java sync/repositories/InvalidRequestException.java sync/repositories/InvalidSessionTransitionException.java sync/repositories/MultipleRecordsForGuidException.java sync/repositories/NoGuidForIdException.java sync/repositories/NoStoreDelegateException.java sync/repositories/NullCursorException.java sync/repositories/ParentNotFoundException.java sync/repositories/ProfileDatabaseException.java sync/repositories/RecordFactory.java sync/repositories/RecordFilter.java sync/repositories/Repository.java sync/repositories/RepositorySession.java sync/repositories/RepositorySessionBundle.java sync/repositories/Server11Repository.java sync/repositories/Server11RepositorySession.java sync/repositories/StoreTracker.java sync/repositories/StoreTrackingRepositorySession.java sync/setup/activities/AccountActivity.java sync/setup/activities/SetupFailureActivity.java sync/setup/activities/SetupSuccessActivity.java sync/setup/activities/SetupSyncActivity.java sync/setup/Constants.java sync/setup/SyncAuthenticatorService.java sync/stage/AndroidBrowserBookmarksServerSyncStage.java sync/stage/AndroidBrowserHistoryServerSyncStage.java sync/stage/CheckPreconditionsStage.java sync/stage/CompletedStage.java sync/stage/EnsureClusterURLStage.java sync/stage/EnsureKeysStage.java sync/stage/FetchInfoCollectionsStage.java sync/stage/FetchMetaGlobalStage.java sync/stage/GlobalSyncStage.java sync/stage/NoSuchStageException.java sync/stage/NoSyncIDException.java sync/stage/ServerSyncStage.java sync/StubActivity.java sync/syncadapter/SyncAdapter.java sync/syncadapter/SyncService.java sync/SyncConfiguration.java sync/SyncConfigurationException.java sync/SyncException.java sync/synchronizer/ConcurrentRecordConsumer.java sync/synchronizer/RecordConsumer.java sync/synchronizer/RecordsChannel.java sync/synchronizer/RecordsChannelDelegate.java sync/synchronizer/RecordsConsumerDelegate.java sync/synchronizer/SerialRecordConsumer.java sync/synchronizer/SessionNotBegunException.java sync/synchronizer/Synchronizer.java sync/synchronizer/SynchronizerDelegate.java sync/synchronizer/SynchronizerSession.java sync/synchronizer/SynchronizerSessionDelegate.java sync/synchronizer/UnbundleError.java sync/synchronizer/UnexpectedSessionException.java sync/SynchronizerConfiguration.java sync/SynchronizerConfigurations.java sync/ThreadPool.java sync/UnexpectedJSONException.java sync/UnknownSynchronizerConfigurationVersionException.java sync/Utils.java From 3cb6eb8661dbcf07e074f524551130b2e6a36a05 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Fri, 3 Feb 2012 13:09:29 -0800 Subject: [PATCH 14/16] Bug 718544, Bug 723992 - adjust Sync's permissions. --- .../sync/manifests/SyncAndroidManifest_permissions.xml.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in b/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in index a5dc0e353dd..7d8545c36e5 100644 --- a/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in +++ b/mobile/android/sync/manifests/SyncAndroidManifest_permissions.xml.in @@ -1,6 +1,6 @@ - + From 652e8966c33501e029024f44b3dc4dc65aeb4b1e Mon Sep 17 00:00:00 2001 From: Brad Lassey Date: Fri, 3 Feb 2012 16:16:48 -0500 Subject: [PATCH 15/16] backing out 394c3ef8a0dc because it breaks the xul build and doesn't fix the problem --- embedding/android/AndroidManifest.xml.in | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/embedding/android/AndroidManifest.xml.in b/embedding/android/AndroidManifest.xml.in index dbd64a30d9b..ec386218c19 100644 --- a/embedding/android/AndroidManifest.xml.in +++ b/embedding/android/AndroidManifest.xml.in @@ -17,8 +17,7 @@ + android:xlargeScreens="true" /> #endif From f318a0e25c0eb6a58b79b33092c60d5f28c4ba53 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Fri, 3 Feb 2012 16:18:46 -0500 Subject: [PATCH 16/16] Bug 723917 - NullPointerException when removing a menu item [r=sriram] --- mobile/android/base/GeckoApp.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index 70e69bf727f..2ff153c4deb 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -905,10 +905,11 @@ abstract public class GeckoApp ExtraMenuItem item = i.next(); if (item.id == id) { sExtraMenuItems.remove(item); + if (sMenu == null) + return; MenuItem menu = sMenu.findItem(id); if (menu != null) sMenu.removeItem(id); - return; } } } else if (event.equals("Toast:Show")) {