chromeos: Use protocol buffers for Contact class.

chromeos: Use protocol buffers for Contact class.

This was formerly a hand-rolled C++ class, but Satoru
pointed out that using protocol buffers would make
serialization to disk much simpler.

BUG=128805
TEST=compiles
TBR=ben@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148868

[Daniel Erat, 2012-07-27 18:44:05]

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact.proto (right):

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact.proto:26: optional bool deleted = 3
[default = false];
(I know that false is the default, but I think that being explicit about
defaults is probably better.)

[Daniel Erat, 2012-07-27 20:04:07]

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact.proto (right):

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact.proto:38: optional bytes png_photo =
10;
I'm still going back and forth on this.  Using a test account, I'm seeing a
photo come down the wire as 2211 bytes (JPEG, presumably) but end up as 15428
bytes after I recompress it with PNG.  I don't want to do my own lossy JPEG
compression after decoding the image, so I'll probably just store the original
untrusted data here and try to make the other end stay performant (e.g. by
decoding within a sandbox the first time the data is needed and caching the
bitmap somewhere).

[satorux1, 2012-07-27 20:46:33]

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact.proto (right):

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact.proto:38: optional bytes png_photo =
10;
On 2012/07/27 20:04:07, Daniel Erat wrote:
> I'm still going back and forth on this.  Using a test account, I'm seeing a
> photo come down the wire as 2211 bytes (JPEG, presumably) but end up as 15428
> bytes after I recompress it with PNG.  I don't want to do my own lossy JPEG
> compression after decoding the image, so I'll probably just store the original
> untrusted data here and try to make the other end stay performant (e.g. by
> decoding within a sandbox the first time the data is needed and caching the
> bitmap somewhere).

Storing the original data makes sense. You might want to name it something like
raw_untrusted_photo, to make it look scary. You might also want to add a new
filed to store the content type (ex. image/jpeg).

[Daniel Erat, 2012-07-27 21:59:01]

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
File chrome/browser/chromeos/contacts/contact.proto (right):

http://codereview.chromium.org/10830052/diff/1007/chrome/browser/chromeos/con...
chrome/browser/chromeos/contacts/contact.proto:38: optional bytes png_photo =
10;
On 2012/07/27 20:46:33, satorux1 wrote:
> On 2012/07/27 20:04:07, Daniel Erat wrote:
> > I'm still going back and forth on this.  Using a test account, I'm seeing a
> > photo come down the wire as 2211 bytes (JPEG, presumably) but end up as
15428
> > bytes after I recompress it with PNG.  I don't want to do my own lossy JPEG
> > compression after decoding the image, so I'll probably just store the
original
> > untrusted data here and try to make the other end stay performant (e.g. by
> > decoding within a sandbox the first time the data is needed and caching the
> > bitmap somewhere).
> 
> Storing the original data makes sense. You might want to name it something
like
> raw_untrusted_photo, to make it look scary. You might also want to add a new
> filed to store the content type (ex. image/jpeg).

Okay, I've switched to storing the original data and renamed the field (I just
had "untrusted_photo" at first, but I guess that throwing in "raw" doesn't
hurt).  I'm punting on the content type for now, since ImageDecoder doesn't use
it and gdata::GetDownloadDataCallback doesn't pass it (I could add a new
callback type that does, but it seems premature if the image-decoding code won't
use it).

[satorux1, 2012-07-27 22:29:11]

LGTM!

[commit-bot: I haz the power, 2012-07-27 23:03:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10830052/9001

[commit-bot: I haz the power, 2012-07-27 23:03:37]

Presubmit check for 10830052-9001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 1.1s to calculate.

[Daniel Erat, 2012-07-27 23:11:23]

TBR ben for chrome/chrome_browser.gypi

[commit-bot: I haz the power, 2012-07-27 23:11:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10830052/9001

[commit-bot: I haz the power, 2012-07-27 23:29:11]

Try job failure for 10830052-9001 on linux_chromeos for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-07-27 23:30:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10830052/9001

[commit-bot: I haz the power, 2012-07-27 23:32:17]

Try job failure for 10830052-9001 on linux_chromeos for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-07-28 00:02:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/10830052/9001

[commit-bot: I haz the power, 2012-07-28 02:14:00]

Change committed as 148868