Add support for transparent background API

Update support for alphaEnabled (transparent windows) API

Rename transparentBackground -> alphaEnabled (per the spec).

Add app.window.alphaEnabled permission to control which apps are allowed to use alphaEnabled when creating windows.

Add browsertests to verify that the API is only available when permission had been granted and Beta channel or newer.

Add a whitelist (only used in Stable) for this API that defaults to the same set of apps as for app.window.shape.

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

[benwells, 2014-05-06 07:02:14]

On 2014/05/05 16:46:30, garykac wrote:

Let's get the API proposal lg'd before we land this.

[garykac, 2014-05-08 03:46:24]

The implementation now matches the updated API proposal.

[garykac, 2014-05-08 03:46:36]

https://codereview.chromium.org/190533006/diff/150001/apps/app_window.cc
File apps/app_window.cc (right):

https://codereview.chromium.org/190533006/diff/150001/apps/app_window.cc#newc...
apps/app_window.cc:164: always_on_top(false) {
required by git cl format

[benwells, 2014-05-12 04:25:17]

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:231: #if
defined(USE_AURA)
Why is this gated on aura? If you try to use this on another platform and don't
have the permission, you should get the same error.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:236: if
(GetExtension()->HasAPIPermission(APIPermission::kExperimental) ||
After thinking about it I'd prefer to remove access via experimental flag since
we're restricting to dev channel. Are there real apps you know you'll break?

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:242: if
(GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV) {
There should also be an error if the user tries to access this and they are on
stable / beta.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_apitest.cc (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:112:
WindowsApiAlphaEnabledHasPermissions) {
Please also add a test for running this in stable channel. See
https://codereview.chromium.org/267303003/ for an example of how to do this (the
test is being altered in the CL).

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:113: #if
defined(USE_AURA)
So ... if aura is enabled, alphaEnabled will be true if requested? Does this
mean it is true on windows outside of ash? Shouldn't it be false on windows
outside of ash?

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/app_window_custom_bindings.js (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
With the curernt code, this will never happen, and isn't tested. Is that right?
will it be used soon? I'm hesitant to add code that isn't triggered or tested;
maybe this should come in a later CL.

[garykac, 2014-07-09 23:19:21]

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:231: #if
defined(USE_AURA)
On 2014/05/12 04:25:17, benwells wrote:
> Why is this gated on aura? If you try to use this on another platform and
don't
> have the permission, you should get the same error.

Done.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:236: if
(GetExtension()->HasAPIPermission(APIPermission::kExperimental) ||
On 2014/05/12 04:25:17, benwells wrote:
> After thinking about it I'd prefer to remove access via experimental flag
since
> we're restricting to dev channel. Are there real apps you know you'll break?

I don't know of any apps. Removed.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:242: if
(GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV) {
On 2014/05/12 04:25:17, benwells wrote:
> There should also be an error if the user tries to access this and they are on
> stable / beta.

Done. Handled in the _permission_features.json.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_apitest.cc (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:112:
WindowsApiAlphaEnabledHasPermissions) {
On 2014/05/12 04:25:17, benwells wrote:
> Please also add a test for running this in stable channel. See
> https://codereview.chromium.org/267303003/ for an example of how to do this
(the
> test is being altered in the CL).

Done, but on Stable requesting app.window.shape prevents the extension from
loading at all, which gives a warning in the test that can't be caught (AFAICT).

This warning can be ignored with the kFlagIgnoreManifestWarnings flag, but then
the extension is loaded and runs even though it shouldn't (because it is Stable
channel).

I fixed this by adding a check for Stable channel in app_window_api.cc, even
though it is redundant (since the permissions means that it should never be
hit). This might be overkill, but the code will be deleted once this feature
goes to Stable.

https://codereview.chromium.org/190533006/diff/210001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:113: #if
defined(USE_AURA)
On 2014/05/12 04:25:17, benwells wrote:
> So ... if aura is enabled, alphaEnabled will be true if requested? Does this
> mean it is true on windows outside of ash? Shouldn't it be false on windows
> outside of ash?

Changed all the #ifdefs to use USE_ASH.

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/app_window_custom_bindings.js (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
On 2014/05/12 04:25:17, benwells wrote:
> With the curernt code, this will never happen, and isn't tested. Is that
right?
> will it be used soon? I'm hesitant to add code that isn't triggered or tested;
> maybe this should come in a later CL.

Removed for later cl. This should be called when the platform switches from a
mode that supports transparency to one that doesn't (and vice-versa). I'm not
sure yet where the hooks are for that.

[jackhou1, 2014-07-10 03:06:56]

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/app_window_custom_bindings.js (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
On 2014/07/09 23:19:21, garykac wrote:
> On 2014/05/12 04:25:17, benwells wrote:
> > With the curernt code, this will never happen, and isn't tested. Is that
> right?
> > will it be used soon? I'm hesitant to add code that isn't triggered or
tested;
> > maybe this should come in a later CL.
> 
> Removed for later cl. This should be called when the platform switches from a
> mode that supports transparency to one that doesn't (and vice-versa). I'm not
> sure yet where the hooks are for that.

CL is here btw: https://codereview.chromium.org/375183002/

[garykac, 2014-07-11 22:34:52]

On 2014/07/10 03:06:56, jackhou1 wrote:
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> File chrome/renderer/resources/extensions/app_window_custom_bindings.js
(right):
> 
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
> dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
> On 2014/07/09 23:19:21, garykac wrote:
> > On 2014/05/12 04:25:17, benwells wrote:
> > > With the curernt code, this will never happen, and isn't tested. Is that
> > right?
> > > will it be used soon? I'm hesitant to add code that isn't triggered or
> tested;
> > > maybe this should come in a later CL.
> > 
> > Removed for later cl. This should be called when the platform switches from
a
> > mode that supports transparency to one that doesn't (and vice-versa). I'm
not
> > sure yet where the hooks are for that.
> 
> CL is here btw: https://codereview.chromium.org/375183002/

There's a bit of overlap between the cls.  Do you want to take these changes and
merge them into yours?

[jackhou1, 2014-07-14 02:57:59]

On 2014/07/11 22:34:52, garykac wrote:
> On 2014/07/10 03:06:56, jackhou1 wrote:
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > File chrome/renderer/resources/extensions/app_window_custom_bindings.js
> (right):
> > 
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
> > dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
> > On 2014/07/09 23:19:21, garykac wrote:
> > > On 2014/05/12 04:25:17, benwells wrote:
> > > > With the curernt code, this will never happen, and isn't tested. Is that
> > > right?
> > > > will it be used soon? I'm hesitant to add code that isn't triggered or
> > tested;
> > > > maybe this should come in a later CL.
> > > 
> > > Removed for later cl. This should be called when the platform switches
from
> a
> > > mode that supports transparency to one that doesn't (and vice-versa). I'm
> not
> > > sure yet where the hooks are for that.
> > 
> > CL is here btw: https://codereview.chromium.org/375183002/
> 
> There's a bit of overlap between the cls.  Do you want to take these changes
and
> merge them into yours?

Yup, I can do that.

Does this CL disable the feature on non-Ash Windows? (I think we need to keep
it.)

[garykac, 2014-07-14 17:35:12]

On 2014/07/14 02:57:59, jackhou1 wrote:
> On 2014/07/11 22:34:52, garykac wrote:
> > On 2014/07/10 03:06:56, jackhou1 wrote:
> > >
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > > File chrome/renderer/resources/extensions/app_window_custom_bindings.js
> > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > > chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
> > > dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
> > > On 2014/07/09 23:19:21, garykac wrote:
> > > > On 2014/05/12 04:25:17, benwells wrote:
> > > > > With the curernt code, this will never happen, and isn't tested. Is
that
> > > > right?
> > > > > will it be used soon? I'm hesitant to add code that isn't triggered or
> > > tested;
> > > > > maybe this should come in a later CL.
> > > > 
> > > > Removed for later cl. This should be called when the platform switches
> from
> > a
> > > > mode that supports transparency to one that doesn't (and vice-versa).
I'm
> > not
> > > > sure yet where the hooks are for that.
> > > 
> > > CL is here btw: https://codereview.chromium.org/375183002/
> > 
> > There's a bit of overlap between the cls.  Do you want to take these changes
> and
> > merge them into yours?
> 
> Yup, I can do that.
> 
> Does this CL disable the feature on non-Ash Windows? (I think we need to keep
> it.)

Yes, this cl assumes that transparency is only available on Ash.

[jackhou1, 2014-07-16 01:05:54]

On 2014/07/14 17:35:12, garykac wrote:
> On 2014/07/14 02:57:59, jackhou1 wrote:
> > On 2014/07/11 22:34:52, garykac wrote:
> > > On 2014/07/10 03:06:56, jackhou1 wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > > > File chrome/renderer/resources/extensions/app_window_custom_bindings.js
> > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
> > > > chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
> > > > dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
> > > > On 2014/07/09 23:19:21, garykac wrote:
> > > > > On 2014/05/12 04:25:17, benwells wrote:
> > > > > > With the curernt code, this will never happen, and isn't tested. Is
> that
> > > > > right?
> > > > > > will it be used soon? I'm hesitant to add code that isn't triggered
or
> > > > tested;
> > > > > > maybe this should come in a later CL.
> > > > > 
> > > > > Removed for later cl. This should be called when the platform switches
> > from
> > > a
> > > > > mode that supports transparency to one that doesn't (and vice-versa).
> I'm
> > > not
> > > > > sure yet where the hooks are for that.
> > > > 
> > > > CL is here btw: https://codereview.chromium.org/375183002/
> > > 
> > > There's a bit of overlap between the cls.  Do you want to take these
changes
> > and
> > > merge them into yours?
> > 
> > Yup, I can do that.

Actually let me take this back. It sounds like the other CL is more urgent, and
they'd like to try to get it merged back to M37. So I think it would be better
if I submit first, and ask you to merge (though I'm happy to do the merge and
upload a patch set if you like). benwells@ is away sick at the moment, but I
think he'd agree with this. 

> > Does this CL disable the feature on non-Ash Windows? (I think we need to
keep
> > it.)
> 
> Yes, this cl assumes that transparency is only available on Ash.

I think it's quite important for Hangouts that this is available on Windows.

[benwells, 2014-07-18 03:31:46]

I've been on leave for some time and am just catching up again.

Can someone help me out - should I be reviewing this change now as it is, or
wait?

[garykac, 2014-07-18 17:02:49]

On 2014/07/16 01:05:54, jackhou1 wrote:
> I think it's quite important for Hangouts that this is available on Windows.

David Levin corrected me that transparency works on Windows.

On 2014/07/18 03:31:46, benwells wrote:
> I've been on leave for some time and am just catching up again.
> 
> Can someone help me out - should I be reviewing this change now as it is, or
> wait?

Jack will be landing his work first and this this cl will be merged in later
once his code lands.  Ah... I notice now that his CL just landed, so I can
merge, cleanup and the ping you.

[garykac, 2014-07-19 02:40:26]

Merged with unittests passing locally and ready for review.

[benwells, 2014-07-22 03:17:56]

On 2014/07/19 02:40:26, garykac wrote:
> Merged with unittests passing locally and ready for review.

jackhou: wanna do a first pass review?

[jackhou1, 2014-07-22 05:35:35]

Thanks for merging!

The CL description also needs to be updated.

https://codereview.chromium.org/190533006/diff/330001/apps/app_window.h
File apps/app_window.h (right):

https://codereview.chromium.org/190533006/diff/330001/apps/app_window.h#newco...
apps/app_window.h:154: bool alpha_enabled;  // Only supported on ash.
There's probably no need for a comment here any more.

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:60: #if
!defined(USE_AURA)
It might be better to just allow apps to set this regardless of platform, but
add documentation indicating that they need to check AppWindow.alphaEnabled().

https://codereview.chromium.org/190533006/diff/330001/ui/views/widget/desktop...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/190533006/diff/330001/ui/views/widget/desktop...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:809: return true;
I don't think this works on Linux, (it might not be possible). We should
probably leave it false here and investigate supporting it if there's demand for
it on Linux.

[garykac, 2014-07-25 22:50:32]

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/app_window_custom_bindings.js (right):

https://codereview.chromium.org/190533006/diff/210001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/app_window_custom_bindings.js:347:
dispatchEventIfExists(currentWindow, "onAlphaEnabledChanged");
On 2014/07/10 03:06:55, jackhou1 wrote:
> On 2014/07/09 23:19:21, garykac wrote:
> > On 2014/05/12 04:25:17, benwells wrote:
> > > With the curernt code, this will never happen, and isn't tested. Is that
> > right?
> > > will it be used soon? I'm hesitant to add code that isn't triggered or
> tested;
> > > maybe this should come in a later CL.
> > 
> > Removed for later cl. This should be called when the platform switches from
a
> > mode that supports transparency to one that doesn't (and vice-versa). I'm
not
> > sure yet where the hooks are for that.
> 
> CL is here btw: https://codereview.chromium.org/375183002/

Acknowledged.

https://codereview.chromium.org/190533006/diff/330001/apps/app_window.h
File apps/app_window.h (right):

https://codereview.chromium.org/190533006/diff/330001/apps/app_window.h#newco...
apps/app_window.h:154: bool alpha_enabled;  // Only supported on ash.
On 2014/07/22 05:35:35, jackhou1 wrote:
> There's probably no need for a comment here any more.

Done.

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:60: #if
!defined(USE_AURA)
On 2014/07/22 05:35:35, jackhou1 wrote:
> It might be better to just allow apps to set this regardless of platform, but
> add documentation indicating that they need to check AppWindow.alphaEnabled().

Done.  Actually, that matches the current doc for this feature.

The downside is that we don't have a way to return a "why" error message
(missing permission vs. platform not supported), but the fallback of creating a
non-opaque window is clearly preferable.

[jackhou1, 2014-07-29 23:39:10]

Sorry for the delay.

lgtm except we should still keep the missing permission error.

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:60: #if
!defined(USE_AURA)
On 2014/07/25 22:50:31, garykac wrote:
> On 2014/07/22 05:35:35, jackhou1 wrote:
> > It might be better to just allow apps to set this regardless of platform,
but
> > add documentation indicating that they need to check
AppWindow.alphaEnabled().
> 
> Done.  Actually, that matches the current doc for this feature.
> 
> The downside is that we don't have a way to return a "why" error message
> (missing permission vs. platform not supported), but the fallback of creating
a
> non-opaque window is clearly preferable.

Oops, I should have been clearer. We should probably still keep the missing
permission error. My reasoning is that a developer would just want to know how
to make the errors go away. So we can tell them to add the permission, but
there's not much they can do about a platform not supporting alpha.

[garykac, 2014-08-04 23:11:53]

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/330001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:60: #if
!defined(USE_AURA)
On 2014/07/29 23:39:10, jackhou1 wrote:
> On 2014/07/25 22:50:31, garykac wrote:
> > On 2014/07/22 05:35:35, jackhou1 wrote:
> > > It might be better to just allow apps to set this regardless of platform,
> but
> > > add documentation indicating that they need to check
> AppWindow.alphaEnabled().
> > 
> > Done.  Actually, that matches the current doc for this feature.
> > 
> > The downside is that we don't have a way to return a "why" error message
> > (missing permission vs. platform not supported), but the fallback of
creating
> a
> > non-opaque window is clearly preferable.
> 
> Oops, I should have been clearer. We should probably still keep the missing
> permission error. My reasoning is that a developer would just want to know how
> to make the errors go away. So we can tell them to add the permission, but
> there's not much they can do about a platform not supporting alpha.

Done.

[garykac, 2014-08-04 23:12:38]

The CQ bit was checked by garykac@chromium.org

[commit-bot: I haz the power, 2014-08-04 23:15:02]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/garykac@chromium.org/190533006/410001

[commit-bot: I haz the power, 2014-08-05 04:46:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-05 04:49:33]

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

[commit-bot: I haz the power, 2014-08-05 04:49:35]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[benwells, 2014-08-05 04:59:17]

On 2014/08/05 04:49:35, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

You're missing owners. I think I have all the OWNERS you need, I'll look soon.

[benwells, 2014-08-05 23:37:06]

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:58: "The alphaEnabled
option requires beta channel or newer.";
Normally we restrict to dev, not beta. Any reason to let beta use this?

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_apitest.cc (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:115: #if
defined(USE_ASH)
Should this be USE_AURA?

https://codereview.chromium.org/190533006/diff/410001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:134:
"0F42756099D914A026DADFA182871C015735DD95",  // http://crbug.com/323773
This feature is restricted to beta in code, so this whitelist won't do anything.

I don't think all these apps need it anyway.

https://codereview.chromium.org/190533006/diff/410001/chrome/test/data/extens...
File
chrome/test/data/extensions/platform_apps/windows_api_alpha_enabled/has_permissions_has_alpha/background.js
(right):

https://codereview.chromium.org/190533006/diff/410001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/windows_api_alpha_enabled/has_permissions_has_alpha/background.js:7:
var createOptions = { id: testId };
Do you need to set the id?

[garykac, 2014-08-07 03:54:01]

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_api.cc (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_api.cc:58: "The alphaEnabled
option requires beta channel or newer.";
On 2014/08/05 23:37:05, benwells wrote:
> Normally we restrict to dev, not beta. Any reason to let beta use this?

I was matching the Shape permissions, but that's not really necessary. Changed
to dev.

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
File chrome/browser/extensions/api/app_window/app_window_apitest.cc (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/browser/extensio...
chrome/browser/extensions/api/app_window/app_window_apitest.cc:115: #if
defined(USE_ASH)
On 2014/08/05 23:37:05, benwells wrote:
> Should this be USE_AURA?

Yes, but it needs to exclude Linux while keeping CrOS.

At one point in the past it seemed like ASH would do what I wanted here, but it
doesn't quite so I'm working on the correct #def magic here.

https://codereview.chromium.org/190533006/diff/410001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/190533006/diff/410001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:134:
"0F42756099D914A026DADFA182871C015735DD95",  // http://crbug.com/323773
On 2014/08/05 23:37:05, benwells wrote:
> This feature is restricted to beta in code, so this whitelist won't do
anything.

Argh. It doesn't seem like we can restrict the permission with a whitelist here
and still provide a useful error message without duplicating the whitelist.

I'll update the code verify against the whitelist before returning the "channel"
error. But I'll do that tomorrow since I have to leave now.

> I don't think all these apps need it anyway.

I've remove the apps that only require shape.

https://codereview.chromium.org/190533006/diff/410001/chrome/test/data/extens...
File
chrome/test/data/extensions/platform_apps/windows_api_alpha_enabled/has_permissions_has_alpha/background.js
(right):

https://codereview.chromium.org/190533006/diff/410001/chrome/test/data/extens...
chrome/test/data/extensions/platform_apps/windows_api_alpha_enabled/has_permissions_has_alpha/background.js:7:
var createOptions = { id: testId };
On 2014/08/05 23:37:06, benwells wrote:
> Do you need to set the id?

Nope. Removed.

[garykac, 2014-08-08 02:51:38]

ptal

[benwells, 2014-08-12 03:38:07]

lgtm

[garykac, 2014-08-12 14:50:03]

The CQ bit was checked by garykac@chromium.org

[commit-bot: I haz the power, 2014-08-12 14:51:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/garykac@chromium.org/190533006/470001

[commit-bot: I haz the power, 2014-08-12 15:36:36]

Change committed as 288983