1403473 - about:url-classifier JS debug error for hashcompleter

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[Thomas Nguyen (:tnguyen)]

Sometimes, it does not work for the first time, we have to disable then enable js log to show js log.
The reason is hashcompleter may be late initialized when it's really necessary.

Comment 1

[Thomas Nguyen (:tnguyen)]

Attachment: Bug 1403473 - Init loggingEnable for hashcompleter

Review commit: https://reviewboard.mozilla.org/r/183938/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183938/

Comment 2

[Dimi Lee [:dimi]]

Comment on attachment 8912610 [details]
Bug 1403473 - Init loggingEnable for hashcompleter

https://reviewboard.mozilla.org/r/183938/#review189490

::: toolkit/components/url-classifier/nsUrlClassifierHashCompleter.js:188
(Diff revision 1)
>    // A map of gethash URLs to next gethash time in miliseconds
>    this._nextGethashTimeMs = {};
>  
>    Services.obs.addObserver(this, "quit-application");
> +
> +  loggingEnabled = Services.prefs.getBoolPref(PREF_DEBUG_ENABLED);

could we just put this one with 'loggingEnabled' declaration ?

Comment 3

[Thomas Nguyen (:tnguyen)]

Comment on attachment 8912610 [details]
Bug 1403473 - Init loggingEnable for hashcompleter

https://reviewboard.mozilla.org/r/183938/#review189490

> could we just put this one with 'loggingEnabled' declaration ?

Thanks Dimi. loggingEnabled has global scope and I don't know if it works or we may have side effect (possible on startup). It is safe to put it in hashcompleter initilization

Comment 4

[Thomas Nguyen (:tnguyen)]

Comment on attachment 8912610 [details]
Bug 1403473 - Init loggingEnable for hashcompleter

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183938/diff/1-2/

Comment 5

[Thomas Nguyen (:tnguyen)]

Comment on attachment 8912610 [details]
Bug 1403473 - Init loggingEnable for hashcompleter

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183938/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a9862245514
Init loggingEnable for hashcompleter r=dimi

Comment 7

[Thomas Nguyen (:tnguyen)]

[Tracking Requested - why for this release]:

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/0a9862245514

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Thomas, could you please nominate a patch for uplift to 57? Thanks!

Comment 10

[Thomas Nguyen (:tnguyen)]

Attachment: Beta uplift

Approval Request Comment
[Feature/Bug causing the regression]: 1297614
[User impact if declined]: Sometimes the JS enable debug will not work for hashcompleter
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
1. Run FF from terminal (for example mac osx /Applications/FirefoxNightly.app/Contents/MacOS/firefox-bin)
2. Enable browser.dom.window.dump.enabled and go to about:url-classifier
3. Update google4 list and click enable JS log
4. Access http://testsafebrowsing.appspot.com/s/phishing.html to see if you could see the page is blocked and check the terminal to see if there's "hashcompleter" keyword log
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: Initialize variable 
[String changes made/needed]: No

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8913585 [details] [diff] [review]
Beta uplift

Help debug, taking it.
Should be in 57b5

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e4a0c97b413f

Comment 13

[Vlad Bacia-Mociran [:VladB]]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20171005100211

This issue has been verified on latest Firefox Nightly Build ID: 20171005100211 on Windows 8.1 x64, Mac OS 10.12 and Ubuntu 14.04. The website is blocked and "hashcompleter" keyword is displayed in the terminal.

This issue has been also verified on latest Firefox Beta 57.0b5 (Build ID: 20171002181526).