476711 - Native App support for windows ce

Status: VERIFIED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: changes to wide API calls and ifdef's out DDE support

This basically requires switching to wide character windows apis and ifdef'ing out the DDE code that doesn't exist on windows ce.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

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...

Comment 3

[Robert Strong (they/them - no direct email)]

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

Comment 4

[Benjamin Smedberg]

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.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

http://hg.mozilla.org/mozilla-central/rev/38bba72321ca

Comment 6

[Brian Crowder]

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?

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

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.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/ab2f5ea8ddcd

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 360359 [details] [diff] [review]
changes to wide API calls and ifdef's out DDE support

a191=beltzner

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/857883de3a2c

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

... and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f2971773de70

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

verified with 20091029 build on my htc touch pro and omnia 2