813801 - null-check blocklistWindow in nsBlocklistService.js

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

It should only be possible to hit this in xpcshell tests, but it'd be nice to at least not have this littering the logs:

*** WARN addons.manager: Exception calling callback: TypeError: blocklistWindow is null


This happens because the xpcshell tests can't open windows, so the open() call returns null. 

Just need a null check in there:
https://hg.mozilla.org/mozilla-central/file/f42a83257653/toolkit/mozapps/extensions/nsBlocklistService.js#l1000


No test needed, though bonus points if you manually verify that the above warning shops showing when running the following xpcshell test:
toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js

Comment 1

[zeyu [:car]]

Attachment: add a null check

I didn't test, can you please guide me how to run test? Thanks!

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Great :) I'll give you a chance to test it before I review your patch.


(In reply to zeyu [:car] from comment #1)
> I didn't test, can you please guide me how to run test? Thanks!

You can run just that test via running the following in your repository:

  mach xpcshell-test toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js

Or if you remove just the filename, keeping the path, it'll run all xpcshell tests in that directory.

The following page has all the gritty details, though it describes the old (more complex) way of running this type of test:
https://developer.mozilla.org/en-US/docs/Writing_xpcshell-based_unit_tests

Comment 3

[zeyu [:car]]

Hi Blair,

I tried to run the test, however it seems that the number of lines that can be displayed in terminal is limited so that I cannot see the full log. I also tried 

./mach -l log.log xpcshell-test toolkit/mozapps/extensions/test/xpcshell /test_blocklistchange.js

it only gave me an empty file. So how can I see the full test log? Thanks! BTW, I am running Ubuntu 12.04

Comment 4

[Gregory Szorc [:gps]]

Mach doesn't currently log test output. I suppose we should file a bug on that.

You can always do something like:

./mach xpcshell-test path/to/test > log.log

Comment 5

[zeyu [:car]]

I've run the test and I believe the warning was gone.

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 684837 [details] [diff] [review]
add a null check

Taking this since Blair is out sick today

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 684837 [details] [diff] [review]
add a null check

Looks good. One minor style nit, the rest of this file doesn't use braces around single statements like this. I've gone ahead and corrected that and will land it in a moment.

Thanks for the patch!

Comment 8

[Dave Townsend [:mossop]]

Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c615b961c7b4

Comment 9

[Ed Morley [:emorley]]

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