AutofillPopupController clarifications + simplifications.

AutofillPopupController clarifications + simplifications.

BUG=166025
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=174490

[Evan Stade, 2012-12-20 03:45:12]

+sky and isherman for review (unless Ilya is ooo, in which case I'll accept a
Scott-only review)

[Ilya Sherman, 2012-12-20 04:34:52]

Thanks, this is much cleaner.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/auto...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/auto...
chrome/browser/autofill/autofill_external_delegate.h:52: int unique_id)
OVERRIDE;
nit: Should we drop "Autofill" from these method names?

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:35: virtual void
MouseLeftPopup() = 0;
Optional nit: Perhaps "exited" rather than "left"?  "MouseLeft" really primes
the reader to expect something about the left mouse button.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:38: virtual void
AcceptSuggestion(size_t index) = 0;
nit: Perhaps name this variable |row|, or else rename |row| below to |index|?

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:48: // Calculates width
of popup based on its contents.
nit: "width of popup" -> "the width of the popup"

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:39: const
std::vector<string16>& sub_labels,
I'd rather prefer you keep the names "values" and "labels" for now, to be
consistent with the names used in the previous implementation; or else rename
the variables there as well.  Moreover, the notion a "label" -- referring to
what you call a sublabel here -- is used elsewhere in the Autofill code as well;
so it would be good to rename those at the same time also.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:112: int
GetRowHeightFromId(int unique_id);
nit: |unique_id| -> |identifier|?

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:112: labels,
labels, labels, autofill_ids);
nit: This now fits on the previous line.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:137:
autofill_ids);
nit: This now fits on the previous line.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:163: labels,
labels, labels, autofill_ids);
nit: This now fits on the previous line.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:203: labels,
labels, labels, autofill_ids);
nit: This now fits on the previous line.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_delegate.h:19: // TODO(estade): this
should only pass |unique_id|.
All autocomplete items have the same unique id, which is 0.  Ditto for passwords
and datalist elements.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/g...
File chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/g...
chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.h:61: void SetupLayout();
nit: SetUpLayout()

[sky, 2012-12-20 16:33:01]

views LGTM

[Evan Stade, 2012-12-20 20:01:25]

+Aurimas FYI for android changes.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/auto...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/auto...
chrome/browser/autofill/autofill_external_delegate.h:52: int unique_id)
OVERRIDE;
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: Should we drop "Autofill" from these method names?

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:35: virtual void
MouseLeftPopup() = 0;
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> Optional nit: Perhaps "exited" rather than "left"?  "MouseLeft" really primes
> the reader to expect something about the left mouse button.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:38: virtual void
AcceptSuggestion(size_t index) = 0;
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: Perhaps name this variable |row|, or else rename |row| below to |index|?

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller.h:48: // Calculates width
of popup based on its contents.
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: "width of popup" -> "the width of the popup"

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:39: const
std::vector<string16>& sub_labels,
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> I'd rather prefer you keep the names "values" and "labels" for now, to be
> consistent with the names used in the previous implementation; or else rename
> the variables there as well.

I don't think it's a worthwhile use of time to update the old implementation.

>  Moreover, the notion a "label" -- referring to
> what you call a sublabel here -- is used elsewhere in the Autofill code as
well;
> so it would be good to rename those at the same time also.

in the context of the UI, "label" and "sub label" make more sense. What is a
"value" to the autofill backend can be used as a "label" in the UI.

I think it's outside of the scope of this CL to update all of autofill, and this
CL is already a yak shave compared to my real goal of implementing payments UI.
But I'll give you my opinion on autofill's variable naming: the term "label"
probably shouldn't be used in the backend at all because it implies something
about the presentation. sub_value or minor_value or differentiating_value or
something would be better. Also, value is fairly generic and non-descriptive, so
it probably should be replaced with something more specific.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:112: int
GetRowHeightFromId(int unique_id);
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: |unique_id| -> |identifier|?

Done, here and everywhere.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:112: labels,
labels, labels, autofill_ids);
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: This now fits on the previous line.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:137:
autofill_ids);
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: This now fits on the previous line.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:163: labels,
labels, labels, autofill_ids);
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: This now fits on the previous line.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc:203: labels,
labels, labels, autofill_ids);
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: This now fits on the previous line.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_delegate.h:19: // TODO(estade): this
should only pass |unique_id|.
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> All autocomplete items have the same unique id, which is 0.  Ditto for
passwords
> and datalist elements.

doesn't sound very unique then. Those things should probably be give actually
unique identifiers.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/g...
File chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/g...
chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.h:61: void SetupLayout();
On 2012/12/20 04:34:52, Ilya Sherman wrote:
> nit: SetUpLayout()

Done.

[aurimas (slooooooooow), 2012-12-20 21:02:36]

lgtm

Do you want me to change the Java side to match the new calls?

[Evan Stade, 2012-12-20 21:09:09]

On 2012/12/20 21:02:36, aurimas wrote:
> lgtm
> 
> Do you want me to change the Java side to match the new calls?

that would be great

[Ilya Sherman, 2012-12-20 21:57:42]

