901198 - test_app_rep.js fails for applications which have safe browsing disabled (aka Permanent orange | TEST-UNEXPECTED-FAIL | false = true)

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

Since bug 837199 landed (but only just detected due to other issues), on Thunderbird, we've been getting failures in test_app_rep.js due to safe browsing being disabled there:

TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js | false == true - See following stack:
JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js :: onComplete :: line 68
JS frame :: /Users/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/downloads/test/unit/test_app_rep.js :: test_shouldBlock :: line 67

The fix is simple - set the preferences at the start of the test to ensure that safe browsing (the element under test) is enabled.

Comment 1

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

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

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

Thanks for catching this and I'm sorry I missed it the first time.

::: toolkit/components/downloads/test/unit/test_app_rep.js
@@ +21,5 @@
>                               "http://localhost:4444/download");
> +  // Ensure safebrowsing is enabled for this test, even if the app
> +  // doesn't have it enabled.
> +  Services.prefs.setBoolPref("browser.safebrowsing.enabled", true);
> +  Services.prefs.setBoolPref("browser.safebrowsing.malware.enabled", true);

You only need to set this second one. The first one (browser.safebrowsing.enabled) is used for phishing checks only.

Comment 2

[:Paolo Amadini]

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

What Monica said :-)

Plus, it seems worth calling Services.pref.clearUserPref on that particular
preference, inside a do_register_cleanup function. Feel free to land a patch
including this change.

Comment 3

[Mark Banner (:standard8)]

Thanks to you both for the comments:

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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

Comment 5

[Mark Banner (:standard8)]

Pushed to aurora as a test-only fix:

https://hg.mozilla.org/releases/mozilla-aurora/rev/fbfcc7ebcf2e

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/760948237ca1