Last Comment Bug 750178 - [OS.File] Export OS.Constants to the main thread
: [OS.File] Export OS.Constants to the main thread
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla16
Assigned To: David Teller [:Yoric] (please use "needinfo")
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 739740 747872 769658
Blocks: 757351 769312 771088
  Show dependency treegraph
 
Reported: 2012-04-30 01:09 PDT by David Teller [:Yoric] (please use "needinfo")
Modified: 2013-04-04 13:52 PDT (History)
3 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Exposing OS.Constants as a xpcom component (6.02 KB, patch)
2012-06-29 07:17 PDT, David Teller [:Yoric] (please use "needinfo")
khuey: review+
Details | Diff | Splinter Review
The module (5.70 KB, patch)
2012-06-29 07:18 PDT, David Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
The module (4.18 KB, patch)
2012-06-29 07:24 PDT, David Teller [:Yoric] (please use "needinfo")
khuey: review+
Details | Diff | Splinter Review
Companion testsuite (1.67 KB, patch)
2012-06-29 07:25 PDT, David Teller [:Yoric] (please use "needinfo")
khuey: review+
Details | Diff | Splinter Review
Companion testsuite (1.68 KB, patch)
2012-07-03 06:10 PDT, David Teller [:Yoric] (please use "needinfo")
dteller: review+
Details | Diff | Splinter Review
The module (4.18 KB, patch)
2012-07-03 06:11 PDT, David Teller [:Yoric] (please use "needinfo")
dteller: review+
Details | Diff | Splinter Review
Exposing OS.Constants as a xpcom component (7.01 KB, patch)
2012-07-03 06:12 PDT, David Teller [:Yoric] (please use "needinfo")
dteller: review+
Details | Diff | Splinter Review
Exposing OS.Constants as a xpcom component (7.72 KB, patch)
2012-07-03 06:56 PDT, David Teller [:Yoric] (please use "needinfo")
dteller: review+
Details | Diff | Splinter Review

Description David Teller [:Yoric] (please use "needinfo") 2012-04-30 01:09:06 PDT
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] (please use "needinfo") 2012-06-29 07:14:56 PDT
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] (please use "needinfo") 2012-06-29 07:17:30 PDT
Created attachment 637897 [details] [diff] [review]
Exposing OS.Constants as a xpcom component
Comment 3 David Teller [:Yoric] (please use "needinfo") 2012-06-29 07:18:57 PDT
Created attachment 637899 [details] [diff] [review]
The module
Comment 4 David Teller [:Yoric] (please use "needinfo") 2012-06-29 07:21:53 PDT
Comment on attachment 637899 [details] [diff] [review]
The module

Sorry, I merged two patches by accident.
Comment 5 David Teller [:Yoric] (please use "needinfo") 2012-06-29 07:24:47 PDT
Created attachment 637900 [details] [diff] [review]
The module
Comment 6 David Teller [:Yoric] (please use "needinfo") 2012-06-29 07:25:38 PDT
Created attachment 637901 [details] [diff] [review]
Companion testsuite
Comment 7 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-02 10:20:37 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-07-02 10:22:23 PDT
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] (please use "needinfo") 2012-07-03 06:10:29 PDT
Created attachment 638679 [details] [diff] [review]
Companion testsuite
Comment 10 David Teller [:Yoric] (please use "needinfo") 2012-07-03 06:11:30 PDT
Created attachment 638680 [details] [diff] [review]
The module
Comment 11 David Teller [:Yoric] (please use "needinfo") 2012-07-03 06:12:08 PDT
Created attachment 638682 [details] [diff] [review]
Exposing OS.Constants as a xpcom component
Comment 12 David Teller [:Yoric] (please use "needinfo") 2012-07-03 06:56:34 PDT
Created attachment 638694 [details] [diff] [review]
Exposing OS.Constants as a xpcom component

Note You need to log in before you can comment on or make changes to this bug.