+cc c#

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:39: const
std::vector<string16>& sub_labels,
On 2012/12/20 20:01:25, Evan Stade wrote:
> On 2012/12/20 04:34:52, Ilya Sherman wrote:
> > I'd rather prefer you keep the names "values" and "labels" for now, to be
> > consistent with the names used in the previous implementation; or else
rename
> > the variables there as well.
> 
> I don't think it's a worthwhile use of time to update the old implementation.
> 
> >  Moreover, the notion a "label" -- referring to
> > what you call a sublabel here -- is used elsewhere in the Autofill code as
> well;
> > so it would be good to rename those at the same time also.
> 
> in the context of the UI, "label" and "sub label" make more sense. What is a
> "value" to the autofill backend can be used as a "label" in the UI.
> 
> I think it's outside of the scope of this CL to update all of autofill, and
this
> CL is already a yak shave compared to my real goal of implementing payments
UI.
> But I'll give you my opinion on autofill's variable naming: the term "label"
> probably shouldn't be used in the backend at all because it implies something
> about the presentation. sub_value or minor_value or differentiating_value or
> something would be better. Also, value is fairly generic and non-descriptive,
so
> it probably should be replaced with something more specific.

"value" is the value that gets filled into the active form field (with the
exception of phone numbers, which are exceptional in the Autofill code in
several ways).  "label" is the at-a-glance summary of the profile.  These terms
are currently used consistently throughout the Autofill code: in the
AutofillManager class, in the chrome://settings handler, in the AutofillAgent
code (which is deprecated), and in the newer UI code prior to this CL.  This CL
changes the semantics of "label", but in only one of these places, which
introduces inconsistency: sometimes a "label" is the profile summary; sometimes
it's the value that will be filled into the field.

In short, please don't overload the semantics of "label".  Either choose a
different term to describe the UI, or just don't rename "value" and "label" as
part of this CL.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_delegate.h:19: // TODO(estade): this
should only pass |unique_id|.
On 2012/12/20 20:01:25, Evan Stade wrote:
> On 2012/12/20 04:34:52, Ilya Sherman wrote:
> > All autocomplete items have the same unique id, which is 0.  Ditto for
> passwords
> > and datalist elements.
> 
> doesn't sound very unique then. Those things should probably be give actually
> unique identifiers.

Everything other than Autofill profiles and credit cards is uniquely identified
by its value.  The profiles and credit cards, OTOH, need more.  What we should
really do is to split this into two fields: a required one that specifies the
data type, and an optional (i.e. sometimes NULL) one that specifies which
FormGroup the data comes from.  I think that will be easier to do once the old
UI is fully dead.

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
File chrome/browser/autofill/autofill_external_delegate.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_external_delegate.cc:243: if (web_contents_) {
Hmm, why is it relevant whether we have a WebContents?

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_external_delegate.h:52: int identifier)
OVERRIDE;
nit: Alignment

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_external_delegate.h:54: virtual void
RemoveAutocompleteEntry(const string16& value) OVERRIDE;
Let's combine these two methods into a single method that either takes an index
or a value+identifier.  "RemoveSuggestion" sounds like it should be able to
handle suggestions of any type, whereas it currently only supports Autofill
profiles or credit cards.

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
File chrome/browser/autofill/autofill_manager.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_manager.cc:1145: guid_pairs[i]));
This diff is not related to this CL...

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/ui/...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/ui/...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:96:
window_->GetFocusManager()->AddFocusChangeListener(this);
This diff is not part of this CL.

[Evan Stade, 2012-12-20 23:03:59]

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_controller_impl.h:39: const
std::vector<string16>& sub_labels,
On 2012/12/20 21:57:42, Ilya Sherman wrote:
> On 2012/12/20 20:01:25, Evan Stade wrote:
> > On 2012/12/20 04:34:52, Ilya Sherman wrote:
> > > I'd rather prefer you keep the names "values" and "labels" for now, to be
> > > consistent with the names used in the previous implementation; or else
> rename
> > > the variables there as well.
> > 
> > I don't think it's a worthwhile use of time to update the old
implementation.
> > 
> > >  Moreover, the notion a "label" -- referring to
> > > what you call a sublabel here -- is used elsewhere in the Autofill code as
> > well;
> > > so it would be good to rename those at the same time also.
> > 
> > in the context of the UI, "label" and "sub label" make more sense. What is a
> > "value" to the autofill backend can be used as a "label" in the UI.
> > 
> > I think it's outside of the scope of this CL to update all of autofill, and
> this
> > CL is already a yak shave compared to my real goal of implementing payments
> UI.
> > But I'll give you my opinion on autofill's variable naming: the term "label"
> > probably shouldn't be used in the backend at all because it implies
something
> > about the presentation. sub_value or minor_value or differentiating_value or
> > something would be better. Also, value is fairly generic and
non-descriptive,
> so
> > it probably should be replaced with something more specific.
> 
> "value" is the value that gets filled into the active form field (with the
> exception of phone numbers, which are exceptional in the Autofill code in
> several ways).  "label" is the at-a-glance summary of the profile.  These
terms
> are currently used consistently throughout the Autofill code: in the
> AutofillManager class, in the chrome://settings handler, in the AutofillAgent
> code (which is deprecated), and in the newer UI code prior to this CL.  This
CL
> changes the semantics of "label", but in only one of these places, which
> introduces inconsistency: sometimes a "label" is the profile summary;
sometimes
> it's the value that will be filled into the field.
> 
> In short, please don't overload the semantics of "label".  Either choose a
> different term to describe the UI, or just don't rename "value" and "label" as
> part of this CL.

