1321229 - Remove legacy generator from toolkit/.

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Tooru Fujisawa [:arai]]

Attachment: Remove legacy generator from toolkit/.

just replaced legacy generator with ES6 generator.

some notes:

* toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
  |reference_compare_files| wasn't used

* toolkit/content/tests/chrome/test_arrowpanel.xul
  ES6 generator doesn't throw StopIteration, so we can just remove final yield

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8815644 [details] [diff] [review]
Remove legacy generator from toolkit/.

Review of attachment 8815644 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a quick question

::: toolkit/content/tests/chrome/findbar_events_window.xul
@@ +64,5 @@
>      function* onDocumentLoaded() {
>        gFindBar.open();
>        gFindBar.onFindCommand();
>  
>        yield testFind();

Mmmhhh... why not `yield*` here, too?

::: toolkit/content/tests/chrome/test_arrowpanel.xul
@@ -210,5 @@
>      yield;
>    }
>  
>    SimpleTest.finish()
> -  yield;

That looks like a dirty hack. I assume you have tested that it still works without that hack.

Comment 2

[Tooru Fujisawa [:arai]]

Thank you for reviewing :D

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #1)
> Comment on attachment 8815644 [details] [diff] [review]
> Remove legacy generator from toolkit/.
> 
> Review of attachment 8815644 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with a quick question
> 
> ::: toolkit/content/tests/chrome/findbar_events_window.xul
> @@ +64,5 @@
> >      function* onDocumentLoaded() {
> >        gFindBar.open();
> >        gFindBar.onFindCommand();
> >  
> >        yield testFind();
> 
> Mmmhhh... why not `yield*` here, too?

yeah, apparently we can use more `yield*` in the test.
(or maybe we can just use async)
will fix them too.


> ::: toolkit/content/tests/chrome/test_arrowpanel.xul
> @@ -210,5 @@
> >      yield;
> >    }
> >  
> >    SimpleTest.finish()
> > -  yield;
> 
> That looks like a dirty hack. I assume you have tested that it still works
> without that hack.

yes, legacy generator throws StopIteration without it.
the final yield was there to prevent it.
ES6 generator doesn't need it because it doesn't throw when returning.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Remove legacy generator from toolkit/.

Changed to use async functions in findbar_events_window.xul

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8815675 [details] [diff] [review]
Remove legacy generator from toolkit/.

Review of attachment 8815675 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't used await/async yet, so I can't review that part of the patch. But you have my r+ for the previous patch if you want to land it.

Comment 5

[Tooru Fujisawa [:arai]]

okay, I'll use previous patch and leave async one to other bug :)

Comment 6

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fc40e02bb78ead28d05158c2b52cfbf84296d135
Bug 1321229 - Remove legacy generator from toolkit/. r=Yoric

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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