Promote chrome app window frame color to stable.

Promote chrome app window frame color to stable.

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

[Matt Giuca, 2014-05-07 11:48:34]

Just having a quick look at this... it seems like you haven't removed all the
dev dependencies.

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

https://codereview.chromium.org/267303003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:47: "Frame options
are only available in dev channel.";
Shouldn't this be going away?

https://codereview.chromium.org/267303003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:439: if
(GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
And this?

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

https://codereview.chromium.org/267303003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:1355: function
testFrameColorsInStable() {
Maybe you should keep this test (at least for a while) that explicitly tests
that stable works the same way as dev now. (The current tests just test whatever
the current version is, so we wouldn't detect problems until we actually hit
beta.)

[benwells, 2014-05-08 02:43:18]

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

https://codereview.chromium.org/267303003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:47: "Frame options
are only available in dev channel.";
On 2014/05/07 11:48:34, Matt Giuca wrote:
> Shouldn't this be going away?

Done.

https://codereview.chromium.org/267303003/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/app_window/app_window_api.cc:439: if
(GetCurrentChannel() > chrome::VersionInfo::CHANNEL_DEV) {
On 2014/05/07 11:48:34, Matt Giuca wrote:
> And this?

Indeed it should. Gone.

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

https://codereview.chromium.org/267303003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:1355: function
testFrameColorsInStable() {
On 2014/05/07 11:48:34, Matt Giuca wrote:
> Maybe you should keep this test (at least for a while) that explicitly tests
> that stable works the same way as dev now. (The current tests just test
whatever
> the current version is, so we wouldn't detect problems until we actually hit
> beta.)

Yeah, I'll keep it around for a little while. Not this bit (the above test is
fine) but the bit that runs it in stable.

[Matt Giuca, 2014-05-08 04:30:14]

lgtm

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

https://codereview.chromium.org/267303003/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/platform_apps/window_api/test.js:1355: function
testFrameColorsInStable() {
Ah that makes sense.

https://codereview.chromium.org/267303003/diff/20001/chrome/browser/apps/app_...
File chrome/browser/apps/app_window_browsertest.cc (right):

https://codereview.chromium.org/267303003/diff/20001/chrome/browser/apps/app_...
chrome/browser/apps/app_window_browsertest.cc:252: // TODO(benwells): Remove
this test once all the things are merged together.
nit: Can you just extend this comment to say that this used to behave
differently on stable/dev, so we now want to test that stable behaves the same
as dev.

[benwells, 2014-05-08 07:35:08]

https://codereview.chromium.org/267303003/diff/20001/chrome/browser/apps/app_...
File chrome/browser/apps/app_window_browsertest.cc (right):

https://codereview.chromium.org/267303003/diff/20001/chrome/browser/apps/app_...
chrome/browser/apps/app_window_browsertest.cc:252: // TODO(benwells): Remove
this test once all the things are merged together.
On 2014/05/08 04:30:15, Matt Giuca wrote:
> nit: Can you just extend this comment to say that this used to behave
> differently on stable/dev, so we now want to test that stable behaves the same
> as dev.

Done.

[benwells, 2014-05-08 07:35:13]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-05-08 07:38:08]

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

[commit-bot: I haz the power, 2014-05-09 00:14:45]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-09 04:11:59]

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

[commit-bot: I haz the power, 2014-05-09 04:12:00]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[benwells, 2014-05-09 05:55:43]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2014-05-09 06:02:46]

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

[commit-bot: I haz the power, 2014-05-09 14:05:34]

Change committed as 269278