Fix SetShape (SetAlphaShape) to allow Null regions (+ tests).

Fix SetShape (SetAlphaShape) to allow Null regions (+ tests).

SetAlphaShape uses a filter to enforce min/max transparency on the window shape specified by the user. This transparency clamping is required by security to ship the SetShape feature.

We added the call to SetAlphaShape earlier, but we needed to temporarily remove it because of bugs with HiDPI devices and NULL shapes.

The issue with HiDPI devices has been fixed in crrev.com/394193003.
This cl fixes Null shape and re-adds the call to SetAlphaShape so that we can enable this feature on Stable.

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

[sky, 2014-08-11 15:26:38]

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/chrome_native_app_window_views.cc (right):

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:664:
native_window->layer()->SetAlphaShape(
Why don't we implement this in the NativeWidget implementations?

https://codereview.chromium.org/456943004/diff/20001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/456943004/diff/20001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:442: if (region)
no need for the if.

[garykac, 2014-08-11 18:19:09]

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/chrome_native_app_window_views.cc (right):

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:664:
native_window->layer()->SetAlphaShape(
On 2014/08/11 15:26:37, sky wrote:
> Why don't we implement this in the NativeWidget implementations?

I'm not sure - the original implementation was done by zork@ in
crrev.com/132343006.  This just re-adding the call now that the HiDPI issues
have been fixed.

zork: Do you have an answer for this?

https://codereview.chromium.org/456943004/diff/20001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/456943004/diff/20001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:442: if (region)
On 2014/08/11 15:26:37, sky wrote:
> no need for the if.

Done.

[Zachary Kuznia, 2014-08-11 20:05:14]

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/apps/chrome_native_app_window_views.cc (right):

https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:664:
native_window->layer()->SetAlphaShape(
On 2014/08/11 18:19:09, garykac wrote:
> On 2014/08/11 15:26:37, sky wrote:
> > Why don't we implement this in the NativeWidget implementations?
> 
> I'm not sure - the original implementation was done by zork@ in
> crrev.com/132343006.  This just re-adding the call now that the HiDPI issues
> have been fixed.
> 
> zork: Do you have an answer for this?

The SetShape() function didn't exist on widget when I wrote the original patch. 
I don't think there's a reason the implementation couldn't be moved there now.

[garykac, 2014-08-11 21:27:54]

On 2014/08/11 20:05:14, Zachary Kuznia wrote:
>
https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
> File chrome/browser/ui/views/apps/chrome_native_app_window_views.cc (right):
> 
>
https://codereview.chromium.org/456943004/diff/20001/chrome/browser/ui/views/...
> chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:664:
> native_window->layer()->SetAlphaShape(
> On 2014/08/11 18:19:09, garykac wrote:
> > On 2014/08/11 15:26:37, sky wrote:
> > > Why don't we implement this in the NativeWidget implementations?
> > 
> > I'm not sure - the original implementation was done by zork@ in
> > crrev.com/132343006.  This just re-adding the call now that the HiDPI issues
> > have been fixed.
> > 
> > zork: Do you have an answer for this?
> 
> The SetShape() function didn't exist on widget when I wrote the original
patch. 
> I don't think there's a reason the implementation couldn't be moved there now.

Argh. I misunderstood sky@'s original question.

There is no reason why it couldn't be done in the NativeWidget (and it makes
more sense for this code to be there).  I've update the code to move the
SetAlphaShape call from ChromeNativeAppWindowViews into NativeWidgetAura.

[sky, 2014-08-11 23:06:18]

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:441: if (window_ && window_->layer())
no need for the window_->layer() part of the conditional. If window_ is valid,
so is it's layer.

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:442:
window_->layer()->SetAlphaShape(make_scoped_ptr(region));
else case to delete region?

[garykac, 2014-08-11 23:20:56]

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
File ui/views/widget/native_widget_aura.cc (right):

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:441: if (window_ && window_->layer())
On 2014/08/11 23:06:18, sky wrote:
> no need for the window_->layer() part of the conditional. If window_ is valid,
> so is it's layer.

Done.

https://codereview.chromium.org/456943004/diff/60001/ui/views/widget/native_w...
ui/views/widget/native_widget_aura.cc:442:
window_->layer()->SetAlphaShape(make_scoped_ptr(region));
On 2014/08/11 23:06:18, sky wrote:
> else case to delete region?

Done.

[sky, 2014-08-11 23:23:30]

LGTM

[garykac, 2014-08-11 23:25:39]

The CQ bit was checked by garykac@chromium.org

[commit-bot: I haz the power, 2014-08-11 23:27:00]

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

[commit-bot: I haz the power, 2014-08-12 03:11:43]

Change committed as 288889