Closed Bug 750178 Opened 8 years ago Closed 8 years ago

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

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: Yoric, Assigned: Yoric)

References

Details

Attachments

(3 files, 5 obsolete files)

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.
Blocks: 757351
Summary: Export libc/windows js-ctypes constants to the main thread → [OS.File] Export OS.Constants to the main thread
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.
Assignee: nobody → dteller
Attachment #637897 - Flags: review?(khuey)
Attached patch The module (obsolete) — Splinter Review
Attachment #637899 - Flags: review?(khuey)
Comment on attachment 637899 [details] [diff] [review]
The module

Sorry, I merged two patches by accident.
Attachment #637899 - Attachment is obsolete: true
Attachment #637899 - Flags: review?(khuey)
Severity: normal → enhancement
Component: js-ctypes → DOM: Other
QA Contact: js-ctypes → general
Attached patch The module (obsolete) — Splinter Review
Attachment #637900 - Flags: review?(khuey)
Attached patch Companion testsuite (obsolete) — Splinter Review
Attachment #637901 - Flags: review?(khuey)
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.
Attachment #637897 - Flags: review?(khuey) → review+
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.
Attachment #637900 - Flags: review?(khuey) → review+
Attachment #637901 - Attachment is obsolete: true
Attachment #638679 - Flags: review+
Attached patch The moduleSplinter Review
Attachment #637900 - Attachment is obsolete: true
Attachment #638680 - Flags: review+
Attachment #637897 - Attachment is obsolete: true
Attachment #638682 - Flags: review+
Version: unspecified → Trunk
Component: DOM: Other → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.