Closed Bug 476711 Opened 11 years ago Closed 11 years ago

Native App support for windows ce

Categories

(Toolkit Graveyard :: XULRunner, defect)

ARM
Windows CE
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: blassey, Assigned: blassey)

References

Details

(Keywords: fixed1.9.1, mobile)

Attachments

(1 file)

This basically requires switching to wide character windows apis and ifdef'ing out the DDE code that doesn't exist on windows ce.
Attachment #360359 - Flags: superreview?(benjamin)
Attachment #360359 - Flags: review?(gavin.sharp)
Flags: blocking-fennec1.0?
Duplicate of this bug: 476710
Attachment #360359 - Flags: review?(gavin.sharp) → review?(benjamin)
Attachment #360359 - Flags: review?(benjamin) → review?(rstrong)
Blocks: 476417
Flags: blocking-fennec1.0? → blocking-fennec1.0+
Attachment #360359 - Flags: review?(rstrong) → review?(robert.bugzilla)
Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

Pretty sure brad meant to request review from :rs...
Attachment #360359 - Flags: review?(robert.bugzilla) → review+
Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

Looks good... I also tested this with Firefox and everything worked as expected.

>diff --git a/toolkit/xre/nsNativeAppSupportWin.cpp b/toolkit/xre/nsNativeAppSupportWin.cpp
>--- a/toolkit/xre/nsNativeAppSupportWin.cpp
>+++ b/toolkit/xre/nsNativeAppSupportWin.cpp
>@@ -164,21 +168,21 @@ struct Mutex {
>     void Unlock() {
>         if ( mHandle && mState == WAIT_OBJECT_0 ) {
> #if MOZ_DEBUG_DDE
>             printf( "Releasing DDE mutex\n" );
> #endif
>             ReleaseMutex( mHandle );
>             mState = -1;
>         }
>     }
> private:
>-    nsCString mName;
>+    nsString mName;
nit: please keep these aligned

>     HANDLE    mHandle;
>     DWORD     mState;

>...
>@@ -450,91 +458,93 @@ NS_CreateNativeAppSupport( nsINativeAppS
>...
>     // Class name: appName + "MessageWindow"
>-    static const char *className() {
>-        static char classNameBuffer[128];
>-        static char *mClassName = 0;
>+    static const PRUnichar *className() {
>+        static PRUnichar classNameBuffer[128];
>+        static PRUnichar *mClassName = 0;
>         if ( !mClassName ) {
>-            ::_snprintf( classNameBuffer,
>+            ::_snwprintf( classNameBuffer,
>                          sizeof classNameBuffer,
>-                         "%s%s",
>-                         gAppData->name,
>-                         "MessageWindow" );
>+                         L"%s%s",
>+                         NS_ConvertUTF8toUTF16(gAppData->name).get(),
>+                         L"MessageWindow" );
nit: please keep these aligned since you are changing all but one line anyways

>@@ -656,47 +666,49 @@ nsNativeAppSupportWin::Start( PRBool *aR
>         *aResult = PR_TRUE;
>         return NS_OK;
>     }
> 
>     nsresult rv = NS_ERROR_FAILURE;
>     *aResult = PR_FALSE;
> 
>     // Grab mutex first.
> 
>     // Build mutex name from app name.
>-    ::_snprintf( mMutexName, sizeof mMutexName, "%s%s%s", MOZ_MUTEX_NAMESPACE,
>+    ::_snwprintf( mMutexName, sizeof mMutexName, L"%s%s%s", MOZ_MUTEX_NAMESPACE,
>                  gAppData->name, MOZ_STARTUP_MUTEX_NAME );
nit: please keep these aligned
Attachment #360359 - Flags: approval1.9.1?
Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

I'm worried that the mutex name may not be compatible with prior versions. It's important that if a user is running FF3.1 and clicks on a FF3 launcher, that it still remotes correctly to FF3.1.

rs says this works, but I'm a little leery of pushing this on to 1.9.1 until it has baked a bit on trunk.
Attachment #360359 - Flags: superreview?(benjamin) → superreview+
http://hg.mozilla.org/mozilla-central/rev/38bba72321ca
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

+            ::_snwprintf( classNameBuffer,
                          sizeof classNameBuffer,

This results in a potentially serious buffer-overflow, since snwprintf expects a count of characters, not a size in bytes.

+    ::_snwprintf( mMutexName, sizeof mMutexName, L"%s%s%s", MOZ_MUTEX_NAMESPACE,

Ditto!

Please eyeball other size calculations I might have missed.  Sorry for late drive-by...  should we backout, or what?
you're right, sorry for missing that.  Just looked at the patch again and I only see the two issues you point out.  I'll push a fix for the them rather than back out.
Assignee: nobody → bugmail
tracking-fennec: --- → ?
Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

a191=beltzner
Attachment #360359 - Flags: approval1.9.1? → approval1.9.1+
Depends on: 487888
verified with 20091029 build on my htc touch pro and omnia 2
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.