Andrew Swan [:aswan] Assignee
	Description • 9 years ago

Part of supporting chrome.downloads in WebExtensions

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

	Andrew Swan [:aswan] Assignee
	Comment 1 • 8 years ago

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

	Kris Maglione [:kmag]
	Comment 2 • 8 years ago

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?

	Andrew Swan [:aswan] Assignee
	Comment 3 • 8 years ago

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?

	Kris Maglione [:kmag]
	Comment 4 • 8 years ago

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.

	Andrew Swan [:aswan] Assignee
	Comment 5 • 8 years ago

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/

	Pulsebot
	Comment 6 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 7 • 8 years ago

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

	Pulsebot
	Comment 8 • 8 years ago

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

	Andrew Swan [:aswan] Assignee
	Comment 9 • 8 years ago

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/

	Andrew Swan [:aswan] Assignee
	Comment 10 • 8 years ago

Pfft.  Now passing lint.

	Kris Maglione [:kmag]
	Comment 11 • 8 years ago

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

	Pulsebot
	Comment 12 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 13 • 8 years ago
bugherder

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