1097222 - Extend desktop client record format

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

Desktop portion of Bug 1097218. See that bug for spec details.

Comment 1

[Richard Newman [:rnewman]]

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

Comment 2

[Richard Newman [:rnewman]]

Attachment: Extend client record format on desktop. v1

This should do the trick for now.

Comment 3

[Richard Newman [:rnewman]]

Attachment: Extend client record format on desktop. v2

Now with one fewer error!

Comment 4

[Mark Hammond [:markh] [:mhammond]]

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.

Comment 5

[Richard Newman [:rnewman]]

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

Comment 6

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/0b6ef3a5b009

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0b6ef3a5b009

Comment 8

[Richard Newman [:rnewman]]

ni me for testing and uplift consideration.