Enhance chrome.app.window API with window background color

Enhance chrome.app.window API with window background color

The default background color of window is white, a flash of white
will be shown if the loaded page uses a dark background. So add a new
property named background_color to CreateWindowOptions that can be configured
the same as page dark background instead of white default color.

BUG=700282

[junwei, 2017-03-10 08:33:42]

junwei.fu@intel.com changed reviewers:
+ reillyg@chromium.org, stevenjb@chromium.org

[junwei, 2017-03-10 08:33:59]

The CQ bit was checked by junwei.fu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 08:34:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 08:34:14]

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

[commit-bot: I haz the power, 2017-03-10 08:34:14]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[junwei, 2017-03-10 08:47:46]

junwei.fu@intel.com changed reviewers:
+ nasko@chromium.org, pkasting@chromium.org

[Peter Kasting, 2017-03-10 10:13:30]

You set multiple reviewers; please say what you want each to review (especially
for me, since I'm not an OWNER of any of these files)

[Reilly Grant (use Gerrit), 2017-03-10 18:39:19]

I only reviewed changes in //extensions.

https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
File extensions/browser/app_window/app_window.h (right):

https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
extensions/browser/app_window/app_window.h:367: // Set background color.
This is a getter, it cannot "set" anything.

https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
extensions/browser/app_window/app_window.h:565: // Set background color
As in the IDL file, please be more specific.

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

https://codereview.chromium.org/2738373002/diff/1/extensions/common/api/app_w...
extensions/common/api/app_window.idl:258: // Set window background color.
Please be more specific about when this parameter is used and the format the
color must be specified in.

https://codereview.chromium.org/2738373002/diff/1/extensions/components/nativ...
File extensions/components/native_app_window/native_app_window_views.cc (right):

https://codereview.chromium.org/2738373002/diff/1/extensions/components/nativ...
extensions/components/native_app_window/native_app_window_views.cc:299:
view->SetBackgroundColor(SK_ColorTRANSPARENT);
This implies that background color and alpha are mutually exclusive. This should
be documented in the IDL.

[stevenjb, 2017-03-10 18:41:40]

On 2017/03/10 18:39:19, Reilly Grant wrote:
> I only reviewed changes in //extensions.
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
> File extensions/browser/app_window/app_window.h (right):
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
> extensions/browser/app_window/app_window.h:367: // Set background color.
> This is a getter, it cannot "set" anything.
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/browser/app_wind...
> extensions/browser/app_window/app_window.h:565: // Set background color
> As in the IDL file, please be more specific.
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/common/api/app_w...
> File extensions/common/api/app_window.idl (right):
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/common/api/app_w...
> extensions/common/api/app_window.idl:258: // Set window background color.
> Please be more specific about when this parameter is used and the format the
> color must be specified in.
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/components/nativ...
> File extensions/components/native_app_window/native_app_window_views.cc
(right):
> 
>
https://codereview.chromium.org/2738373002/diff/1/extensions/components/nativ...
> extensions/components/native_app_window/native_app_window_views.cc:299:
> view->SetBackgroundColor(SK_ColorTRANSPARENT);
> This implies that background color and alpha are mutually exclusive. This
should
> be documented in the IDL.

I added a comment on the issue but forgot to add one here: This proposal should
go through at least a preliminary API review before we consider the
implementation.

[Peter Kasting, 2017-08-29 04:12:41]

Ping.  Rietveld is deprecated.  I want to make sure my queue here is empty. 
What's the status here?

[junwei, 2017-08-29 04:19:44]

On 2017/08/29 04:12:41, Peter Kasting wrote:
> Ping.  Rietveld is deprecated.  I want to make sure my queue here is empty. 
> What's the status here?

Please ignore the CL that has deprecated, thanks

[junwei, 2017-08-29 04:21:03]

Description was changed from

==========
Enhance chrome.app.window API with window background color

The default background color of window is white, a flash of white
will be shown if the loaded page uses a dark background. So add a new
property named background_color to CreateWindowOptions that can be configured
the same as page dark background instead of white default color.

BUG=700282
==========

to

==========
Enhance chrome.app.window API with window background color

The default background color of window is white, a flash of white
will be shown if the loaded page uses a dark background. So add a new
property named background_color to CreateWindowOptions that can be configured
the same as page dark background instead of white default color.

BUG=700282
==========