390324 - url-classifier needs to make sure psm is initialized on the main thread

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Dave Camp (:dcamp)]

Attachment: make sure stuff needed by cryptohash is initialized in the main thread.

This is causing assertion failures in the test cases (bug 387196 comment 21), among other big scary potential problems.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Arguably, PSM should deal with this situation...

Comment 2

[Kai Engert [:KaiE:]]

Dave, why is it necessary to init PSM/NSS on the main thread?
Is it because of nsNSSComponent::RegisterObservers?

I had assumed most services should be ok to be accessed from any thread. I wasn't aware our multithreaded abilities are so limited.

Boris, you say that PSM should deal with the init on the right thread?
What should PSM do?
Should it proxy all calls to observer service to the main thread?

Comment 3

[Dave Camp (:dcamp)]

(In reply to comment #2)
> Dave, why is it necessary to init PSM/NSS on the main thread?
> Is it because of nsNSSComponent::RegisterObservers?

That's the problem I was having, I didn't look closely to see if there were others.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 274644 [details] [diff] [review]
make sure stuff needed by cryptohash is initialized in the main thread.

dcamp's out for the next few days, and because this prevents me from doing a complete run of xpcshell tests, I'm going to steal this from him to get it in the tree.

This patch fixes an assertion preventing at least me (and probably a fair number of other people) from running xpcshell tests in debug builds.

Comment 5

[Dave Camp (:dcamp)]

Checking in toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v  <--  nsUrlClassifierDBService.cpp
new revision: 1.28; previous revision: 1.27
done

(I don't believe toolkit patches need approval yet)

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 274644 [details] [diff] [review]
make sure stuff needed by cryptohash is initialized in the main thread.

a1.9=dbaron

(Does this force PSM to be loaded before we bring up the browser UI?  Was that the case before?  I know for a while we got it out of the startup-crtical path.)

Comment 7

[Dave Camp (:dcamp)]

The url classifier isn't initialized in the startup-critical path (if it were the bug would have caused assertion failures on startup in the tests)

Comment 8

[Boris Zbarsky [:bzbarsky]]

> The url classifier isn't initialized in the startup-critical path

That's not true, if the homepage is loaded from HTTP.  Conveniently, the tests don't test this case.  I filed bug 462807 on the fact that we init NSS during startup as a result.