Bug 1392078 - More detailed strong assertions to help narrow down root cause r=rnewman
MozReview-Commit-ID: JSN9Q837u6R
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -2762,17 +2762,32 @@ public class BrowserProvider extends Sha
int changed = 0;
try {
for (String guid : syncVersionsForGuids.keySet()) {
final int syncVersion = syncVersionsForGuids.get(guid);
compiledStatement.clearBindings();
compiledStatement.bindLong(1, syncVersion); // NB: 1-based index.
compiledStatement.bindString(2, guid);
- changed += compiledStatement.executeUpdateDelete();
+
+ // We expect this to be 1.
+ final int didUpdate = compiledStatement.executeUpdateDelete();
+
+ // These strong assertions are here to help figure out root cause of Bug 1392078.
+ // This will throw if there are duplicate GUIDs present.
+ if (didUpdate > 1) {
+ throw new IllegalStateException("Modified more than a single GUID during syncVersion update");
+ }
+
+ // This will throw if the requested GUID is missing from the database.
+ if (didUpdate == 0) {
+ throw new IllegalStateException("Expected to modify syncVersion for a guid, but did not");
+ }
+
+ changed += didUpdate;
}
markWriteSuccessful(db);
} finally {
endWrite(db);
}
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/VersioningDelegateHelper.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/VersioningDelegateHelper.java
@@ -85,34 +85,49 @@ public class VersioningDelegateHelper {
public RepositorySessionStoreDelegate getStoreDelegate(RepositorySessionStoreDelegate delegate) {
return new VersionedStoreDelegate(delegate);
}
public void persistSyncVersions() {
// At this point we can be sure that all records in our guidsMap have been successfully
// uploaded. Move syncVersions forward for each GUID en masse.
final Bundle data = new Bundle();
- data.putSerializable(BrowserContract.METHOD_PARAM_DATA, this.localVersionsOfGuids);
+ final int versionMapSizeBeforeUpdate;
+
+ synchronized (this.localVersionsOfGuids) {
+ data.putSerializable(BrowserContract.METHOD_PARAM_DATA, this.localVersionsOfGuids);
+ versionMapSizeBeforeUpdate = this.localVersionsOfGuids.size();
+ }
final Bundle result = context.getContentResolver().call(
repositoryContentUri,
BrowserContract.METHOD_UPDATE_SYNC_VERSIONS,
repositoryContentUri.toString(),
data
);
if (result == null) {
throw new IllegalStateException("Expected to receive a result after decrementing change counters");
}
- int changed = (int) result.getSerializable(BrowserContract.METHOD_RESULT);
- if (changed != this.localVersionsOfGuids.size()) {
- // TODO perhaps not worth throwing, but this shouldn't happen either...
- throw new IllegalStateException("Decremented incorrect number of change counters");
+ final int changed = (int) result.getSerializable(BrowserContract.METHOD_RESULT);
+ final int versionMapSizeAfterUpdate = this.localVersionsOfGuids.size();
+ // None of these should happen, but something's clearly amiss. These strong assertions are
+ // here to help figure out root cause for Bug 1392078.
+ if (versionMapSizeBeforeUpdate != versionMapSizeAfterUpdate) {
+ throw new IllegalStateException("Version/guid map changed during syncVersion update");
+ }
+
+ if (changed < versionMapSizeBeforeUpdate) {
+ throw new IllegalStateException("Updated fewer sync versions than expected");
+ }
+
+ if (changed > versionMapSizeBeforeUpdate) {
+ throw new IllegalStateException("Updated more sync versions than expected");
}
}
private class VersionedFetchDelegate implements RepositorySessionFetchRecordsDelegate {
private final RepositorySessionFetchRecordsDelegate inner;
VersionedFetchDelegate(RepositorySessionFetchRecordsDelegate delegate) {
this.inner = delegate;