Bug 562878 switched the "machine name" to use "oscpu", which shows up as "Mac OS 10.6" but also "Linux armv7l" for Nexus One. I was aiming to get "Android", and seems like that's available from "platform", but that shows up as "MacIntel". Ideally we can show things like "Nexus One" or "MacBook Pro", etc (not sure what Windows equivalent would be.)

Hardware device names can be accessed through Java on Android; I don't know if other platforms have equivalent APIs. http://developer.android.com/reference/android/os/Build.html#MODEL

For OS X, I found this code.. http://www.cocoadev.com/index.pl?MacintoshModels But that seems to need an explicit list of internal model id to some string. But maybe we just want "machine_name" instead of "machine_model": ~$ system_profiler -xml SPHardwareDataType | grep -A1 machine_ <key>machine_model</key> <string>MacBook5,1</string> <key>machine_name</key> <string>MacBook</string>

This is a minimal prototype; not suitable for checkin. I'm experimenting with using js-ctypes to call Android Java code through our C++ bridge without XPCOM. If we actually want to use this approach, we would need to: - Make the C++ bridge code as generic as possible, to avoid writing new code for every Java function we call. - Create some helper objects to reduce boilerplate in the JavaScript code. - Move the C bridge functions out of libxul.so into a separate shared library, to reduce startup perf impact of exported symbols. Also, there is a bug in this code - sometimes the string returned from GetDeviceModel has garbage characters appended, as though GetStringUTFLength is returning the wrong result. I have no idea yet why this happens.

The System Info service (@mozilla.org/system-info;1) already has code to do this on Maemo, and it looks like the right place to add the code for Android too: http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsSystemInfo.cpp

The "device" property is currently implemented on Maemo only.

Moving to Fennec:General so I can set the blocking-fennec flag.

Comment on attachment 473074 [details] [diff] [review] part 1: Use nsSystemInfo to get device name >+++ b/services/sync/modules/engines/clients.js Wed Sep 08 08:58:35 2010 -0700 >+ let system = Cc["@mozilla.org/system-info;1"] >+ .getService(Ci.nsIPropertyBag2).get("device") || You can just use Svc.SysInfo.get("device") [it was originally added for .get("host") when we used hostnames as part of the client id.]

Comment on attachment 473074 [details] [diff] [review] part 1: Use nsSystemInfo to get device name What Mardak says :). Looks good otherwise. Please note that this should land in fx-sync to avoid diverging codebases. (We periodically merge fx-sync to m-c).

Part 1 pushed to fx-sync: http://hg.mozilla.org/services/fx-sync/rev/7a0e2d0b4366

Comment on attachment 473076 [details] [diff] [review] part 2: implement "device" property for Android >+ nsString str; >+ int len = env->GetStringLength(s); >+ str.SetLength(len); >+ env->GetStringRegion(s, 0, len, str.BeginWriting()); why not just use nsJNIString? >-#endif >+#endif >+ >+#ifdef ANDROID >+ nsString str = mozilla::AndroidBridge::GetStaticField("android/os/Build", "MODEL"); >+ SetPropertyAsAString(NS_ConvertASCIItoUTF16("device"), str); >+#endif This can only be called from the chrome process, please either add a check for that or if your sure that shouldn't ever happen an assertion.

(In reply to comment #11 > why not just use nsJNIString? Oh, handy. Done. > This can only be called from the chrome process, please either add a check for > that or if your sure that shouldn't ever happen an assertion. Added a check. Also did some minor cleanup. Re-requesting review for the changes.

Comment on attachment 473221 [details] [diff] [review] part 2 (v2): implement "device" property for Android >+nsString AndroidBridge::GetStaticField(const char *className, const char *fieldName) this should be returned as an out param. Sorry I didn't catch that the first time.

Use an outparam, and add error checking.

Comment on attachment 473544 [details] [diff] [review] part 2 (v3): implement "device" property for Android >+PRBool >+AndroidBridge::GetStaticField(const char *className, const char *fieldName, nsAString &result) >+{ >+ jclass cls = (jclass) mJNIEnv->NewGlobalRef(mJNIEnv->FindClass(className)); >+ if (!cls) >+ return PR_FALSE; >+ >+ jfieldID field = mJNIEnv->GetStaticFieldID(cls, fieldName, "Ljava/lang/String;"); >+ if (!field) >+ return PR_FALSE; >+ >+ jstring jstr = (jstring) mJNIEnv->GetStaticObjectField(cls, field); >+ if (!jstr) >+ return PR_FALSE; >+ >+ result.Assign(nsJNIString(jstr)); >+ return PR_TRUE; >+} >+ You need an AutoLocalJNIFrame here instead of grabbing a global reference. (the code in the bridge initialization is called from Java so there's already a local frame) Also, I suggest naming this GetStaticStringField and use bool for the return value. >@@ -118,17 +122,25 @@ nsSystemInfo::Init() > break; > } > } > } > if (line) > free(line); > fclose(fp); > } >-#endif >+#endif >+ >+#ifdef ANDROID >+ if (mozilla::AndroidBridge::Bridge()) { >+ nsString str; nsAutoString, unless you believe your string will be over 63 characters long most of the time. >+ if (mozilla::AndroidBridge::Bridge()->GetStaticField("android/os/Build", "MODEL", str)) >+ SetPropertyAsAString(NS_ConvertASCIItoUTF16("device"), str); NS_LITERAL_STRING

Addresses requests from comment 15.

Android patch pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/753ddb918cd5 I guess this issue can be marked resolved when fx-sync is merged to m-c. We can enter followup bugs if we want to do similar things on other platforms (Mac, Windows...).

Actually we resolve once we land in fx-sync. I marked this blocking the next merge (bug 594506).