Provide UI for per-extension enabling/disabling of error collection.

Provide UI for per-extension enabling/disabling of error collection.
BUG=362623
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264822

[not at google - send to devlin, 2014-04-15 21:57:41]

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:40: // The default mask
(for the time being) is to report nothing.
"default" mask is pretty meaningless without context. could you mention what it
means in the comment and/or name the variable better?

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:275: // Registered
preferences take priority over everything else.
nit: put this comment above the declaration of |pref| since it's related

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.h (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.h:148:
ExtensionError::Type type) const;
making callers specify |type| all the time is a bit awkward. if you add a new
type (say, install) does that mean you need to update all callers to take it
into account? what is the advantage in having this so flexible? could you
introduce an ALL enum or a different function to set it for everything at once?

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:176: <div class="checkbox">
make sure this control is only shown if the activity log is enabled

[Devlin, 2014-04-16 18:10:48]

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:40: // The default mask
(for the time being) is to report nothing.
On 2014/04/15 21:57:41, kalman wrote:
> "default" mask is pretty meaningless without context. could you mention what
it
> means in the comment and/or name the variable better?

Done.

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:275: // Registered
preferences take priority over everything else.
On 2014/04/15 21:57:41, kalman wrote:
> nit: put this comment above the declaration of |pref| since it's related

Done.

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.h (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.h:148:
ExtensionError::Type type) const;
On 2014/04/15 21:57:41, kalman wrote:
> making callers specify |type| all the time is a bit awkward. if you add a new
> type (say, install) does that mean you need to update all callers to take it
> into account? what is the advantage in having this so flexible? could you
> introduce an ALL enum or a different function to set it for everything at
once?

Different function added.

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/238073002/diff/10001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:176: <div class="checkbox">
On 2014/04/15 21:57:41, kalman wrote:
> make sure this control is only shown if the activity log is enabled

Good catch. Thanks. :) (assuming s/activity log/error console)

[not at google - send to devlin, 2014-04-16 21:28:31]

more comments

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:54:
default_mask_(kDefaultMask),
why is default_mask_ a member variable rather than using kDefaultMask
everywhere?

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:88:
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
I'm not actually sure that having pref_map_  is much of an advantage over
querying Preferences directly. I presume that they do their own caching, it
wouldn't read from disk every time you access a preference.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:90: mask = enabled ?
mask | (1 << type) : mask &~(1 << type);
I find this pretty odd to read and parse. I would prefer

if (enabled)
  mask |= (1 << type);
else
  mask &= ~(1 << type);

much as it's an extra 3 lines...

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
have a value like ExtensionError::NUM_ERROR or something then use mask |=
(ExtensionError::NUM_ERROR - 1);

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:115:
ExtensionError::RUNTIME_ERROR);
it would be nice if this method could look more like

