1317841 - Update version of Kinto.js client to 6.0.0

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Ethan Glasser-Camp (:glasserc)]

kinto.js released a major version, v6.0.0, which removes FirefoxStorage from the repo. This blob of code which provides a Sqlite.jsm adapter for Kinto is really Gecko-specific so it's intended for it to migrate into the Gecko codebase. See https://github.com/Kinto/kinto.js/pull/562 for more information.

Because this release is a bit tricky, I'm hoping to get it out of the way sooner rather than later.

Comment 1

[Ethan Glasser-Camp (:glasserc)]

Attachment: Bug 1317841 - Update version of kinto.js to v6.0.0,

This major version of kinto.js released without a FirefoxStorage
adapter. Since Gecko is the only project with a need for this adapter
as well as the only one who can use it, that file moves to this repo
as a new kinto-storage-adapter.js. This file is mostly a copy of the
most recent FirefoxStorage.js, plus some other non-exported utility
functions copied from kinto.js to make it work.

This changes the export of the kinto-offline-client.js from a
`loadKinto` function which returns the Kinto class, to the actual
Kinto class itself, with the user expected to "hook up" the
adapter. It turns out that this doesn't change much in how we actually
use Kinto, since we were always previously explicitly selecting the
Kinto adapter.

This release of kinto.js also changes some of the build options
around, which changes the minified output of kinto-offline-client.js.

There are still some outstanding stylistic complaints about
FirefoxAdapter having to do with its _init static method which is
public and the fact that sometimes FirefoxAdapter manages its own
Sqlite connection and sometimes it doesn't. These will be addressed in
a future patch.

Review commit: https://reviewboard.mozilla.org/r/93300/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93300/

Comment 2

