1368146 - Update Screenshots to version 8.1.0

Status: RESOLVED FIXED

(Firefox :: Screenshots, defect)

Description

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Changelog: https://github.com/mozilla-services/screenshots/blob/master/CHANGELOG.md#version-800

Comment 1

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Attachment: Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

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

Comment 2

[Ian Bicking (:ianbicking)]

Comment on attachment 8871883 [details]
Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

https://reviewboard.mozilla.org/r/143402/#review147118

Comment 3

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8871883 [details]
Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

https://reviewboard.mozilla.org/r/143402/#review147918

::: browser/extensions/screenshots/moz.build:18
(Diff revision 1)
>  # AUTOMATIC INSERTION START
>  FINAL_TARGET_FILES.features['[e-mail]']["webextension"] += [
> +  'webextension/.eslintrc.js',
>    'webextension/assertIsTrusted.js',
>    'webextension/blank.html',
>    'webextension/buildSettings.js.template',

This looks wrong. We want buildSettings.js, not the template file.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8871883 [details]
Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

https://reviewboard.mozilla.org/r/143402/#review147782

Looks reasonable. I've put a few notes below. I'd like the .eslintrc.js shipping issue fixed before this ships. The other issues I think we can file bugs on, but we must work out the regressing code (i.e. yield -> await / picking up block changes landing in the tree) before the next version of screenshots ships.

::: browser/extensions/screenshots/bootstrap.js:48
(Diff revision 1)
>    }
>  };
>  
> +const appStartupObserver = {
> +  register() {
> +    Services.obs.addObserver(this, "sessionstore-windows-restored", false); // eslint-disable-line mozilla/no-useless-parameters

I've a feeling this is going to mess with (mochi)tests when we try to enable screenshots for tests again, however, lets get this updated, and then work that out...

::: browser/extensions/screenshots/moz.build:15
(Diff revision 1)
>  ]
>  
>  # This file list is automatically generated by Screenshots' export scripts.
>  # AUTOMATIC INSERTION START
>  FINAL_TARGET_FILES.features['[e-mail]']["webextension"] += [
> +  'webextension/.eslintrc.js',

We shouldn't be trying to ship this file.

::: browser/extensions/screenshots/test/browser/browser_screenshots_ui_check.js:10
(Diff revision 1)
>      is(!!document.getElementById(id), expectPresent, "element " + id + (expectPresent ? " is" : " is not") + " present");
>    }
>  }
>  
> -add_task(async function() {
> -  await promiseScreenshotsEnabled();
> +add_task(function*() {
> +  yield promiseScreenshotsEnabled();

This is worrying, if we're regressing the latest changes from the tree - this needs to be re-imported into the github repo.

Comment 5

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8871883 [details]
Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

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

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Comment on attachment 8871883 [details]
Bug 1368146 - Export Screenshots 8.1.0 to Firefox;

https://reviewboard.mozilla.org/r/143402/#review147782

> We shouldn't be trying to ship this file.

Fixed by 8.1.0 changes

> This is worrying, if we're regressing the latest changes from the tree - this needs to be re-imported into the github repo.

Cool. We'll fix this later in https://github.com/mozilla-services/screenshots/issues/2917

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0e0d7d31cd5
Export Screenshots 8.1.0 to Firefox; r=ianb,standard8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e0e0d7d31cd5

Comment 9

[Ionuț Goldan [:igoldan]]

== Change summary for alert #6946 (as of May 30 2017 19:29 UTC) ==

Improvements:

 11%  ts_paint_webext linux64 opt e10s     1,625.25 -> 1,438.58

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=6946