Closed Bug 778608 Opened 8 years ago Closed 8 years ago
Browsing .jsm to toolkit
Other apps (mobile, b2g, webrt) will likely want to be using the code in SafeBrowsing.jsm. So let's move it to Toolkit. This leaves very little stuff in in browser/components/safebrowsing, so I've folded it into browser/base/content. This is on top of the patch in bug 778606, and is not yet tested. ;-)
Note to self, after this we should probably move Bugzilla's Firefox::PhishingProtection component over to Toolkit.
Oops, forgot to fix a jar.mn path, but it seems to be working fine now.
Oops again, tiny fix to makefiles.sh.
Fixed few other small mistakes. Mostly-green in https://tbpl.mozilla.org/?tree=Try&rev=dbacbd4d62f4 Fixed the M(oth) problem: https://tbpl.mozilla.org/?tree=Try&rev=028530f8507a
Attachment #648582 - Attachment is patch: true
Comment on attachment 648582 [details] [diff] [review] Patch v.4 It really pains me to add more tests to browser/base, particularly well-grouped tests that could live somewhere else - a browser chrome run of browser-base takes way too long already. I guess I don't have a better suggestion at the moment, though :/
Attachment #648582 - Flags: review?(gavin.sharp) → review+
This patch caused "Timed out while waiting for: test #6 succesful finish" failures in browser_webconsole_bug_595934_message_categories.js on win debug; exactly like in your second try push.
Exciting. I can't reproduce on a local win-debug build, nor is it at all obvious how that test could possibly be affected by this patch! Time to debug via Try, I guess. :(
The test (referenced in comment 7) is loading an invalid XML page, and watching for "no element found" to be logged in the error console via a console observer. So far it appears that the test is successfully loading the page, but the parsing goop simply isn't logging this message. So, still baffled as to what's happening.
Comment on attachment 648582 [details] [diff] [review] Patch v.4 Review of attachment 648582 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/url-classifier/Makefile.in @@ +20,5 @@ > +# Normally the "client ID" sent in updates is appinfo.name, but for > +# official Firefox releases from Mozilla we use a special identifier. > +ifdef MOZILLA_OFFICIAL > +ifdef MOZ_PHOENIX > +DEFINES += -DUSE_HISTORIC_SAFEBROWSING_ID=1 Could we avoid the MOZ_PHOENIX, or maybe move it to a branding prefs file? (probably as a separate bug) ... it tends to be a pain when we have app specific ifdefs in core (e.g. for xulrunner style builds etc). It'd also avoid the need for preprocessing the file.
As pointed out here https://bugzilla.mozilla.org/show_bug.cgi?id=741808#c4 there's also issues with this define for Fennec.
A while back the thought occurred to me that the bizarre test failures this triggered before were likely just test timing issues. I trimmed down the patch to leave the tests in-place, and that's passed a few try runs at different times. I keep forgetting to land it, though, so I'll do so now. Will file followup to move tests and finish debugging (if it's still reproducible).
Attachment #648582 - Attachment is obsolete: true
Pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/70d9ba6b84f8 Filed bug 806757 to investigate moving the tests.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.