Closed Bug 1097222 Opened 10 years ago Closed 10 years ago

Extend desktop client record format

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

Attachments

(1 file, 1 obsolete file)

Desktop portion of Bug 1097218. See that bug for spec details.
To get hardware details on desktop is (obviously) OS-dependent. It's also non-trivial; on the Mac, this code

#import <Foundation/Foundation.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/sysctl.h>

int main(int argc, const char * argv[]) {
    @autoreleasepool {
        size_t len = 0;
        sysctlbyname("hw.model", NULL, &len, NULL, 0);
        
        if (len)
        {
            char *model = malloc(len*sizeof(char));
            sysctlbyname("hw.model", model, &len, NULL, 0);
            NSString *model_ns = [NSString stringWithUTF8String:model];
            free(model);
            NSLog(@"FOO: %@", model_ns);
        }
    }
    return 0;
}

will print "MacBookPro10,1" rather than a human-readable string.

We could make a network request to Apple with the serial number, or we could call system_profiler and parse the output.

http://stackoverflow.com/questions/8058151/how-does-system-profiler-retrieve-the-full-mac-hardware-identifier


It looks like the Windows option is:

http://msdn.microsoft.com/en-us/library/windows/desktop/ms724423%28v=vs.85%29.aspx

and

http://stackoverflow.com/questions/1013354/how-to-check-the-machine-type-laptop-or-desktop
http://blogs.technet.com/b/heyscriptingguy/archive/2004/09/21/how-can-i-determine-if-a-computer-is-a-laptop-or-a-desktop-machine.aspx
This should do the trick for now.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Now with one fewer error!
Attachment #8523362 - Flags: review?(mhammond)
Attachment #8523361 - Attachment is obsolete: true
Comment on attachment 8523362 [details] [diff] [review]
Extend client record format on desktop. v2

Review of attachment 8523362 [details] [diff] [review]:
-----------------------------------------------------------------

I'd have thought it would be nice for a test to check at least there are non-empty strings for the ones we expect, but that's at your discretion.

::: services/sync/modules/engines/clients.js
@@ +31,5 @@
>  
> +Utils.deferGetSet(ClientsRec,
> +                  "cleartext",
> +                  ["name", "type", "commands",
> +                   "version", "protocols",

I'd have thought you could leave this line on the previous line and and up with 2 lines roughly the same length?

@@ +429,5 @@
> +      record.appPackage = appInfo.ID;
> +      record.application = this.engine.brandName   // "Nightly"
> +
> +      // We can't compute these yet.
> +      // record.device = "";

FWIW, I notice that http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3368 grabs the manufacturer for crash reporting, but yeah, it sounds hard to get this generically in the medium term.
Attachment #8523362 - Flags: review?(mhammond) → review+
> I'd have thought it would be nice for a test to check at least there are
> non-empty strings for the ones we expect, but that's at your discretion.

*grumble grumble*

:D

Yeah, I'll write a test or two before I land.


> I'd have thought you could leave this line on the previous line and and up
> with 2 lines roughly the same length?

This is more a semantic distinction than a line-length thing: version/protocols were the 1.5 semi-optional extension fields, with name/type/commands the original non-optional core set.

I didn't care enough to bother adding piles of comments, but splitting on a notional category felt right to me.


> FWIW, I notice that
> http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.
> cpp#3368 grabs the manufacturer for crash reporting...

Great pointer, thanks!

I'll be filing followups to grab more data when we can, so this will come in handy.
Blocks: 1100722
Blocks: 1100723
https://hg.mozilla.org/mozilla-central/rev/0b6ef3a5b009
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
ni me for testing and uplift consideration.
Flags: needinfo?(rnewman)
Flags: needinfo?(rnewman)
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.