renamed to name and subtext.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_delegate.h:19: // TODO(estade): this
should only pass |unique_id|.
On 2012/12/20 21:57:42, Ilya Sherman wrote:
> On 2012/12/20 20:01:25, Evan Stade wrote:
> > On 2012/12/20 04:34:52, Ilya Sherman wrote:
> > > All autocomplete items have the same unique id, which is 0.  Ditto for
> > passwords
> > > and datalist elements.
> > 
> > doesn't sound very unique then. Those things should probably be give
actually
> > unique identifiers.
> 
> Everything other than Autofill profiles and credit cards is uniquely
identified
> by its value.  The profiles and credit cards, OTOH, need more.  What we should
> really do is to split this into two fields: a required one that specifies the
> data type, and an optional (i.e. sometimes NULL) one that specifies which
> FormGroup the data comes from.  I think that will be easier to do once the old
> UI is fully dead.

how would the delegate know which password you're accepting?  

I've removed the TODO.

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
File chrome/browser/autofill/autofill_external_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_external_delegate.h:52: int identifier)
OVERRIDE;
On 2012/12/20 21:57:42, Ilya Sherman wrote:
> nit: Alignment

Done.

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/aut...
chrome/browser/autofill/autofill_external_delegate.h:54: virtual void
RemoveAutocompleteEntry(const string16& value) OVERRIDE;
On 2012/12/20 21:57:42, Ilya Sherman wrote:
> Let's combine these two methods into a single method that either takes an
index
> or a value+identifier.  "RemoveSuggestion" sounds like it should be able to
> handle suggestions of any type, whereas it currently only supports Autofill
> profiles or credit cards.

Done.

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/ui/...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/11002/chrome/browser/ui/...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:96:
window_->GetFocusManager()->AddFocusChangeListener(this);
On 2012/12/20 21:57:42, Ilya Sherman wrote:
> This diff is not part of this CL.

please ignore the parts that are part of my previous CLs.

[Ilya Sherman, 2012-12-20 23:11:55]

LGTM, thanks.

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
File chrome/browser/ui/autofill/autofill_popup_delegate.h (right):

https://chromiumcodereview.appspot.com/11636040/diff/2005/chrome/browser/ui/a...
chrome/browser/ui/autofill/autofill_popup_delegate.h:19: // TODO(estade): this
should only pass |unique_id|.
On 2012/12/20 23:03:59, Evan Stade wrote:
> On 2012/12/20 21:57:42, Ilya Sherman wrote:
> > On 2012/12/20 20:01:25, Evan Stade wrote:
> > > On 2012/12/20 04:34:52, Ilya Sherman wrote:
> > > > All autocomplete items have the same unique id, which is 0.  Ditto for
> > > passwords
> > > > and datalist elements.
> > > 
> > > doesn't sound very unique then. Those things should probably be give
> actually
> > > unique identifiers.
> > 
> > Everything other than Autofill profiles and credit cards is uniquely
> identified
> > by its value.  The profiles and credit cards, OTOH, need more.  What we
should
> > really do is to split this into two fields: a required one that specifies
the
> > data type, and an optional (i.e. sometimes NULL) one that specifies which
> > FormGroup the data comes from.  I think that will be easier to do once the
old
> > UI is fully dead.
> 
> how would the delegate know which password you're accepting?  

A given username on a given site only ever has one password associated with it.

https://chromiumcodereview.appspot.com/11636040/diff/31/chrome/browser/ui/aut...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/31/chrome/browser/ui/aut...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:32: // The amount
of minimum padding between the Autofill value and label in pixels.
nit: Please update this comment.

[Evan Stade, 2012-12-20 23:15:00]

https://chromiumcodereview.appspot.com/11636040/diff/31/chrome/browser/ui/aut...
File chrome/browser/ui/autofill/autofill_popup_controller_impl.cc (right):

https://chromiumcodereview.appspot.com/11636040/diff/31/chrome/browser/ui/aut...
chrome/browser/ui/autofill/autofill_popup_controller_impl.cc:32: // The amount
of minimum padding between the Autofill value and label in pixels.
On 2012/12/20 23:11:55, Ilya Sherman wrote:
> nit: Please update this comment.

Done.

[commit-bot: I haz the power, 2012-12-21 20:08:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/11636040/25001

[commit-bot: I haz the power, 2012-12-21 20:27:35]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-12-21 22:05:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/11636040/10030

[commit-bot: I haz the power, 2012-12-22 01:44:38]

Retried try job too often on win_rel for step(s) browser_tests