Add frame color option to packaged app windows.

Add frame color option to packaged app windows.

If app windows are created with a frame color option, that color is used
for the frame. If no color is supplied a native style look is used
instead. This is currently implemented on linux aura and windows only.

This change also removes the --apps-use-native-frame flag on windows.

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

[Matt Giuca, 2014-02-18 06:03:17]

Looks pretty good (visually, I mean ;)).

https://codereview.chromium.org/166443004/diff/1/apps/app_window.h
File apps/app_window.h (right):

https://codereview.chromium.org/166443004/diff/1/apps/app_window.h#newcode1
apps/app_window.h:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
CL description:
This works on Linux pretty well ("no color" gives the standard blue window, and
colours looks the same as on Windows). We may want to tweak this on Linux but
you may as well say "on Windows and Linux" rather than "on windows only".

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:93: bool
ParseCSSColorString(const std::string& color_string, SkColor* result) {
It's kind of surprising that there isn't already such a method in the Chrome
code base. I searched for one and couldn't find one either. If there really is
none, do you think it's worth putting this somewhere more central (base?).

It should also have a proper unit test (independent of JavaScript and the
windowing API) since it has some hairy edge cases.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:94: std::string
formatted_color = "#";
Nit: Why does formatted_color need to start with a #?

In the #FFF case, this only gets in the way. In the #FFFFFF case, you can just
copy a substring excluding the #.

It just simplifies the arithmetic a bit later (no need for "1 +").

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:96: if
(color_string[0] != '#')
if (color_string.empty() || ...) (potential buffer overflow).

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:101: for (size_t i =
1; i < color_string.length(); i++) {
Why not: i < 4
Also: ++i

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:105: } else {
How about:
// You could even move this first, since it is more common.
else if (color_string.length() == 7) {
  formatted_color = color_string;
} else {
  return false;
}

DCHECK(formatted_color.length() == 7);

Just feels a bit more natural. Also, in the #FFF case, your code currently
builds a string of length 7, then returns false if it's length is not 7. A
DCHECK is more appropriate there.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:114: int
color_ints[3] = {0};
Use HexStringToBytes instead of manually looping.

Aside from simplifying the code, it avoids a couple of weird cases where
HexStringToInt accepts "0x" as well as + and - signs. (For example, the string
"#+F+F+F" is a valid parse because "+F" maps to 15.)

Also, since SkColorSetARGB accepts uint8, your color_ints array should be uint8,
not int, which means you don't need to do range checks. (HexStringToBytes gives
you a vector of uint8 so this is good). Change this comment, removing "and make
sure it is in the correct value range".

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:115: for (int i = 0;
i < 3; i++) {
Nit: ++i

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:117: color_ints + i))
Nit: &color_ints[i]

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:119: if
(color_ints[i] > 255 || color_ints[i] < 0)
This should be a DCHECK, since it is not physically possible for two hex digits
to be outside this range.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:195: //
TODO(benwells): Remove HTML titlebar injection.
I think that removing HTML titlebar injection should be a separate CL from this
one (either before or after; it doesn't matter). It seems like a lot of the
changes in this CL are about removing this unrelated* feature.

*I realise that it's related in that this feature makes that one obsolete, but
the changes aren't dependent on one another.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:368: error_ =
app_window_constants::kInvalidColorSpecification;
Where does this error get displayed or sent? In your sample app, an invalid
colour specification produces no JS console output, and the following Chrome
console output:

[6376:5176:0218/051747:INFO:CONSOLE(0)] "Error in response to app.window.create:
TypeError: Cannot read property 'viewId' of undefined
    at HTMLInputElement.white
(chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/test.js:7:21)", source:
chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/_generated_background_page.html
(0)
[6376:5176:0218/051747:INFO:CONSOLE(0)] "Error in response to app.window.create:
TypeError: Cannot read property 'contentWindow' of undefined
    at HTMLInputElement.white
(chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/test.js:7:21)", source:
chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/_generated_background_page.html
(0)

It's like it just sets the window to null and doesn't print your error message
at all.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:385: // If not on
trunk or dev channel, always use the standard white frame. This
Can you change the second sentence into a TODO(benwells).

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
File chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm:839: return false;
// TODO(benwells)...

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm:843: return SkColor();
// TODO(benwells)...

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
File chrome/browser/ui/views/apps/native_app_window_views.cc (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/native_app_window_views.cc:458: bool
NativeAppWindowViews::ShouldUseNativeFrame() const {
I'm concerned that this will break the --apps-use-native-frame flag on stable
channel. Am I right that on stable channel, all windows will have a white
colour, no matter what? Then --apps-use-native-frame should still work to change
it into a native frame.

Alternatively, if you mean to remove that flag early, say in the CL description
that you are removing --apps-use-native-frame on Windows. You should also update
about:flags to remove kOsWin from this flag.

https://codereview.chromium.org/166443004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/166443004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/app_window.idl:85: // TODO(benwells): Fix our IDL
parsing so I can have this called frame with
s/I/we/

https://codereview.chromium.org/166443004/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/platform_apps/window_api/test.js (right):

https://codereview.chromium.org/166443004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:582: function
testWithBlack() {
I would prefer if one of these tests (black or white) was changed to a colour
with different channel values (like #abcd12) to ensure that the parsing
algorithm is correct.

https://codereview.chromium.org/166443004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:642: function
testWithInvalidColor() {
Can you also test with an invalid string starting with # and length 7, such as
"#bcdefg" (which will test some other code paths).

Actually, preferably to exhaustively testing the color parsing code here would
be to have a separate unit test (not involving JS or the windowing API) to test
all the nutty cases.

[benwells, 2014-02-18 07:47:25]

https://codereview.chromium.org/166443004/diff/1/apps/app_window.h
File apps/app_window.h (right):

https://codereview.chromium.org/166443004/diff/1/apps/app_window.h#newcode1
apps/app_window.h:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
On 2014/02/18 06:03:18, Matt Giuca wrote:
> CL description:
> This works on Linux pretty well ("no color" gives the standard blue window,
and
> colours looks the same as on Windows). We may want to tweak this on Linux but
> you may as well say "on Windows and Linux" rather than "on windows only".

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:93: bool
ParseCSSColorString(const std::string& color_string, SkColor* result) {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> It's kind of surprising that there isn't already such a method in the Chrome
> code base. I searched for one and couldn't find one either. If there really is
> none, do you think it's worth putting this somewhere more central (base?).
> 
> It should also have a proper unit test (independent of JavaScript and the
> windowing API) since it has some hairy edge cases.

Yeah I forgot I had copy pasted this :-/

Using the original one now. I'll apply your review comments to that one.

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:96: if
(color_string[0] != '#')
On 2014/02/18 06:03:18, Matt Giuca wrote:
> if (color_string.empty() || ...) (potential buffer overflow).

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:101: for (size_t i =
1; i < color_string.length(); i++) {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> Why not: i < 4
> Also: ++i

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:105: } else {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> How about:
> // You could even move this first, since it is more common.
> else if (color_string.length() == 7) {
>   formatted_color = color_string;
> } else {
>   return false;
> }
> 
> DCHECK(formatted_color.length() == 7);
> 
> Just feels a bit more natural. Also, in the #FFF case, your code currently
> builds a string of length 7, then returns false if it's length is not 7. A
> DCHECK is more appropriate there.

Done. I think a DCHECK is unwarranted.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:114: int
color_ints[3] = {0};
On 2014/02/18 06:03:18, Matt Giuca wrote:
> Use HexStringToBytes instead of manually looping.
> 
> Aside from simplifying the code, it avoids a couple of weird cases where
> HexStringToInt accepts "0x" as well as + and - signs. (For example, the string
> "#+F+F+F" is a valid parse because "+F" maps to 15.)
> 
> Also, since SkColorSetARGB accepts uint8, your color_ints array should be
uint8,
> not int, which means you don't need to do range checks. (HexStringToBytes
gives
> you a vector of uint8 so this is good). Change this comment, removing "and
make
> sure it is in the correct value range".

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:115: for (int i = 0;
i < 3; i++) {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> Nit: ++i

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:195: //
TODO(benwells): Remove HTML titlebar injection.
On 2014/02/18 06:03:18, Matt Giuca wrote:
> I think that removing HTML titlebar injection should be a separate CL from
this
> one (either before or after; it doesn't matter). It seems like a lot of the
> changes in this CL are about removing this unrelated* feature.
> 
> *I realise that it's related in that this feature makes that one obsolete, but
> the changes aren't dependent on one another.

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:368: error_ =
app_window_constants::kInvalidColorSpecification;
On 2014/02/18 06:03:18, Matt Giuca wrote:
> Where does this error get displayed or sent? In your sample app, an invalid
> colour specification produces no JS console output, and the following Chrome
> console output:
> 
> [6376:5176:0218/051747:INFO:CONSOLE(0)] "Error in response to
app.window.create:
> TypeError: Cannot read property 'viewId' of undefined
>     at HTMLInputElement.white
> (chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/test.js:7:21)", source:
>
chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/_generated_background_page.html
> (0)
> [6376:5176:0218/051747:INFO:CONSOLE(0)] "Error in response to
app.window.create:
> TypeError: Cannot read property 'contentWindow' of undefined
>     at HTMLInputElement.white
> (chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/test.js:7:21)", source:
>
chrome-extension://bcepbjddpgkemenmofbnkinhlohnbjhf/_generated_background_page.html
> (0)
> 
> It's like it just sets the window to null and doesn't print your error message
> at all.

It gets put in chrome.runtime.lastError. For some reason we stopped logging
these errors in general. Maybe we should keep logging them.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:385: // If not on
trunk or dev channel, always use the standard white frame. This
On 2014/02/18 06:03:18, Matt Giuca wrote:
> Can you change the second sentence into a TODO(benwells).

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
File chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm:839: return false;
On 2014/02/18 06:03:18, Matt Giuca wrote:
> // TODO(benwells)...

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/cocoa/apps...
chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm:843: return SkColor();
On 2014/02/18 06:03:18, Matt Giuca wrote:
> // TODO(benwells)...

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
File chrome/browser/ui/views/apps/native_app_window_views.cc (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/native_app_window_views.cc:458: bool
NativeAppWindowViews::ShouldUseNativeFrame() const {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> I'm concerned that this will break the --apps-use-native-frame flag on stable
> channel. Am I right that on stable channel, all windows will have a white
> colour, no matter what? Then --apps-use-native-frame should still work to
change
> it into a native frame.
> 
> Alternatively, if you mean to remove that flag early, say in the CL
description
> that you are removing --apps-use-native-frame on Windows. You should also
update
> about:flags to remove kOsWin from this flag.

Good one, done.

https://codereview.chromium.org/166443004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/app_window.idl (right):

https://codereview.chromium.org/166443004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/app_window.idl:85: // TODO(benwells): Fix our IDL
parsing so I can have this called frame with
On 2014/02/18 06:03:18, Matt Giuca wrote:
> s/I/we/

Oh yeah. In fact all personal pronouns are bad.

Done.

https://codereview.chromium.org/166443004/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/platform_apps/window_api/test.js (right):

https://codereview.chromium.org/166443004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:582: function
testWithBlack() {
On 2014/02/18 06:03:18, Matt Giuca wrote:
> I would prefer if one of these tests (black or white) was changed to a colour
> with different channel values (like #abcd12) to ensure that the parsing
> algorithm is correct.

As discussed, color parsing is unit tested.

[Matt Giuca, 2014-02-19 02:26:42]

lgtm

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
File chrome/browser/ui/views/apps/native_app_window_views.cc (right):

https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/native_app_window_views.cc:458: bool
NativeAppWindowViews::ShouldUseNativeFrame() const {
So, to confirm: you're happy for stable channel users to be missing any way to
get native frames until this color proposal goes through to stable?

If so, lgtm.

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
File chrome/browser/extensions/api/extension_action/extension_action_api.cc
(right):

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
chrome/browser/extensions/api/extension_action/extension_action_api.cc:657:
*result = SkColorSetARGB(255, color_bytes[0], color_bytes[1], color_bytes[2]);
Nit: DCHECK_EQ(3, color_bytes.size());

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
File
chrome/browser/extensions/api/extension_action/extension_browser_actions_api_unittest.cc
(right):

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
chrome/browser/extensions/api/extension_action/extension_browser_actions_api_unittest.cc:53:
RunFailTest("#+22128");
Thanks :)

Hopefully this doesn't break anything... but anybody relying on this behaviour
should not be.

[benwells, 2014-02-19 02:33:28]

On 2014/02/19 02:26:42, Matt Giuca wrote:
> lgtm
> 
>
https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
> File chrome/browser/ui/views/apps/native_app_window_views.cc (right):
> 
>
https://codereview.chromium.org/166443004/diff/1/chrome/browser/ui/views/apps...
> chrome/browser/ui/views/apps/native_app_window_views.cc:458: bool
> NativeAppWindowViews::ShouldUseNativeFrame() const {
> So, to confirm: you're happy for stable channel users to be missing any way to
> get native frames until this color proposal goes through to stable?
> 
> If so, lgtm.

Yes that is right. Flags are for chrome developers to try things, not for chrome
users to set preferences.

> 
>
https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
> File chrome/browser/extensions/api/extension_action/extension_action_api.cc
> (right):
> 
>
https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
> chrome/browser/extensions/api/extension_action/extension_action_api.cc:657:
> *result = SkColorSetARGB(255, color_bytes[0], color_bytes[1], color_bytes[2]);
> Nit: DCHECK_EQ(3, color_bytes.size());
> 
>
https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
> File
>
chrome/browser/extensions/api/extension_action/extension_browser_actions_api_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
>
chrome/browser/extensions/api/extension_action/extension_browser_actions_api_unittest.cc:53:
> RunFailTest("#+22128");
> Thanks :)
> 
> Hopefully this doesn't break anything... but anybody relying on this behaviour
> should not be.

[benwells, 2014-02-19 05:28:30]

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
File chrome/browser/extensions/api/extension_action/extension_action_api.cc
(right):

https://codereview.chromium.org/166443004/diff/190001/chrome/browser/extensio...
chrome/browser/extensions/api/extension_action/extension_action_api.cc:657:
*result = SkColorSetARGB(255, color_bytes[0], color_bytes[1], color_bytes[2]);
On 2014/02/19 02:26:43, Matt Giuca wrote:
> Nit: DCHECK_EQ(3, color_bytes.size());

Done.

[benwells, 2014-02-19 05:28:35]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-19 05:28:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/320002

[commit-bot: I haz the power, 2014-02-19 06:56:50]

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

[commit-bot: I haz the power, 2014-02-19 06:56:51]

Try jobs failed on following builders: mac_chromium_rel

[benwells, 2014-02-19 23:18:45]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-19 23:59:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 01:15:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 01:31:25]

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

[commit-bot: I haz the power, 2014-02-20 01:31:26]

Try jobs failed on following builders: mac_chromium_rel

[benwells, 2014-02-20 01:50:15]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-20 02:06:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 02:30:18]

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

[commit-bot: I haz the power, 2014-02-20 02:30:19]

Try jobs failed on following builders: mac_chromium_rel

[benwells, 2014-02-20 03:01:19]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-20 05:17:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 09:17:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 12:34:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/560001

[commit-bot: I haz the power, 2014-02-20 12:51:18]

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

[commit-bot: I haz the power, 2014-02-20 12:51:18]

Try jobs failed on following builders: mac_chromium_rel

[benwells, 2014-02-21 03:29:56]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-21 03:30:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/980001

[commit-bot: I haz the power, 2014-02-21 04:40:48]

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

[commit-bot: I haz the power, 2014-02-21 04:40:49]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[benwells, 2014-02-21 07:52:36]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-02-21 07:52:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/166443004/1190001

[commit-bot: I haz the power, 2014-02-21 14:30:28]

Change committed as 252548