Closed Bug 1299631 Opened 5 years ago Closed 5 years ago

Avoid accessing dead windows in test-firefox-windows.js

Categories

(Add-on SDK Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This is fallout from bug 1276366 where test-firefox-windows.js ended up accessing dead object wrappers.

Rather than checking for window.closed we can just check the number of active
windows.
:zer0 it looks like you did the last review in this area, feel free to redirect.
Attachment #8786954 - Flags: review?(zer0)
Comment on attachment 8786954 [details] [diff] [review]
Avoid accessing dead windows in test-firefox-windows.js

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

::: addon-sdk/source/test/windows/test-firefox-windows.js
@@ +293,2 @@
>  
> +  assert.equal(windows.length, 1, "Correct number of browser windows");

Maybe we should also check that the window left open, is the one expected to be – just to be a more similar version of the check we had before; where we checked specifically that window 2 and window 3 where closed.

But I'm fine either way.
Attachment #8786954 - Flags: review?(zer0) → review+
(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #3)
> Comment on attachment 8786954 [details] [diff] [review]
> Avoid accessing dead windows in test-firefox-windows.js
> 
> Review of attachment 8786954 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: addon-sdk/source/test/windows/test-firefox-windows.js
> @@ +293,2 @@
> >  
> > +  assert.equal(windows.length, 1, "Correct number of browser windows");
> 
> Maybe we should also check that the window left open, is the one expected to
> be – just to be a more similar version of the check we had before; where we
> checked specifically that window 2 and window 3 where closed.
> 
> But I'm fine either way.

Thanks for the quick review, I'll update with your suggestion before landing.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cfb84806c0863af4facbdde2b37ac5fdd40567c
Bug 1299631 - Avoid accessing dead windows in test-firefox-windows.js. r=zer0
https://hg.mozilla.org/mozilla-central/rev/0cfb84806c08
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.