Spinoff of bug 552924 comment #19 This is necessary since there will be both a Universal ppc/i386 and a Universal i386/x86_64 distribution and there is no way to distinguish between the two. This is primarily for app update but the blocklist service should also be updated to use this. http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#112 http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#172

I think: 1) The "isUniversal" check should also check for x86-64 binaries, and return true if we have >1 supported arch (ppc, x86, x86_64). 2) That same code should be factored out into a helper method, and a new property on nsIMacUtils should be exposed, something like "universalArchitectures", which, on a universal binary, should return a concatenation of the binaries contained within. So something like "ppc-i386" on our existing UBs and "i386-x86_64" on the new 32/64 binaries.

Might be interesting to know which binary is the one running as well... perhaps have it be the first one?

(In reply to comment #2) > Might be interesting to know which binary is the one running as well... perhaps > have it be the first one? I take that back... AUS will likely want prefer this to be the same string for updates.

That'd be trivial to add if you wanted it, just #ifdef __i386__ str = "i386"; #elifdef __ppc__ str = "ppc" #elifdef __x86_64__ str = "x86_64"; #endif

Comment on attachment 456454 [details] [diff] [review] fix v1.0 Not being that familiar with Mac programming or an xpcom peer I'd prefer only feedback r+'ing this.

Comment on attachment 456454 [details] [diff] [review] fix v1.0 >+ * Returns a string containing a list of architectures delimited >+ * by "-". String will be the same for every matching architecture >+ * set. Would it be useful to explicitly state a stronger property about the ordering of current and new architectures since this is an interface? Perhaps that it is non empty and that ppc > x86 > ppc64 > x86_64 > (any future addition) if the call succeed? >+ if (!::CFNumberGetValue(arch, kCFNumberIntType, &archInt)) { >+ ::CFRelease(archList); >+ return NS_ERROR_FAILURE; >+ } [...] >+ >+ archString.Assign(mBinaryArchs); >+ >+ return (archString.IsEmpty() ? NS_ERROR_FAILURE : NS_OK); In some but not all error cases we assign the empty string to archString. This is inconsistent. >+ CFArrayRef archList = ::CFBundleCopyExecutableArchitectures(mainBundle); >+ if (!archList) { >+ return NS_ERROR_FAILURE; >+ } archList does not get released if 'CFBundleCopyExecutableArchitectures' returns a non-null empty CFArrayRef but that not a problem if we're 100% sure CFBundleCopyExecutableArchitectures will never return an empty array. >+ // The delimiter char in the arch string is '-', so if that character >+ // is in the string we know we have multiple architectures. >+ *aIsUniversalBinary = (archString.Find("-") == 0); Is '(archString.Find("-") == 0)' correct? Perhaps we want '(archString.Find("-") != -1)' or even use FindChar. https://developer.mozilla.org/en/Mozilla_external_string_guide#Finding_Text_Within_Strings

(In reply to comment #7) > >+ CFArrayRef archList = ::CFBundleCopyExecutableArchitectures(mainBundle); > >+ if (!archList) { > >+ return NS_ERROR_FAILURE; > >+ } > > archList does not get released if 'CFBundleCopyExecutableArchitectures' returns > a non-null empty CFArrayRef but that not a problem if we're 100% sure > CFBundleCopyExecutableArchitectures will never return an empty array. Are you sure about that? archList is always released after the loop, and it is released in the loop at the only early return.

ohh you're right, I got confused by the early return.

pushed to mozilla-central http://hg.mozilla.org/mozilla-central/rev/b141a304ad08