1293002 - Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in mobile/

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1290637.
see bug 1290637 for the details.

Required code changes are following:
  * Check each usage of non-standard Iterator [1] function,
    and replace it with Object.entries [2] or Object.values [3],
    or something appropriate for the specific usage

Here's the rough list for Iterator() usage (not exhaustive)
https://dxr.mozilla.org/mozilla-central/search?q=%22+Iterator(%22+path%3Amobile+-path%3Anews.bbcimg.co.uk&redirect=false

for example:
  for (let [k, v] in Iterator(obj)) {
    ...
  }
can be replaced to:
  for (let [k, v] of Object.entries(obj)) {
    ...
  }

another example:
  for (let [, v] in Iterator(obj)) {
    ...
  }
can be replaced to:
  for (let v of Object.values(obj)) {
    ...
  }


[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator
[2] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/entries
[3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/values

Comment 1

[tuhina chatterjee]

Hi I would like to work on this bug. It will be my first bug.

Comment 2

[Tooru Fujisawa [:arai]]

Thank you for your comment :)

If you have any question related to the required fix, building, or testing, feel free to ask here,
or ping me (:arai) in IRC #introduction channel

Comment 3

[tuhina chatterjee]

I found two occurrences of Iterator(obj) in the code. I have replaced them with Object.entries and Object.values as mentioned above. How do I test it? Do I write automated tests? The build is successful.

Comment 4

[tuhina chatterjee]

Edit : I have changed all the occurences of Iterator(obj). I have replaced them with Object.entries and Object.values as mentioned above. How do I test it? Do I write automated tests? The build is successful.

Comment 5

[Tooru Fujisawa [:arai]]

(In reply to tuhina chatterjee from comment #4)
> Edit : I have changed all the occurences of Iterator(obj). I have replaced
> them with Object.entries and Object.values as mentioned above. How do I test
> it? Do I write automated tests? The build is successful.

Unfortunately, there seems to be no existing automated test that tests the related code.
It would be great if you can write a testcase for it, but for this case, this code seems to be tested manually for previous changes.

according to bug 630341 and bug 1035439 (that added those Iterator), here are steps for manual test

steps for bug 630341
1. run Firefox for Android
2. open "Setting"
3. in "Advanced" pane, "Restore tabs" option, choose "Always restore"
4. close "Setting"
5. open slashdot.org
6. hit Home button to suspend Firefox for Android
7. terminate Firefox for Android process
8. restart Firefox for Android
   (the old tab is opened as part of session restore)
9. close the tab
10. confirm Slashdot is in "RECENT TABS" in home screen
11. open "Setting"
12. in "Clear Private Data", choose "Clear now"
13. choose everithing , "Clear now" dialog, and hit "CLEAR DATA"
14. hit Home button to suspend Firefox for Android
15. terminate Firefox for Android process
16. restart Firefox for Android
17. confirm Slashdot is *not* in "RECENT TABS" in home screen

steps for bug 1035439
1. run Firefox for Android
2. open bugzilla.mozilla.org
3. close the tab
4. create a "New Private Tab"
5. click URL bar
6. confirm bugzilla.mozilla.org is not in "RECENT TABS" in home screen


If the result with those operations doesn't change after applying patch, the code change should be okay.

Comment 6

[tuhina chatterjee]

Attachment: Changes

Bug fix replacing Iterator() with Object.entries or Object.values in mobile/ directory.
Please review this.

Comment 7

[Tooru Fujisawa [:arai]]

Comment on attachment 8779504 [details] [diff] [review]
Changes

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

Thank you for your patch :)

The change looks fine.
Can you export it as a patch file? ("hg export" command, or "git format-patch" command)
this file doesn't have commit message, nor author information.

Comment 8

[tuhina chatterjee]

Attachment: hg_export

Added patch (created using hg export)

Comment 9

[Tooru Fujisawa [:arai]]

Comment on attachment 8779653 [details] [diff] [review]
hg_export

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

Thanks again :)
It's perfect.

for the next time posting a patch, please set "review" flag to "?" and choose reviewer (Requestee) from suggested reviewers,
and also mark previous patch "obsolete".

anyway, forwarding to liuche, as I'm not a module owner/peer here.

Comment 10

[tuhina chatterjee]

Can I be assigned to the bug ?

Comment 11

[Tooru Fujisawa [:arai]]

yes
thanks :)

Comment 12

[Chenxia Liu [:liuche]]

Comment on attachment 8779653 [details] [diff] [review]
hg_export

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

Looks fine to me.

Please upload a version of the patch with the addition of r=reviewer (in this case, me, liuche) as the patch message. That way, when looking at commits in-tree, it's possible to tell who reviewed the code.

Comment 13

[tuhina chatterjee]

Sure liuche. I will keep that in mind the next time.

Comment 14

[Tooru Fujisawa [:arai]]

Thank you :)

Sheriff will add "r=reviewer" part when landing a patch.

Comment 15

[Tooru Fujisawa [:arai]]

oops, I forgot try run, sorry.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d0a625ee469

will add checkin-needed again when it finishes.

Comment 16

[Tooru Fujisawa [:arai]]

okay, it passed try run.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/e336054c4c85
Replace in-tree consumer of non-standard Iterator() with Object.{values,entries} in mobile/. r=liuche

Comment 18

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

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