Bug 1536744 Comment 1 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
So I've got a very unpolished set of patches that moves the creation of most protocol types to NS_NewURIOnAnyThread.
However, I don't think we can work around the protocol handlers for resource and moz-extension URIs.
So the strategy would be to make those protocol handlers (just .newURI() actually) threadsafe. This would be accomplished by making SubstitutingProtocolHandler use NS_INLINE_DECL_THREADSAFE_REFCOUNTING, then making sure that all member variables of SubstitutingProtocolHandler, nsResProtocolHandler, ExtensionProtocolHandler that are touched by newURI be accessed under a lock (when read off main thread or modified. Reading them on the main thread doesn't really need to lock them if they can only be changed on the main thread).

nsResProtocolHandler::ResolveSpecialCases
- `nsResProtocolHandler::mAppURI`
- `nsResProtocolHandler::mGREURI`
- `nsResProtocolHandler::mApkURI`

ExtensionProtocolHandler::ResolveSpecialCases ->
SubstitutingProtocolHandler::HasSubstitution
- `SubstitutingProtocolHandler::mSubstitutions`

ExtensionProtocolHandler::ResolveSpecialCases ->
ExtensionPolicyService::GetGeneratedBackgroundPageUrl ->
- `ExtensionPolicyService::mExtensionHosts`
- maybe `WebExtensionPolicy::mBackgroundScripts` - but it doesn't seem to change after the constructor, so this may be OK.
SubstitutingProtocolHandler::GetSubstitution

- `SubstitutingProtocolHandler::mSubstitutions`
Revision 1 by
on 
So I've got a very unpolished set of patches that moves the creation of most protocol types to NS_NewURIOnAnyThread.
However, I don't think we can work around the protocol handlers for resource and moz-extension URIs.
So the strategy would be to make those protocol handlers (just .newURI() actually) threadsafe. This would be accomplished by making SubstitutingProtocolHandler use NS_INLINE_DECL_THREADSAFE_REFCOUNTING, then making sure that all member variables of SubstitutingProtocolHandler, nsResProtocolHandler, ExtensionProtocolHandler that are touched by newURI be accessed under a lock (when read off main thread or modified. Reading them on the main thread doesn't really need to lock them if they can only be changed on the main thread).

nsResProtocolHandler::ResolveSpecialCases
- `nsResProtocolHandler::mAppURI`
- `nsResProtocolHandler::mGREURI`
- `nsResProtocolHandler::mApkURI`

ExtensionProtocolHandler::ResolveSpecialCases ->
SubstitutingProtocolHandler::HasSubstitution
- `SubstitutingProtocolHandler::mSubstitutions`

ExtensionProtocolHandler::ResolveSpecialCases ->
ExtensionPolicyService::GetGeneratedBackgroundPageUrl ->
- `ExtensionPolicyService::mExtensionHosts` ( ExtensionPolicyService can't be made thread safe )
- maybe `WebExtensionPolicy::mBackgroundScripts` - but it doesn't seem to change after the constructor, so this may be OK.
SubstitutingProtocolHandler::GetSubstitution

- `SubstitutingProtocolHandler::mSubstitutions`
Revision 2 by
on 
So I've got a very unpolished set of patches that moves the creation of most protocol types to NS_NewURIOnAnyThread.
However, I don't think we can work around the protocol handlers for resource and moz-extension URIs.
So the strategy would be to make those protocol handlers (just .newURI() actually) threadsafe. This would be accomplished by making SubstitutingProtocolHandler use NS_INLINE_DECL_THREADSAFE_REFCOUNTING, then making sure that all member variables of SubstitutingProtocolHandler, nsResProtocolHandler, ExtensionProtocolHandler that are touched by newURI be accessed under a lock (when read off main thread or modified. Reading them on the main thread doesn't really need to lock them if they can only be changed on the main thread).

nsResProtocolHandler::ResolveSpecialCases
- `nsResProtocolHandler::mAppURI`
- `nsResProtocolHandler::mGREURI`
- `nsResProtocolHandler::mApkURI`

ExtensionProtocolHandler::ResolveSpecialCases ->
SubstitutingProtocolHandler::HasSubstitution
- `SubstitutingProtocolHandler::mSubstitutions`

ExtensionProtocolHandler::ResolveSpecialCases ->
ExtensionPolicyService::GetGeneratedBackgroundPageUrl ->
- `ExtensionPolicyService::mExtensionHosts`
- maybe `WebExtensionPolicy::mBackgroundScripts` - but it doesn't seem to change after the constructor, so this may be OK.
SubstitutingProtocolHandler::GetSubstitution

- `SubstitutingProtocolHandler::mSubstitutions`

Back to Bug 1536744 Comment 1