750178 - [OS.File] Export OS.Constants to the main thread

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[David Teller [:Yoric] - still alive but not very active]

At the moment, libc constants/windows constants are exported only to Chrome Workers. Once we start working on main thread implementation of OS.File, we will need these constants also on the main thread.

The most transparent way is probably to place the initialization of these constants in the same XPConnect call as the initialization of of js-ctypes.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Note a subtletly: OS.Constants now uses a pinch of thread-sensitive code (see bug 763848). We need to ensure that we do not break anything.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Attachment: Exposing OS.Constants as a xpcom component

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Attachment: The module

Comment 4

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 637899 [details] [diff] [review]
The module

Sorry, I merged two patches by accident.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: The module

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Companion testsuite

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 637897 [details] [diff] [review]
Exposing OS.Constants as a xpcom component

Review of attachment 637897 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/IOSFileConstantsService.idl
@@ +6,5 @@
> +
> +#include "nsISupports.idl"
> +
> +[scriptable, uuid(d6dd239f-34d6-4b34-baa1-f69ab4a20bc4)]
> +interface IOSFileConstantsService: nsISupports

nsIOSFileConstantsService

::: dom/system/OSFileConstants.cpp
@@ +86,5 @@
>    gLibDirectory = new nsString();
>    return libDir->GetPath(*gLibDirectory);
>  }
>  
>  nsresult CleanupOSFileConstants()

There's no need for this to return an nsresult anymore.  Just make it void.

@@ +470,5 @@
>  
> +NS_IMPL_ISUPPORTS1(OSFileConstantsService, IOSFileConstantsService)
> +
> +OSFileConstantsService::OSFileConstantsService()
> +{

Add an NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); to the ctor.

@@ +476,5 @@
> +
> +OSFileConstantsService::~OSFileConstantsService()
> +{
> +  mozilla::DebugOnly<nsresult> rv = mozilla::CleanupOSFileConstants();
> +  MOZ_ASSERT(NS_SUCCEEDED(rv));

And then this assertion is unnecessary.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 637900 [details] [diff] [review]
The module

Review of attachment 637900 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/build/nsLayoutModule.cpp
@@ +1041,5 @@
>    { &kSMS_SERVICE_CID, false, NULL, nsISmsServiceConstructor },
>    { &kSMS_DATABASE_SERVICE_CID, false, NULL, nsISmsDatabaseServiceConstructor },
>    { &kSMS_REQUEST_MANAGER_CID, false, NULL, SmsRequestManagerConstructor },
>    { &kNS_POWERMANAGERSERVICE_CID, false, NULL, nsIPowerManagerServiceConstructor },
> +  { &kOSFILECONSTANTSSERVICE_CID, false, NULL, OSFileConstantsServiceConstructor },

Make the second argument true.  That indicates whether or not it's a service.  It's unenforced (as you can tell by all the above services) but it doesn't hurt to set it correctly.

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Attachment: Companion testsuite

Comment 10

[David Teller [:Yoric] - still alive but not very active]

Attachment: The module

Comment 11

[David Teller [:Yoric] - still alive but not very active]

Attachment: Exposing OS.Constants as a xpcom component

Comment 12

[David Teller [:Yoric] - still alive but not very active]

Attachment: Exposing OS.Constants as a xpcom component

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9fb64b00563
https://hg.mozilla.org/integration/mozilla-inbound/rev/460c8abe0a91
https://hg.mozilla.org/integration/mozilla-inbound/rev/e997600270c2

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f9fb64b00563
https://hg.mozilla.org/mozilla-central/rev/460c8abe0a91
https://hg.mozilla.org/mozilla-central/rev/e997600270c2