1245599 - Implement chrome.downloads.onCreated

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Andrew Swan [:aswan]]

Part of supporting chrome.downloads in WebExtensions

https://developer.chrome.com/extensions/downloads#event-onCreated

Comment 1

[Andrew Swan [:aswan]]

Attachment: MozReview Request: Bug 1245599 - Implement chrome.downloads.onCreated r?kmag

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

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8727990 [details]
MozReview Request: Bug 1245599 - Implement chrome.downloads.onCreated r?kmag

https://reviewboard.mozilla.org/r/38725/#review35427

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_misc.html:57
(Diff revision 1)
> +            || !expected.every((item, i) => compare(events[i], item))) {

Can we explicitly fail if one of these doesn't match?

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_misc.html:122
(Diff revision 1)
> +  extension = ExtensionTestUtils.loadExtension({

Can you return this rather than making it a global?

Comment 3

[Andrew Swan [:aswan]]

https://reviewboard.mozilla.org/r/38725/#review35427

> Can you return this rather than making it a global?

You're looking at this partway through its journey into becoming a richer test suite, I plan to add additional tests in separate tasks, so things like setup() and runInExtension() would be outside the actual test tasks.  extension is still scoped to this script, is the desire for it to not be global just for general tidiness or is there another reason to prefer a narrower scope?

Comment 4

[Kris Maglione [:kmag]]

https://reviewboard.mozilla.org/r/38725/#review35427

> You're looking at this partway through its journey into becoming a richer test suite, I plan to add additional tests in separate tasks, so things like setup() and runInExtension() would be outside the actual test tasks.  extension is still scoped to this script, is the desire for it to not be global just for general tidiness or is there another reason to prefer a narrower scope?

I generally prefer test tasks to be as self-contained as possible, but I guess this is fine.

Comment 5

[Andrew Swan [:aswan]]

Comment on attachment 8727990 [details]
MozReview Request: Bug 1245599 - Implement chrome.downloads.onCreated r?kmag

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

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6d5004acc11c

Comment 7

[Carsten Book [:Tomcat]]

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=7893663&repo=fx-team

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/a7c7725f1c32

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8727990 [details]
MozReview Request: Bug 1245599 - Implement chrome.downloads.onCreated r?kmag

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

Comment 10

[Andrew Swan [:aswan]]

Pfft.  Now passing lint.

Comment 11

[Kris Maglione [:kmag]]

You might want to enable the ESLint pre-commit hook[1] to prevent that from happening again.

[1]: https://wiki.mozilla.org/WebExtensions/Hacking#Version_control_hooks

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0813fdd079bf

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0813fdd079bf