Closed Bug 390324 Opened 15 years ago Closed 15 years ago

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

Categories

(Toolkit :: Safe Browsing, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dcamp, Assigned: dcamp)

References

Details

Attachments

(1 file)

This is causing assertion failures in the test cases (bug 387196 comment 21), among other big scary potential problems.
Attachment #274644 - Flags: review?(tony)
Arguably, PSM should deal with this situation...
Blocks: 387196
Attachment #274644 - Flags: review?(tony) → review+
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?
(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 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.
Attachment #274644 - Flags: approval1.9?
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)
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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.)
Attachment #274644 - Flags: approval1.9? → approval1.9+
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)
Depends on: 462807
> 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.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.