[Ethan Glasser-Camp (:glasserc)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea1a8b7b32f1

Comment 3

[Matthew N. [:MattN]]

Attachment: Bug 1317841 - Update version of kinto.js to v6.0.0,

This major version of kinto.js released without a FirefoxStorage
adapter. Since Gecko is the only project with a need for this adapter
as well as the only one who can use it, that file moves to this repo
as a new kinto-storage-adapter.js. This file is mostly a copy of the
most recent FirefoxStorage.js, plus some other non-exported utility
functions copied from kinto.js to make it work.

This changes the export of the kinto-offline-client.js from a
`loadKinto` function which returns the Kinto class, to the actual
Kinto class itself, with the user expected to "hook up" the
adapter. It turns out that this doesn't change much in how we actually
use Kinto, since we were always previously explicitly selecting the
Kinto adapter.

This release of kinto.js also changes some of the build options
around, which changes the minified output of kinto-offline-client.js.

There are still some outstanding stylistic complaints about
FirefoxAdapter having to do with its _init static method which is
public and the fact that sometimes FirefoxAdapter manages its own
Sqlite connection and sometimes it doesn't. These will be addressed in
a future patch.

Review commit: https://reviewboard.mozilla.org/r/93308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93308/

Comment 4

[Matthew N. [:MattN]]

(In reply to Matthew N. [:MattN] from comment #3)
> Created attachment 8811092 [details]
> Bug 1317841 - Update version of kinto.js to v6.0.0,

This is an version of the patch using `hg cp --after services/common/kinto-offline-client.js services/common/kinto-storage-adapter.js` so that the history of the firefox adapter is preserved and so I can see the interdiff between the implementations. I will review this version instead. 

Ethan, can you update your patch to preserve the history? If not, I can land it for you with this change.

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8811092 [details]
Bug 1317841 - Update version of kinto.js to v6.0.0,

https://reviewboard.mozilla.org/r/93308/#review93310

r=me with questions about why the style is diverging from the usual browser/toolkit style and also with a preference to have an `hg copy`.

::: services/common/kinto-storage-adapter.js:193
(Diff revision 1)
> -  constructor(collection, options = {}) {
> +  constructor(collection, options={}) {
>      super();
> -    const { sqliteHandle = null } = options;
> +    const {sqliteHandle=null} = options;

Why the change in style? Mozilla-central style guide is to have spaces around operators.

::: services/common/kinto-storage-adapter.js:223
(Diff revision 1)
>  
> -  _executeStatement(statement, params) {
> +  _executeStatement(statement, params){
>      if (!this._connection) {
>        throw new Error("The storage adapter is not open");
>      }
>      return this._connection.executeCached(statement, params);
>    }
>  
> +
>    open() {
>      const self = this;
> -    return Task.spawn(function* () {
> +    return Task.spawn(function* (){

Ditto for the lack of space before `{`.

::: services/common/kinto-storage-adapter.js:270
(Diff revision 1)
> +        ...options.preload
> +      ];

Nit: Add a trailing comma here? That's the usual style in m-c (to prevent the change in blame when another item is added to the array) and I think I saw it was used elsewhere in this file.

Comment 6

[Ethan Glasser-Camp (:glasserc)]

Attachment: Bug 1317841 - Stylistic pass on new kinto-storage-adapter,

- Reflow things to try to avoid long lines.

- Add terminal comma where possible.

Review commit: https://reviewboard.mozilla.org/r/93482/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93482/

Comment 7

[Ethan Glasser-Camp (:glasserc)]

Comment on attachment 8811083 [details]
Bug 1317841 - Update version of kinto.js to v6.0.0,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/93300/diff/1-2/

Comment 8

[Ethan Glasser-Camp (:glasserc)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c369a31a5426

Comment 9

[Ethan Glasser-Camp (:glasserc)]

I believe I've updated my patch to maintain the history using `hg cp --after`. (I've also rebased it.)

The first `try` build was one that I launched using `--artifact`, just as an experiment. It seems to have broken in a lot of ways. I tried again just to see, but it looks like that's going to break too, so I'm going to launch another one without `--artifact`.

The stylistic changes were because the original FirefoxStorage.js (which I copied) was a source file, and what we had in kinto-offline-client.js was the browserify'd output. Stylistic stuff got cleaned up automatically as part of the browserify step. I've tried to maintain the style (of the browserify'd output, which seems to match m-c house style) in the first commit, and then did some explicit cleanups as part of a second one. If there are other stylistic things you'd like me to clean up while I'm here, I'd be happy to do that. It doesn't seem like there's an eslint file that applies to this directory, since everything is eslint clean no matter what I do.

Comment 10

[Ethan Glasser-Camp (:glasserc)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb82215d4127

Comment 11

[Ethan Glasser-Camp (:glasserc)]

Comment on attachment 8811083 [details]
Bug 1317841 - Update version of kinto.js to v6.0.0,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/93300/diff/2-3/

Comment 12

[Ethan Glasser-Camp (:glasserc)]

Comment on attachment 8811328 [details]
Bug 1317841 - Stylistic pass on new kinto-storage-adapter,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/93482/diff/1-2/

Comment 13

[Ethan Glasser-Camp (:glasserc)]

I think the failed tests are unrelated intermittents: browser/base/content/test/general/browser_save_private_link_perwindowpb.js, devtools/client/debugger/test/mochitest/browser_dbg_pretty-print-04.js, browser/components/originattributes/test/browser/browser_cache.js, browser/components/sessionstore/test/browser_windowStateContainer.js, browser/base/content/test/newtab/browser_newtab_bug722273.js, and devtools/client/netmonitor/test/browser_net_statistics-01.js.

Comment 14

[Matthew N. [:MattN]]

Comment on attachment 8811328 [details]
Bug 1317841 - Stylistic pass on new kinto-storage-adapter,

https://reviewboard.mozilla.org/r/93482/#review94432

Thanks

Comment 15

[Matthew N. [:MattN]]

Comment on attachment 8811083 [details]
Bug 1317841 - Update version of kinto.js to v6.0.0,

https://reviewboard.mozilla.org/r/93300/#review94434

Comment 16

[Matthew N. [:MattN]]

LGTM, the one leak failure doesn't seem like an exact intermittent failure bug match but it's unlikely related to this.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea5ef470629e
Update version of kinto.js to v6.0.0, r=MattN
https://hg.mozilla.org/integration/autoland/rev/cb29d98401b6
Stylistic pass on new kinto-storage-adapter, r=MattN

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ea5ef470629e
https://hg.mozilla.org/mozilla-central/rev/cb29d98401b6