gecko-dev/services
Ethan Glasser-Camp b228d5b47d Bug 1321570 - use dependency injection for fxAccounts, r=kmag
Get rid of the ugly hack where test code monkeypatches a singleton to
convince the rest of the ExtensionStorageSync code that a fake user is
logged in. Instead, take a handle to the fxAccounts service at
construction time. Test code can provide any kind of fxAccounts it
wants, including one that has a hard-coded user.

This provokes a bunch of changes:

- ExtensionStorageSync is now no longer a singleton, but a class. You
  have to instantiate it in order to do anything with it. A global
  instance extensionStorageSync is provided for use by Sync code as
  well as WebExtension code.

- CryptoCollection is now also a class, and each ExtensionStorageSync
  instance has its own CryptoCollection. This dependency should maybe
  also be injected, but for the time being it doesn't provide us any
  value to do so.

- Converting singletons with asynchronous methods to classes is a pain
  in the neck. We convert async method foo from `foo:
  Task.async(....)` to `async foo() { .... }`. While we're here,
  convert KeyRingEncryptionRemoteTransformer#decode to async/await to
  eliminate a need for `let self = this`.

- Update Sync code and WebExtension code to use extensionStorageSync.

- There's a cyclic dependency where CryptoCollection#sync depends on
  ExtensionStorageSync#_syncCollection which depends on
  CryptoCollection#getKeyRing. As a short-term hack, we now require an
  ExtensionStorageSync argument to CryptoCollection#sync.

- KeyRingEncryptionRemoteTransformer now takes a handle to fxAccounts
  on construction time as well. Because this is the only
  EncryptionRemoteTransformer subclass that accesses fxAccounts, we
  can get rid of the hack where the tests monkeypatch something in the
  EncryptionRemoteTransformer prototype.

- CollectionKeyEncryptionRemoteTransformer now takes a handle to a
  CryptoCollection, rather than relying on a global one.

- A bunch of methods that previously assumed access to fxAccounts now
  check if fxAccounts is present (i.e. if we're on Android). Strictly
  speaking, this isn't required by this change, but it helps make
  certain kinds of failure a little easier to diagnose.

- Update tests, including extracting a domain-specific helper method
  to hide the use of CollectionKeyEncryptionRemoteTransformer. We now
  no longer monkeypatch in our mock fxaService, but pass it to the
  test so that it can do whatever it wants with it. We also provide an
  ExtensionStorageSync instance for convenience. Access to the global
  cryptoCollection is now done through an ExtensionStorageSync
  instance.

To summarize, we've gone from a situation where a bunch of singletons
had implicit dependencies on other singletons in a shared global
namespace, to a situation where dependencies are provided explicitly
using method/constructor arguments. This highlights some of the
dependencies present:

- ExtensionStorageSync depends on CryptoCollection and fxAccounts if
  it needs to sync

- Every collection created via openCollection needs a handle to
  CryptoCollection so it can correctly create its remote transformers

- CryptoCollection needs a handle to fxAccounts so it can create its
  own remote transformer for its special collection

Most of this is only possible, or at least much easier, because we no
longer try to juggle Sqlite connections but just keep one around
forever.

However, please note:

- CryptoCollection needs a handle to ExtensionStorageSync to actually
  perform syncing logic because that's where we foolishly put the
  logic to make requests

- There's still a backing Sqlite store which is shared by everything

- There's still a singleton tracking contexts that opened extensions
  which we manage to try to clean contexts up correctly

MozReview-Commit-ID: DGIzyRTdYZ1

--HG--
extra : rebase_source : 305a98b8e1ece3ae7a5e8a2fdb5c58d8e7c496d9
2017-01-10 16:44:01 -05:00
..
blocklists Bug 1224528 - Load initial JSON files for blocklist r=mgoodwin 2016-12-11 14:37:22 -10:00
cloudsync Bug 1333044 - Prepare services/ for enabling no-undef eslint rule. r=jaws 2017-01-17 12:25:43 +00:00
common Bug 1224528 - Load initial JSON files for blocklist r=mgoodwin 2016-12-11 14:37:22 -10:00
crypto Bug 1333485 - Remove LogUtils.jsm from services/crypto/modules/. r=markh. 2017-02-01 20:55:09 +01:00
fxaccounts Merge inbound to central, a=merge 2017-02-08 16:08:42 -08:00
sync Bug 1321570 - use dependency injection for fxAccounts, r=kmag 2017-01-10 16:44:01 -05:00
.eslintrc.js Bug 1333044 - Enable no-undef eslint rule for services/. r=markh 2017-01-23 15:15:05 +00:00
moz.build Bug 1224528 - Load initial JSON files for blocklist r=mgoodwin 2016-12-11 14:37:22 -10:00