null-check blocklistWindow in nsBlocklistService.js

RESOLVED FIXED in mozilla20

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Unfocused, Assigned: car)

Tracking

Trunk
mozilla20
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=bmcbride@mozilla.com][lang=js])

Attachments

(1 attachment)

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
(Assignee)

Comment 1

5 years ago
Created attachment 684837 [details] [diff] [review]
add a null check

I didn't test, can you please guide me how to run test? Thanks!
Attachment #684837 - Flags: review?(bmcbride)
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
Assignee: nobody → zeyufly
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
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

5 years ago
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
(Assignee)

Comment 5

5 years ago
I've run the test and I believe the warning was gone.
Comment on attachment 684837 [details] [diff] [review]
add a null check

Taking this since Blair is out sick today
Attachment #684837 - Flags: review?(bmcbride) → review?(dtownsend+bugmail)
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!
Attachment #684837 - Flags: review?(dtownsend+bugmail) → review+
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c615b961c7b4
Target Milestone: --- → mozilla20

Comment 9

4 years ago
https://hg.mozilla.org/mozilla-central/rev/c615b961c7b4
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.