1061112 - test_app_rep.js fails when "browser.safebrowsing.downloads.enabled" is false or not set

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

Not quite sure when this was introduced, however, it is currently failing on Thunderbird:

17:49:30  WARNING -  TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js | 2147549183 == 2147746065 - See following stack:
17:49:30     INFO -  /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js:onComplete:145
17:49:30     INFO -  /builds/slave/test/build/tests/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js:test_nullSourceURI:141

2147746065 is NS_ERROR_NOT_AVAILABLE, and digging into the code reveals that the "browser.safebrowsing.downloads.enabled" not having a value in Thunderbird, causes it to return this value.

Setting the pref in the test fixes the issue.

Comment 2

[:Paolo Amadini]

Comment on attachment 8482157 [details] [diff] [review]
The fix

Seems bug 1057848 is related, but didn't look in detail. Redirecting to mmc.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8482157 [details] [diff] [review]
The fix

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

Given that Thunderbird doesn't have this pref set, I assume it can't use the functionality yet?

Comment 4

[Mark Banner (:standard8)]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #3)
> Given that Thunderbird doesn't have this pref set, I assume it can't use the
> functionality yet?

We haven't looked at what is needed for porting it.

Comment 5

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bed5e74a36b9

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8482157 [details] [diff] [review]
The fix

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

You may want to apply the same fix to test_app_rep_windows.js in the same directory.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix for windows

For windows, "urlclassifier.download[Block|Allow]Table" are also needed.

try server results:
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=e0c42357b44c

Comment 9

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8486225 [details] [diff] [review]
Fix for windows

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

Thanks, Hiro! Just a couple minor fixes.

::: toolkit/components/downloads/test/unit/test_app_rep_windows.js
@@ +192,5 @@
> +  Services.prefs.setCharPref("urlclassifier.downloadAllowTable",
> +                             "goog-downloadwhite-digest256");
> +  do_register_cleanup(function() {
> +    Services.prefs.clearUserPref("urlclassifier.downloadBlockTable");
> +    Services.prefs.clearUserPref("urlclassifier.downloadAllowTable");

There should only be one do_register_cleanup. Can you move these above to the existing function? Similarly, can you move all of the pref setting before the test starts together?

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix for windows v2

Moved those lines.

Comment 11

[Mark Banner (:standard8)]

Thanks Hiro,

https://hg.mozilla.org/integration/mozilla-inbound/rev/50e9d1718ee7

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/50e9d1718ee7

Comment 13

[Mark Banner (:standard8)]

I pushed these to aurora to fix the bustage there as well, with a=test-only.

https://hg.mozilla.org/releases/mozilla-aurora/rev/c42ce91dedaa
https://hg.mozilla.org/releases/mozilla-aurora/rev/7d8ceef4fd5f