461858 - windows mobile minor cleanup of xulrunner

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Doug Turner (:dougt)]

we removed much of the functionality in the windows ce shunt library.  This minor patch allows us to continue to build.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 3

[Benjamin Smedberg]

Comment on attachment 346494 [details] [diff] [review]
patch v.2

>diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp

>@@ -473,7 +475,9 @@ ApplyUpdate(nsIFile *greDir, nsIFile *up
>   chdir(applyToDir.get());
>   execv(updaterPath.get(), argv);
> #elif defined(XP_WIN)
>+#ifndef WINCE
>   _wchdir(applyToDir.get());
>+#endif

You don't put in any replacement function here... I'm sure the update driver isn't going to work without some additional patches, since it uses opens files on paths such as components/nsFoo.js.

Please don't commit this file. I think we should just --disable-updater on WinCE until this can be fixed, and make sure that there's a bug filed about actually fixing it.

>diff --git a/toolkit/xre/nsXREDirProvider.cpp b/toolkit/xre/nsXREDirProvider.cpp

>@@ -916,7 +920,11 @@ nsXREDirProvider::GetUpdateRootDir(nsIFi
>   PRUint32 bufLength = longPath.GetMutableData(&buf, MAXPATHLEN);
>   NS_ENSURE_TRUE(bufLength >= MAXPATHLEN, NS_ERROR_OUT_OF_MEMORY);
> 
>+#ifndef WINCE
>   DWORD len = GetLongPathNameW(appPath.get(), buf, bufLength);
>+#else
>+  DWORD len = 0;
>+#endif

This is a confusing way of writing what you're actually doing. I'd prefer something like:

#ifdef WINCE
  longPath = appPath;
#else
  ... all that GetLongPathName code
#endif

>diff --git a/xulrunner/stub/nsXULStub.cpp b/xulrunner/stub/nsXULStub.cpp
>--- a/xulrunner/stub/nsXULStub.cpp
>+++ b/xulrunner/stub/nsXULStub.cpp
>@@ -260,8 +262,11 @@ main(int argc, char **argv)
>            "%sxulrunner" XPCOM_FILE_PATH_SEPARATOR XPCOM_DLL,
>            iniPath);
> 
>+#ifndef WINCE
>   greFound = (access(greDir, R_OK) == 0);
>-
>+#else
>+  greFound = PR_TRUE;
>+#endif

This code means that WinCE will only ever load xulrunner from the application directory, never a system location. Is this what you really mean to be doing? (I hope not.) If access() isn't implemented on WinCE, we should use whatever other function checks for the existence of a directory.

Comment 4

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

Attachment: updated based on bsmedberg's comments

Comment 5

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

Attachment: switched to use wide version

Comment 6

[Benjamin Smedberg]

Comment on attachment 348073 [details] [diff] [review]
switched to use wide version

>diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp

>diff --git a/xulrunner/stub/nsXULStub.cpp b/xulrunner/stub/nsXULStub.cpp

>+#ifdef WINCE
>+  DWORD fileAttrs = GetFileAttributesW(NS_ConvertUTF8toUTF16(greDir).get());
>+  greFound = fileAttrs != 0xFFFFFFFF && fileAttrs | FILE_ATTRIBUTE_DIRECTORY;

As with the other bug, I think you want the symbolic INVALID_FILE_ATTRIBUTES instead of the magic constant here. r=me with that change.

Comment 7

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

http://hg.mozilla.org/mozilla-central/rev/eae733a929f5

Comment 8

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

Comment on attachment 348073 [details] [diff] [review]
switched to use wide version

a191=beltzner

Comment 9

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8c2574907e89

Comment 10

[Doug Turner (:dougt)]

this landed on the 1.9.1 branch.  adding keyword fixed1.9.1

Comment 11

[Aiko]

+  greFound = fileAttrs != INVALID_FILE_ATTRIBUTES && 
+             fileAttrs | FILE_ATTRIBUTE_DIRECTORY;

fileAttrs | FILE_ATTRIBUTE_DIRECTORY is always true, I guess you meant
fileAttrs & FILE_ATTRIBUTE_DIRECTORY.