return pref_map_[extension_id] > 0;

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.h (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.h:70: void
SetReportingForExtensionForAllTypes(const std::string& extension_id,
SetReportingAllForExtension?

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:155: if
(errorCollectionEnabled !== undefined) {
this seems a bit fragile, all it takes is a subtle change in the way that the
model is populated and we'll have an odd consequence. just pass down a flag
similar to how developer mode is passed through.

p.s. extension.hasOwnProperty('errorCollectionEnabled') would be a slightly
stronger check.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:162: [extension.id,
String(e.target.checked)]);
weird that we're converting these to strings to pass to webui, but... well,
whatever, seems to be the practice.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:176: <div class="checkbox
error-collection-control">
hidden by default?

[Devlin, 2014-04-16 22:50:40]

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:54:
default_mask_(kDefaultMask),
On 2014/04/16 21:28:31, kalman wrote:
> why is default_mask_ a member variable rather than using kDefaultMask
> everywhere?

because we also have the option to set the default mask, either for testing
purposes, or if we decide to in the future have a UI option for it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:88:
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
On 2014/04/16 21:28:31, kalman wrote:
> I'm not actually sure that having pref_map_  is much of an advantage over
> querying Preferences directly. I presume that they do their own caching, it
> wouldn't read from disk every time you access a preference.

Oh, I know that we don't read from disk each time, but it's still a little more
complicated, just because you have to grab the extension dictionary, and then
grab the preference, and then do all the base::Value stuff, etc.  It's not a ton
of work, but since this will be queried each time an error is reported,
performance is important, and I think it might be best to keep it.  But, if you
feel strongly, I can change it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:90: mask = enabled ?
mask | (1 << type) : mask &~(1 << type);
On 2014/04/16 21:28:31, kalman wrote:
> I find this pretty odd to read and parse. I would prefer
> 
> if (enabled)
>   mask |= (1 << type);
> else
>   mask &= ~(1 << type);
> 
> much as it's an extra 3 lines...

No!  Too many lines spoil the code! ;)

Done.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 21:28:31, kalman wrote:
> have a value like ExtensionError::NUM_ERROR or something then use mask |=
> (ExtensionError::NUM_ERROR - 1);

Hmm... it does break a few other things.  But I suppose it could be worth it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:115:
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 21:28:31, kalman wrote:
> it would be nice if this method could look more like
> 
> return pref_map_[extension_id] > 0;

Good point. Done.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.h (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.h:70: void
SetReportingForExtensionForAllTypes(const std::string& extension_id,
On 2014/04/16 21:28:31, kalman wrote:
> SetReportingAllForExtension?

Sure, why not.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:155: if
(errorCollectionEnabled !== undefined) {
On 2014/04/16 21:28:31, kalman wrote:
> this seems a bit fragile, all it takes is a subtle change in the way that the
> model is populated and we'll have an odd consequence. just pass down a flag
> similar to how developer mode is passed through.
> 
> p.s. extension.hasOwnProperty('errorCollectionEnabled') would be a slightly
> stronger check.

Passed a flag, but, per in-person conversation, not like developer mode, because
adding classes to the whole page is not a good idea for this.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:162: [extension.id,
String(e.target.checked)]);
On 2014/04/16 21:28:31, kalman wrote:
> weird that we're converting these to strings to pass to webui, but... well,
> whatever, seems to be the practice.

Yeah, I thought it was weird too.  I can only assume that it's to force a type,
rather than implicitly convert between int/bool.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:176: <div class="checkbox
error-collection-control">
On 2014/04/16 21:28:31, kalman wrote:
> hidden by default?

I swear I added this... done.

[Devlin, 2014-04-16 22:50:40]

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:54:
default_mask_(kDefaultMask),
On 2014/04/16 21:28:31, kalman wrote:
> why is default_mask_ a member variable rather than using kDefaultMask
> everywhere?

because we also have the option to set the default mask, either for testing
purposes, or if we decide to in the future have a UI option for it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:88:
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
On 2014/04/16 21:28:31, kalman wrote:
> I'm not actually sure that having pref_map_  is much of an advantage over
> querying Preferences directly. I presume that they do their own caching, it
> wouldn't read from disk every time you access a preference.

Oh, I know that we don't read from disk each time, but it's still a little more
complicated, just because you have to grab the extension dictionary, and then
grab the preference, and then do all the base::Value stuff, etc.  It's not a ton
of work, but since this will be queried each time an error is reported,
performance is important, and I think it might be best to keep it.  But, if you
feel strongly, I can change it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:90: mask = enabled ?
mask | (1 << type) : mask &~(1 << type);
On 2014/04/16 21:28:31, kalman wrote:
> I find this pretty odd to read and parse. I would prefer
> 
> if (enabled)
>   mask |= (1 << type);
> else
>   mask &= ~(1 << type);
> 
> much as it's an extra 3 lines...

No!  Too many lines spoil the code! ;)

Done.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 21:28:31, kalman wrote:
> have a value like ExtensionError::NUM_ERROR or something then use mask |=
> (ExtensionError::NUM_ERROR - 1);

Hmm... it does break a few other things.  But I suppose it could be worth it.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:115:
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 21:28:31, kalman wrote:
> it would be nice if this method could look more like
> 
> return pref_map_[extension_id] > 0;

Good point. Done.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.h (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.h:70: void
SetReportingForExtensionForAllTypes(const std::string& extension_id,
On 2014/04/16 21:28:31, kalman wrote:
> SetReportingAllForExtension?

Sure, why not.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:155: if
(errorCollectionEnabled !== undefined) {
On 2014/04/16 21:28:31, kalman wrote:
> this seems a bit fragile, all it takes is a subtle change in the way that the
> model is populated and we'll have an odd consequence. just pass down a flag
> similar to how developer mode is passed through.
> 
> p.s. extension.hasOwnProperty('errorCollectionEnabled') would be a slightly
> stronger check.

Passed a flag, but, per in-person conversation, not like developer mode, because
adding classes to the whole page is not a good idea for this.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:162: [extension.id,
String(e.target.checked)]);
On 2014/04/16 21:28:31, kalman wrote:
> weird that we're converting these to strings to pass to webui, but... well,
> whatever, seems to be the practice.

Yeah, I thought it was weird too.  I can only assume that it's to force a type,
rather than implicitly convert between int/bool.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extensions.html:176: <div class="checkbox
error-collection-control">
On 2014/04/16 21:28:31, kalman wrote:
> hidden by default?

I swear I added this... done.

[not at google - send to devlin, 2014-04-16 23:22:07]

lgtm, the UINT comment is a bit silly, but the helper-function-for-prefs thing
would be quite nice to fix.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:88:
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
On 2014/04/16 22:50:41, D Cronin wrote:
> On 2014/04/16 21:28:31, kalman wrote:
> > I'm not actually sure that having pref_map_  is much of an advantage over
> > querying Preferences directly. I presume that they do their own caching, it
> > wouldn't read from disk every time you access a preference.
> 
> Oh, I know that we don't read from disk each time, but it's still a little
more
> complicated, just because you have to grab the extension dictionary, and then
> grab the preference, and then do all the base::Value stuff, etc.  It's not a
ton
> of work, but since this will be queried each time an error is reported,
> performance is important, and I think it might be best to keep it.  But, if
you
> feel strongly, I can change it.

I feel like we should have a helper function to get/set that bit in prefs.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 22:50:41, D Cronin wrote:
> On 2014/04/16 21:28:31, kalman wrote:
> > have a value like ExtensionError::NUM_ERROR or something then use mask |=
> > (ExtensionError::NUM_ERROR - 1);
> 
> Hmm... it does break a few other things.  But I suppose it could be worth it.

you could also be mega-sneaky and do "mask = -1" or slightly less sneaky
MAX_UINT.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:155: if
(errorCollectionEnabled !== undefined) {
On 2014/04/16 22:50:41, D Cronin wrote:
> On 2014/04/16 21:28:31, kalman wrote:
> > this seems a bit fragile, all it takes is a subtle change in the way that
the
> > model is populated and we'll have an odd consequence. just pass down a flag
> > similar to how developer mode is passed through.
> > 
> > p.s. extension.hasOwnProperty('errorCollectionEnabled') would be a slightly
> > stronger check.
> 
> Passed a flag, but, per in-person conversation, not like developer mode,
because
> adding classes to the whole page is not a good idea for this.

wantsErrorCollection vs errorCollectionEnabled sure is a fine line :)

[Devlin, 2014-04-16 23:58:22]

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:88:
ErrorPreferenceMap::iterator pref = pref_map_.find(extension_id);
On 2014/04/16 23:22:07, kalman wrote:
> On 2014/04/16 22:50:41, D Cronin wrote:
> > On 2014/04/16 21:28:31, kalman wrote:
> > > I'm not actually sure that having pref_map_  is much of an advantage over
> > > querying Preferences directly. I presume that they do their own caching,
it
> > > wouldn't read from disk every time you access a preference.
> > 
> > Oh, I know that we don't read from disk each time, but it's still a little
> more
> > complicated, just because you have to grab the extension dictionary, and
then
> > grab the preference, and then do all the base::Value stuff, etc.  It's not a
> ton
> > of work, but since this will be queried each time an error is reported,
> > performance is important, and I think it might be best to keep it.  But, if
> you
> > feel strongly, I can change it.
> 
> I feel like we should have a helper function to get/set that bit in prefs.

Per chat discussion, removed pref_map_ in favor of only using ExtensionPrefs.

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 23:22:07, kalman wrote:
> On 2014/04/16 22:50:41, D Cronin wrote:
> > On 2014/04/16 21:28:31, kalman wrote:
> > > have a value like ExtensionError::NUM_ERROR or something then use mask |=
> > > (ExtensionError::NUM_ERROR - 1);
> > 
> > Hmm... it does break a few other things.  But I suppose it could be worth
it.
> 
> you could also be mega-sneaky and do "mask = -1" or slightly less sneaky
> MAX_UINT.

masks are ints, not size_ts, so wouldn't -1 be something like 100...001?  (I
could be wrong - it's been awhile since I've done any fancy bitwise logic.)  And
I'm a little hesitant about UINT_MAX because of it's different definitions on
different architectures (I don't think it's one of the ones we redefine in
base/).

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/resources...
chrome/browser/resources/extensions/extension_list.js:155: if
(errorCollectionEnabled !== undefined) {
On 2014/04/16 23:22:07, kalman wrote:
> On 2014/04/16 22:50:41, D Cronin wrote:
> > On 2014/04/16 21:28:31, kalman wrote:
> > > this seems a bit fragile, all it takes is a subtle change in the way that
> the
> > > model is populated and we'll have an odd consequence. just pass down a
flag
> > > similar to how developer mode is passed through.
> > > 
> > > p.s. extension.hasOwnProperty('errorCollectionEnabled') would be a
slightly
> > > stronger check.
> > 
> > Passed a flag, but, per in-person conversation, not like developer mode,
> because
> > adding classes to the whole page is not a good idea for this.
> 
> wantsErrorCollection vs errorCollectionEnabled sure is a fine line :)

Yeah, chose it because it mirrors "wantsFileAccess" and "allowFileAccess", etc.

[not at google - send to devlin, 2014-04-17 00:15:52]

lgtm

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/30001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:104: (1 <<
ExtensionError::RUNTIME_ERROR);
On 2014/04/16 23:58:22, D Cronin wrote:
> On 2014/04/16 23:22:07, kalman wrote:
> > On 2014/04/16 22:50:41, D Cronin wrote:
> > > On 2014/04/16 21:28:31, kalman wrote:
> > > > have a value like ExtensionError::NUM_ERROR or something then use mask
|=
> > > > (ExtensionError::NUM_ERROR - 1);
> > > 
> > > Hmm... it does break a few other things.  But I suppose it could be worth
> it.
> > 
> > you could also be mega-sneaky and do "mask = -1" or slightly less sneaky
> > MAX_UINT.
> 
> masks are ints, not size_ts, so wouldn't -1 be something like 100...001?  (I
> could be wrong - it's been awhile since I've done any fancy bitwise logic.) 
And
> I'm a little hesitant about UINT_MAX because of it's different definitions on
> different architectures (I don't think it's one of the ones we redefine in
> base/).

oh, read it as uint32, my bad.

https://codereview.chromium.org/238073002/diff/70001/chrome/browser/extension...
File chrome/browser/extensions/error_console/error_console.cc (right):

https://codereview.chromium.org/238073002/diff/70001/chrome/browser/extension...
chrome/browser/extensions/error_console/error_console.cc:92: extension_id,
kStoreExtensionErrorsPref, &mask);
write a GetPref() method? TBH the "extension" in SetExtensionPref is a tautology
but I'm done with nits for the day.

[Devlin, 2014-04-17 16:22:49]

+finnur for extensions_settings_handler

[Finnur, 2014-04-17 17:07:16]

extensions_settings_handler LGTM

[Devlin, 2014-04-17 17:16:15]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:18:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/238073002/...

[commit-bot: I haz the power, 2014-04-17 18:24:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-17 18:24:58]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Devlin, 2014-04-17 21:46:54]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-04-17 21:47:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/238073002/...

[commit-bot: I haz the power, 2014-04-17 22:56:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-17 22:56:21]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[Devlin, 2014-04-18 16:28:03]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-04-18 16:28:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/238073002/...

[commit-bot: I haz the power, 2014-04-18 18:55:06]

Change committed as 264822