The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla16

Status

()

Core
DOM
--
enhancement
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

Trunk
mozilla16
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

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.

Updated

5 years ago
Blocks: 757351
Blocks: 769312
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.
Created attachment 637897 [details] [diff] [review]
Exposing OS.Constants as a xpcom component
Assignee: nobody → dteller
Attachment #637897 - Flags: review?(khuey)
Created attachment 637899 [details] [diff] [review]
The module
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
Created attachment 637900 [details] [diff] [review]
The module
Attachment #637900 - Flags: review?(khuey)
Created attachment 637901 [details] [diff] [review]
Companion testsuite
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 - Flags: review?(khuey) → review+
Depends on: 769658
Created attachment 638679 [details] [diff] [review]
Companion testsuite
Attachment #637901 - Attachment is obsolete: true
Attachment #638679 - Flags: review+
Created attachment 638680 [details] [diff] [review]
The module
Attachment #637900 - Attachment is obsolete: true
Attachment #638680 - Flags: review+
Created attachment 638682 [details] [diff] [review]
Exposing OS.Constants as a xpcom component
Attachment #637897 - Attachment is obsolete: true
Attachment #638682 - Flags: review+
Created attachment 638694 [details] [diff] [review]
Exposing OS.Constants as a xpcom component
Attachment #638682 - Attachment is obsolete: true
Attachment #638694 - Flags: review+
Keywords: checkin-needed
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
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/f9fb64b00563
https://hg.mozilla.org/mozilla-central/rev/460c8abe0a91
https://hg.mozilla.org/mozilla-central/rev/e997600270c2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Version: unspecified → Trunk
Blocks: 771088
Component: DOM: Other → DOM
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.