Comment from bugzilla on this ugly hack:
While processing bookmarks, we sometimes need to mark them for re-upload as we're inserting new ones or updating existing ones. For example, we might set or update a dateAdded field.
We perform insertions "in-bulk", and so we might be inserting some bookmarks which need to be re-uploaded, and some bookmarks which don't. We compile an array of ContentValue objects, and make a single call to our ContentProvider. This means we can't use a URI param to indicate our intent, and so a non-column field in ContentValues objects - modifiedFromSync - is set for those bookmarks which need special treatment during insertion.
Bug 1392802 added a similar mechanism for updating bookmarks. However, updates are done differently - currently, we perform a single call to our ContentProvider for each bookmark. Which means we _can_ use a URI field as a signaling mechanism, which is what that patch did. However, in haste I forgot to take into consideration existing signaling mechanism, which lead to update failures.
And so we're left with an even clumsier interface to our data store, with two ways to signal the same thing in different circumstances... A quick solution is to just make sure 'modifiedBySync' field never makes its way to contentprovider on updates; a more refined fix would probably modify update logic to use a ContentValues field for consistency... Either way, there's going to be something ugly, somewhere in the code.
I anticipate a lot of this code changing sometime soon in order to support better transactionality of bookmark syncing, and smarter merging, and so I'm inclined to just to the simple thing for now.
MozReview-Commit-ID: H10LFsqjbFY
--HG--
extra : rebase_source : f7f311d266d75c505bb8871a567ac96d39f1b1cb
This patch moves some of the state tracking (fetchEnd/storeEnd timestamps) away from RecordsChannel
and into individual RepositorySessions. The core assumption behind this move is that
sessions are better suited to know when they were fetched from during this sync, and when they
were stored to.
Sessions are growing in complexity - local ones are wrapped in a buffer, remote
now support batching downloads and uploads. In order to hide these details, it's easier to let
sessions keep track of the fetch/store timestamps in the way that fits their implementations.
Instead of flowing these timestamps upwards from sessions and into the SynchronizerSession,
the latter now simply queries sessions at the end of their flows.
The default behavior if a certain operation wasn't performed - that is, if fetchEnd or storeEnd
aren't set during sync for a session - is to return timestamp persisted during the previous sync.
This allows us to skip certain flows (no remote data available), and ensure that we're always
using correct timestamps of the same origin for any given session.
Prior behaviour was to "make up" a timestamp at the RecordsChannel level in cases of certain
errors or skipped flows, which resulted in comparing timestamps of different origins on the consequent sync.
MozReview-Commit-ID: 2wqeTo7mhz3
--HG--
extra : rebase_source : 21b02d4164abf75422920225749ffcfd3fc71e91
We might decide that there's an older dateAdded timestamp present for an incoming bookmark while processing it,
in which case we need to ensure that our changes will be uploaded.
MozReview-Commit-ID: BKLh4rYBiRu
--HG--
extra : rebase_source : 3f8ac41de99d7082cd9d7fc7254386d99d5431bd
This is meant as a stop-gap measure to stop the obviously bad thing from happening.
MozReview-Commit-ID: Gqvc32K04xD
--HG--
extra : rebase_source : 04f1b5cb7ead6b7949b8433b3fc75c0d67283315
Versioned syncing work later in the patch series introduces functionality that is best suited to live
within the RepositorySession inheritance chain.
We'd like to introduce a new RepositorySession subclass which individual RepositorySessions are able to
inherit. And that's when the current inheritance structure gets in the way: history and bookmarks both share
a superclass, and we'd like to only introduce this new functionality for bookmarks.
This makes our task, as stated, impossible without breaking apart the current inheritance structure.
This patch introduces a few "delegate" objects:
- SessionHelper
--> HistorySessionHelper
--> BookmarksSessionHelper
... which absorb most of the functionality from AndroidBrowserRepositorySession (removed) and bookmark and history
repository sessions.
This change is not functional - everything remains as before otherwise.
MozReview-Commit-ID: 7WwUmY3Wql7
--HG--
extra : rebase_source : a8cd49fd14cdc76b9e2906d4ee8c2052b9152413
As part of moving toward versioned syncing, we need to start decoupling change tracking
concepts from parts of the system that facilitate flow of records. This allows us to track
what changed differently for different data types, while maintaining a consistent and predictable API.
A move toward that is to let repositories own determinining that a record has been modified.
Repositories are now asked to provide modified records, instead of a very specific "records modified since".
This patch does not change behaviour of the system: every repository still uses timestamp-based
change tracking to actually provide modified records to the caller. A changeover to version
tracking will come later in this series for bookmarks, and as part of Bug 1383894 for other repositories.
MozReview-Commit-ID: LQuWYdlNHpt
--HG--
extra : rebase_source : 5552d74d4a967ce85af09aaa57ca438fe5b949f3
We're moving toward version-based syncing. This is one of the bricks in that road,
removing an unused timestamp-based interface for accessing changed records.
MozReview-Commit-ID: CYUyASWXrMW
--HG--
extra : rebase_source : b95687409bc5f3e8e21fb9b084efdcd14a975d01
Error reporting, and especially the split between per-stage and global session errors,
are a bit of a mess; at the very least, this patch unifies things a little bit, and
ensures we're not passing around nulls unexpectedly.
MozReview-Commit-ID: JTSp7GuOji0
--HG--
extra : rebase_source : 19bbd2c98776b32b803d7febb55549bc430cbc3e
TLS_DHE_RSA_WITH_AES_128_CBC_SHA is no longer supported in API26+.
MozReview-Commit-ID: AtNf2xZh2Bz
--HG--
extra : rebase_source : fef7d2018e77a4a4a7594bf32de750c8fa39e2ea
Remove all references to Build.SDK_INT comparing 14 and lower
MozReview-Commit-ID: JdAjYvQ6mfX
--HG--
extra : rebase_source : f6cae8af84c26f42dcc02c133e7bc702f1af61e6
Parent class (FxAccountSyncAdapter) is essentially a singleton, and so we'd end up re-using class
fields between syncs, among them the collected telemetry data. It's cleaner and safer to move
ownership of TelemetryCollector into IntrumentedSessionCallback. With this change, telemetry
data is contained within and eventually emitted from a single owner object.
MozReview-Commit-ID: Abx13VmILcE
--HG--
extra : rebase_source : b68b44951361727015c2a10895e42f6a34806b27
While this patch does make it clearer that telemetry error handling could be factored better,
at least it gets us to a consistent usage pattern.
MozReview-Commit-ID: 4Oamt9D03Ue
--HG--
extra : rebase_source : da73247ae0a27ba6ae3d6ad0d8814c1e2249e722
The approach here is to simply mark current TelemetryCollector as having restarted.
The downside of this approach is that two technically separate syncs are combined into one
telemetry ping. However, the two syncs are logically connected to each other, and combining
their telemetry will make it easier to figure out why a restart occurred, as well as what
happened after the restart.
MozReview-Commit-ID: AtJbge2ulMz
--HG--
extra : rebase_source : 4f9efb83da8f31b2e0470df6538c67533872f23a
While this is a "named" stage, it doesn't follow the Repository<->Repository
semantics of other named stages, and so it needs to be instrumented separately.
MozReview-Commit-ID: IKrc5Fb1bYm
--HG--
extra : rebase_source : 59c83e44235101f76b42f0eced867ce7b9d5a464
SyncAdapter owns a TelemetryCollector, which is passed into GlobalSession to be "filled up"
with telemtry data.
GlobalSession obtains instances of TelemetryStageCollector from the TelemetryCollector, and
passes them into individual stages. They are filled up with telemetry as stages are executed.
Stage errors are recorded in TelemetryStageCollector.
Various global errors are recorded in TelemetryCollector itself.
On completion (success, failure, abort), telemetry is "built" and broadcasted via LocalBroadcastManager.
TelemetryContract is used to establish a key convention between the "broadcaster" and whoever is
on the receiving end of this telemetry.
This patch instruments stages which follow the Repository<->Repository flow semantics. Other stages,
such as the clients stage, meta/globa, info/* and crypto/keys are instrumented separately in follow-up
patches.
MozReview-Commit-ID: 5VLRc96GLdV
--HG--
extra : rebase_source : 4c7a7e1fde2e32d401eb28c70b9f04fdbd148ffd
This is what we (and other platforms) use as part of telemetry payloads in place of either
our local FxA Device ID or the sync client ID.
Note that this server API is currently undocumented.
Parameter introduced in 2021994ca4
MozReview-Commit-ID: 64sY5RZ2ZxK
--HG--
extra : rebase_source : d1790feae1c0f46dc5f420aeed347da12a6ac85c
We will need them later for telemetry reporting. For now we're just keeping the last exception which
we encountered (which agrees with desktop's behaviour), and Bug 1362208 explores follow-up work to
aggregate and count the exceptions as we see them.
MozReview-Commit-ID: 8yKkZVGJZ9e
--HG--
extra : rebase_source : 501ff746ecfb3022a0fe89844e307153bfdb5164
This patch:
- introduces a way to signal that a record has been reconciled; this is not a "flow control"
event type, and must be used in addition to regular "recordStored" delegate call
- draws a clearer distinction between "attempted to store" and "stored, as reported by session's storage layer"
MozReview-Commit-ID: 99UbUJzu57w
--HG--
extra : rebase_source : d7424fec748b9a2d07d1c98b78ce89fd418750e4
It's not just used for testing, and annotation is causing IDE to highlight its uses in code as invalid.
MozReview-Commit-ID: JvzX2VgNKom
--HG--
extra : rebase_source : a16933121371818307329523916d35e82b2446c9
The primary issue is that we use a throwing InputStreamReader
constructor. If it throws, then any nested streams will be lost.
We can fix that by using the non-throwing InputStreamReader
constructor (which uses a Charset as the second parameter,
instead of a String which causes an Exception to be thrown
if it can't be parsed)
We also simplify some nested Stream's a little: most of the
Stream constructors don't throw, so there's no harm in not keeping
individual references to those that don't throw - and that
results in less Stream references for us to handle.
MozReview-Commit-ID: 2hyRFGVmGnU
--HG--
extra : rebase_source : 9d2b25997e0f71089c0ef56c0069cafe068f821e
The primary issue is that we use a throwing InputStreamReader
constructor. If it throws, then any nested streams will be lost.
We can fix that by using the non-throwing InputStreamReader
constructor (which uses a Charset as the second parameter,
instead of a String which causes an Exception to be thrown
if it can't be parsed)
We also simplify some nested Stream's a little: most of the
Stream constructors don't throw, so there's no harm in not keeping
individual references to those that don't throw - and that
results in less Stream references for us to handle.
MozReview-Commit-ID: 2hyRFGVmGnU
--HG--
extra : rebase_source : 15dd97d28012a017326b01ae8ddc370c7f1ec484
Incoming records might be missing the dateAdded field, and so we perform some pre-processing:
- during reconciliation, dateAdded is set to the lowest of (remote lastModified, remote dateAdded, local dateAdded)
- during insertion, if dateAdded is missing it is set to lastModified
Whenever we modify dateAdded for a record during sync, we also bump its lastModified value. This will trigger an
upload of this record, and consequently a re-upload by clients which are able to provide an older dateAdded value.
It is possible that this might cause conflicts on other devices, but the expected likelyhood of that happening is low.
MozReview-Commit-ID: 3tDeXKSBgrO
--HG--
extra : rebase_source : 26cb13838df7a4adb6d4fe3c51f0ecf3fd2eda95
Confusion between storeDone() and storeDone(long end) resulted in certain sessions (bookmarks
and form history) not overriding the current method. As a result, their final "flush the queue"
methods weren't being called by the buffering middleware.
This patch removes the storeDone(long end) method, making such confusion a non-issue.
Given that a lot of sessions tend to build up buffers which they need to then flush after a storeDone()
call, passing in a timestamp into that method doesn't make sense. Instead, let's supply a default
implementation in RepositorySession which calls onStoreCompleted(endTimestamp) with current time,
and allow sessions to override this method and own the onStoreCompleted(endTimestamp) call.
MozReview-Commit-ID: 84o7aAL8RPC
--HG--
extra : rebase_source : 41767ad502bd5ad8a0a487235bfdca8cf0d0c927
Since we're uploading records atomically, order in which they're processed by the uploader
only matters if we want to do sanity checks on certain types of records. Server might still
preserve some of the order, but for our purposes here it shouldn't matter.
We'd like to ensure that we process the "mobile root" bookmark record along with other folder
records first, so that we increase our chances of avoiding making a failing network request if
that those records' payload is too large.
Sorting by bookmark type achieves this.
MozReview-Commit-ID: KrAs3zepaOk
--HG--
extra : rebase_source : 24f1d3d6aa2ee3b6777dc38abdd1e01aba5213c2
If we try to upload a record whose payload BSO field is larger than the limit specified
by the server, fail that record during BatchingUploader's processing.
Consequently, Synchronizer will fail current sync stage and advance to the next.
Previous behaviour was to essentially rely on the server to fail our POST request,
at which point we'll fail current sync stage. So in a way, this is an optimization to
avoid making network requests which we know will fail.
MozReview-Commit-ID: 5aJRRNoUuXe
--HG--
extra : rebase_source : 18920cfe7b7599be1984c53ebc0c9897c98fb7d9
We need to access sessionToken in the Engaged state in order to perform device
registration. We expose getSessionToken() on the base State class, to allow
customers to get the sessionToken easily instead of having to downcast the
TokensAndKeysState/Engaged states.
MozReview-Commit-ID: 8s2C350noUG
--HG--
extra : rebase_source : e0bc8bf7ebfdcb7a31bb4a6ddb5b928acf7baba9
We upload meta/global in three scenarios:
- fresh start
- when it was modified after a successful sync
- when it was modified after an aborted sync
Use X-I-U-S header to assert what we believe about meta/global's presence (during freshStart)
and last-modified timestamp (in all other cases).
We might encounter a concurrent modification condition, manifesting as a 412 error. If we see such an error:
- on fresh start, we restart globalSession
- on regular upload, we request a re-sync of all stages
MozReview-Commit-ID: 3qyb6rUSOeY
--HG--
extra : rebase_source : 166be44aceb634b4e9fa3a8e20f7047cfec2af54
On startup and at the beginning of a sync we check how long it has been since we've subscribed
to a channel for fxa service. If it's been over 21 days, request re-subscription.
MozReview-Commit-ID: GzvPecZ9hTy
--HG--
extra : rebase_source : d0292acddbdd231502808469d4e5502a4ac93779
BatchingDownloader uses provided RepositoryStateProvider instance in order to track
offset and high water mark as it performs batching.
The state holder objects are initialized by individual ServerSyncStages, and prefixes are used to ensure keys
won't clash.
Two RepositoryStateProvider implementations are used: persistent and non-persistent. Non-persistent
state provider does not allow for resuming after a sync restart, while persistent one does.
Persistent state provider is used by the history stage. It is fetched oldest-first, and records are applied
to live storage as they're downloaded. These conditions let use resume downloads. It's also possible to
resume downloads for stages which use a persistent buffer, but currently we do not have any.
Offset value and its context is reset if we hit a 412 error; it is maintained if we hit a sync deadline, allowing us to
minimize number of records we'll redownload. BatchingDownloaderController owns resuming and context checking logic.
High water mark (h.w.m.) is maintained across syncs and used instead of stage's "last-synced" timestamp if said stage is
set to fetch oldest-first and explicitely allows use of a h.w.m. Server15RepositorySession provides correct timestamp
to RecordsChannel, decoupling BatchingDownloader from this logic.
MozReview-Commit-ID: IH28YrDU4vW
--HG--
extra : rebase_source : 63bd7daaa1fd2a63e10289d6d4cd198aaf81498b
Stage re-sync is requested if:
- We hit a 412 either during batching download or batching upload
- We hit a sync deadline either during batching download or when merging records from the buffer
SessionStoreDelegate interface was expanded with onStoreFailed,
indicating that not just a particular record failed, but the whole operation did.
onFetchFailed is used to inform delegates of 412/deadline failures during downloads.
Three new exception types were added, to facilitated messaging between different layers.
MozReview-Commit-ID: Ltdi5noEvdV
--HG--
extra : rebase_source : 9d4af039198b9bc92fbbf25cf8e3d32375a2ab26
This commit does two things:
1) It simplifies history insertion logic, which wrongly assumed that history which was
being inserted might be not new. As such, it was necessary to check for collisions of
visit inserts, record number of visits actually inserted, and update remote visit counts
correspondingly in a separate step, making history insert a three step operation (insert
history record, insert its visits, update history record with a count). However, bulkInsert
runs only for records which were determined to be entirely new, so it's possible to drop
the third step.
2) Makes all of the insertions (history records and their visits) run in one transaction.
Prepared statements for both history and visit inserts are used are used as a
performance optimization measure.
MozReview-Commit-ID: 48T4G5IsQNS
--HG--
extra : rebase_source : 280d468ef9b57163a178e42707aee610977625c4
We're at Sync 1.5 now, so might as well rename the files.
Also, renamed the ConstrainedRepository... to a name that's more reflective
of that session's role after the changes.
MozReview-Commit-ID: 96XCzoBzD5D
--HG--
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/Server11PreviousPostFailedException.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/Server15PreviousPostFailedException.java
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/Server11RecordPostFailedException.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/Server15RecordPostFailedException.java
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConfigurableServer15Repository.java
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15Repository.java
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15RepositorySession.java
rename : mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestServer11Repository.java => mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestServer15Repository.java
rename : mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestServer11RepositorySession.java => mobile/android/tests/background/junit4/src/org/mozilla/android/sync/test/TestServer15RepositorySession.java
extra : rebase_source : 96f7211951611ce7785edbef9dce412accb2878d
Recent history stage will only run if full history stage did not complete yet.
Bug 1316110 tracks follow up work to make this more efficient.
MozReview-Commit-ID: 7dtbfEFUMGB
--HG--
extra : rebase_source : 94a3e652d9dcf7996e14b96aee28810baee078ea
History stage does not wrap history respository in a buffer, because we'd like to
use a high-water-mark and offset resuming later on, and using a persistent buffer
for this stage does not make sense.
MozReview-Commit-ID: FS1swml2bIC
--HG--
extra : rebase_source : be197e0459d86a320076174936cea8ee76e1dbed
Intended to signal that a group of records have been fetched, and more are
to come after a pause.
MozReview-Commit-ID: 8ozZTc6aNdA
--HG--
extra : rebase_source : e2fdf70d6db6e242e65b788dcb6a09f975b5124b
Otherwise we often end up with delegate meaning both fetch delegate and store delegate
in extending classes, which gets a little confusing.
MozReview-Commit-ID: L4Sd79jLr88
--HG--
extra : rebase_source : c8df4e2ea373dd415e1c113ccf37c09e392a5302
Previous state:
- Two threads were racing to get to batchMeta - one to reset its state, and the other to
read its internal state to construct a network request, and then to update its internal state.
- This resulted in data corruption when payloads had to be split into multiple batches.
A core problem was that there is a lot of state shared across thread boundaries. Specifically,
BatchMeta is being written and read both by record consumer threads running off of a thread pool,
and by the network worker thread(s).
This patch refactors BatchMeta and scheduling of network runnables to ensure that cross-thread access
is minimized, and "who owns/accesses what" is explicit.
- PayloadDispatcher owns scheduling payload runnables and any data they need to share between each other.
- UploaderMeta owns information that's necessary to process incoming records.
MozReview-Commit-ID: 9hFs3fXGaGM
--HG--
rename : mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/uploaders/BatchMeta.java => mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/uploaders/UploaderMeta.java
rename : mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/uploaders/BatchMetaTest.java => mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/repositories/uploaders/UploaderMetaTest.java
extra : rebase_source : f0f1a05f19f40a6514d4f0dac9d531b086c3f3ed
When debug mode is toggled, hardRefresh() is called, removing and adding all preferences. However, profileAvatarTarget wasn't being updated with the new instance of profilePreference. This was resolved by moving the creation of profileAvatarTarget to refresh(), which is called in hardRefresh().
MozReview-Commit-ID: 8raarHbmR23
--HG--
extra : rebase_source : cf0bf90ca67b4e5dd322af25ed16ac43116fafd1
Since we're no longer pausing Gecko when entering this activity, it must implement this interface so we can still properly pause Gecko if we get backgrounded while on the Sync preferences screen.
Most actions here are actually done via the application context (i.e. GeckoApplication), so overriding startActivity et al. and using mGeckoActivityOpened doesn't achieve all that much for most cases, but since we currently at most exit the screen (activity is finishing, so won't trigger a GeckoThread.onPause() call) and stay within our application (open a new tab in Firefox), we're still fine for now.
MozReview-Commit-ID: 3760hXMjckX
--HG--
extra : rebase_source : 026654ca101082140f9fbbc922562f9890daab50
Grigory Kruglov
Mark Hammond
Thom Chiovoloni
Nevin Chen
Ryan VanderMeulen
Edouard Oger
kimsaehun
friedger
udara
Andrzej Hunt
Carsten "Tomcat" Book
Wes Kocher
Sean Maltby
Jan Henning