resource://gre/modules/HiddenFrame.jsm, line 23: TypeError: can't access property Symbol.iterator, ChromeUtils.nondeterministicGetWeakSetKeys(...) is undefined
Categories
(Toolkit :: General, defect, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | wontfix |
firefox79 | --- | wontfix |
firefox80 | --- | wontfix |
firefox81 | --- | fixed |
People
(Reporter: robwu, Assigned: casverploegen, Mentored)
References
(Regression)
Details
(Keywords: good-first-bug, regression, Whiteboard: [lang=js])
Attachments
(1 file)
In a log of an unrelated bug (with a test failure), I spotted the following message in the log:
JavaScript error: resource://gre/modules/HiddenFrame.jsm, line 23: TypeError: can't access property Symbol.iterator, ChromeUtils.nondeterministicGetWeakSetKeys(...) is undefined
When I look at the code at https://searchfox.org/mozilla-central/rev/46e3b1ce2cc120a188f6940b5c6eab6b24530e4f/toolkit/modules/HiddenFrame.jsm#23
... then the failure reason is quite obvious: ChromeUtils.nondeterministicGetWeakSetKeys
is used on gAllHiddenFrames
, which is not a WeakSet
but just a Set
(so the return value is undefined
instead of an array). This bug can be fixed by directly iterating over gAllHiddenFrames
instead.
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
The severity field is not set for this bug.
:Dolske, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Hi. If I understand correctly, what you're saying is to basically change ChromeUtils.nondeterministicGetWeakSetKeys(gAllHiddenFrames)
to gAllHiddenFrames
. If that's all I'd like to have a go at this as my first bugfix!
Reporter | ||
Comment 3•4 years ago
|
||
It's indeed that simple. To verify the fix, I suggest that you use Searchfox to find a test that triggers this error, and run that test before fixing this bug.
When you do observe the failure, fix the bug and run the test again. The reported error should be gone.
Assignee | ||
Comment 4•4 years ago
|
||
It took some time to get up and running, I think everything works now. I ran into one problem, I am unable to find a test on Searchfox that triggers this exact error. Could you walk me through the steps or provide a link to the test? Thanks in advance. Furthermore, could I assign you as the reviewer once the patch is ready?
Reporter | ||
Comment 5•4 years ago
|
||
To find a test that triggers the error, look up where ensureCleanupRegistered
is called, and go all the way up. Ultimately it is called from the get
method of HiddenFrame
. So look for files that import/use HiddenFrame
and call .get()
on it.
Furthermore, could I assign you as the reviewer once the patch is ready?
Sure.
Assignee | ||
Comment 6•4 years ago
|
||
Thanks, with this method I found the correct test to be located in testing/specialpowers/content/SpecialPowersParent.jsm
(https://searchfox.org/mozilla-central/source/testing/specialpowers/content/SpecialPowersParent.jsm#1160). However, when trying to run the test with ./mach test testing/specialpowers/content/SpecialPowersParent.jsm
I get the error message that it was unable to find tests from the given argument(s). I also tried running it with wpt
, mochitest
, xpcshell-test
and reftest
instead of test
, but with no luck yet. Could you point me in the right direction on how to run the test correctly?
Reporter | ||
Comment 7•4 years ago
|
||
SpecialPowersParent.jsm
is not a test file, but an implementation of the test framework. Test files in Firefox are commonly stored in a directory (or subdirectory) of a directory called test
.
I guess that you searched for HiddenFrame.get
to find that file. The result is coincidentally correct, but only because that file decided to use a variable name with a similar name as the class instance that you're looking for (hiddenFrame
).
To find better results, I suggest that you try the following:
- Find files that import
HiddenFrame.jsm
- Find occurrences of the
HiddenFrame
constructor (which was imported fromHiddenFrame.jsm
). - Look for
.get()
calls.
There aren't that many instances, so for this specific case you could just look at test files that import HiddenFrame.jsm
, but nevertheless it would be a good practice to truly find where the code is being called.
Assignee | ||
Comment 8•4 years ago
|
||
Okay, I looked again and found three instances of .get()
calls on the HiddenFrame
instance (https://searchfox.org/mozilla-central/source/browser/components/uitour/test/browser_no_tabs.js#24 , https://searchfox.org/mozilla-central/source/browser/components/extensions/test/browser/browser_ext_webRequest.js#16 , https://searchfox.org/mozilla-central/source/browser/base/content/test/static/browser_parsable_css.js#357). However, when running these tests, the error does not occur in the output logs. Am I looking in the wrong place or is something else at play here?
For more info, I ran ./mach test browser/components/uitour/test/browser_no_tabs.js
and similar commands for the other test files.
Reporter | ||
Comment 9•4 years ago
|
||
The issue occurs at late shutdown, and I don't recall how to trigger it from a test.
Let's submit a patch without running automated tests then.
If you want to verify that the logic is correct without running tests, then you can open the global JavaScript console of Firefox (https://developer.mozilla.org/en-US/docs/Tools/Browser_Console) and run a code snippet to verify that the fix should work.
The relevant logic is here: https://searchfox.org/mozilla-central/rev/8cf9cec21122aeab86126ddb566cba52bb6777d8/toolkit/modules/HiddenFrame.jsm#16,23-26
You can start by confirming the bug: Running that logic triggers an error.
Then, after your fix, you need to verify that you are able to iterate over the set.
Assignee | ||
Comment 10•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
I confirmed the bug in the browser console. After changing the code, the bug was indeed gone and it was able to iterate over the gAllHiddenFrames
Set without throwing an error. The changes were committed and submitted to Phabricator, with you (Rob Wu) as the reviewer. Thanks a lot for the help, I look forward to your review and working on more bugs in the future.
Reporter | ||
Comment 12•4 years ago
|
||
As you might have noticed, I have reviewed and approved the patch, so it is ready to land. To land the patch, add the checkin-needed keyword in Phabricator, as explained at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Thanks again for your contribution.
Comment 13•4 years ago
|
||
Pushed by apavel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4b30578a9880 Fixed TypeError on HiddenFrame Set. r=robwu
Comment 14•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Description
•