contacts: Give up after repeated transient photo errors.

contacts: Give up after repeated transient photo errors.

This make GDataContactsService report failure after getting
more than two 5xx errors while downloading a particular
contact's photo.  I'm doing this so we don't keep pounding
away indefinitely if the server is having trouble.

BUG=128805
TEST=none


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

[satorux1, 2012-08-17 21:55:21]

https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_contacts_service.cc (right):

https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_contacts_service.cc:730:
std::map<contacts::Contact*, int>
I'm slightly concerned about using a key as a pointer. It probably won't happen
here, but with a pointer as a key, the following scenario could happen:

1) an object is created at address 1234 and added to the map
2) the object is deleted, but the map is not updated
3) a different object is crated at address 1234 again. oops.

Maybe using provider_id is slightly better?

BTW, 'privider_id' sounds like the ID of the provider, rather than a
provider-assigned unique ID. You might want to rename it to something like
contact_id.

[Daniel Erat, 2012-08-17 22:05:03]

https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
File chrome/browser/chromeos/gdata/gdata_contacts_service.cc (right):

https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
chrome/browser/chromeos/gdata/gdata_contacts_service.cc:730:
std::map<contacts::Contact*, int>
On 2012/08/17 21:55:21, satorux1 wrote:
> I'm slightly concerned about using a key as a pointer. It probably won't
happen
> here, but with a pointer as a key, the following scenario could happen:
> 
> 1) an object is created at address 1234 and added to the map
> 2) the object is deleted, but the map is not updated
> 3) a different object is crated at address 1234 again. oops.
> 
> Maybe using provider_id is slightly better?
> 
> BTW, 'privider_id' sounds like the ID of the provider, rather than a
> provider-assigned unique ID. You might want to rename it to something like
> contact_id.

I'll change this if you really want me to, but none of this object's contacts
are deleted while it's being used, and I prefer using known-width pointers as
keys instead of potentially-very-long URLs assigned by the provider.

I'll rename provider_id to contact_id in a separate change.  I initially wanted
to keep "provider" in the name to imply that these IDs are necessarily unique
across providers, but at least in the case of contacts that we get from Google,
I believe that they are always unique (i.e. the user's account name appers in
the URL).

[satorux1, 2012-08-17 22:07:58]

LGTM

On 2012/08/17 22:05:03, Daniel Erat wrote:
>
https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
> File chrome/browser/chromeos/gdata/gdata_contacts_service.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10829398/diff/2001/chrome/browser/chro...
> chrome/browser/chromeos/gdata/gdata_contacts_service.cc:730:
> std::map<contacts::Contact*, int>
> On 2012/08/17 21:55:21, satorux1 wrote:
> > I'm slightly concerned about using a key as a pointer. It probably won't
> happen
> > here, but with a pointer as a key, the following scenario could happen:
> > 
> > 1) an object is created at address 1234 and added to the map
> > 2) the object is deleted, but the map is not updated
> > 3) a different object is crated at address 1234 again. oops.
> > 
> > Maybe using provider_id is slightly better?
> > 
> > BTW, 'privider_id' sounds like the ID of the provider, rather than a
> > provider-assigned unique ID. You might want to rename it to something like
> > contact_id.
> 
> I'll change this if you really want me to, but none of this object's contacts
> are deleted while it's being used, and I prefer using known-width pointers as
> keys instead of potentially-very-long URLs assigned by the provider.

No no, you don't have to. I just forgot to send LGTM in the last feedback.

> 
> I'll rename provider_id to contact_id in a separate change.  I initially
wanted
> to keep "provider" in the name to imply that these IDs are necessarily unique
> across providers, but at least in the case of contacts that we get from
Google,
> I believe that they are always unique (i.e. the user's account name appers in
> the URL).

[commit-bot: I haz the power, 2012-08-17 22:08:30]

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

[commit-bot: I haz the power, 2012-08-18 02:21:42]

Change committed as 152219