[Ash] ShapedAppWindowTargeter treats empty shapes as empty hit test masks.

[Ash] ShapedAppWindowTargeter treats empty shapes as empty hit test masks.

Previously, setShape({'rects': []}) or
setShape({'rects': [{left:0, width:0, top:0, height:1}]})
would make the window completely transparent, but still capture input.

This was because ShapedAppWindowTargeter::GetHitTestMask returned
false if the shape was empty. It now returns false only if the shape
is NULL.

BUG=417014
Committed: https://crrev.com/c0c8e095397d530fd31f7a77d6e39f39bd72b436
Cr-Commit-Position: refs/heads/master@{#296975}

[jackhou1, 2014-09-25 03:00:25]

jackhou@chromium.org changed reviewers:
+ tapted@chromium.org

[tapted, 2014-09-25 04:39:14]

Are there any security concerns to consider? I haven't really looked much into
the shape stuff - we should probably get wez or sadrul to look at this too.

Also do you need to change WidgetHasHitTestMask()?

[jackhou1, 2014-09-25 08:26:38]

Patchset #3 (id:40001) has been deleted

[jackhou1, 2014-09-25 08:28:52]

On 2014/09/25 04:39:14, tapted wrote:
> Are there any security concerns to consider? I haven't really looked much into
> the shape stuff - we should probably get wez or sadrul to look at this too.

Yeah, the bug is basically the security concern, that an app can capture input
that is directed at another window. This CL should fix this.

> Also do you need to change WidgetHasHitTestMask()?

Done.

[jackhou1, 2014-09-25 08:29:04]

jackhou@chromium.org changed reviewers:
+ wez@chromium.org

[jackhou1, 2014-09-25 08:29:04]

wez, could you take a look at this?

[Wez, 2014-09-25 23:36:50]

The behaviour as described in this CL does not match the API design.

setShape() with no "rects" should un-set any existing shape (i.e. restore to
rectangular).

setShape() with an empty list of "rects" should set the window to be completely
invisible, and un-hittable by cursor input.

The bug in crbug.com/417014 is that the latter case has regressed and currently
results in an invisible but still hittable window.

[jackhou1, 2014-09-26 04:41:24]

wez, PTAL

[Wez, 2014-09-26 05:04:27]

The CQ bit was checked by wez@chromium.org

[Wez, 2014-09-26 05:04:28]

lgtm

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602013002/80001

[commit-bot: I haz the power, 2014-09-26 05:12:11]

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

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

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

[jackhou1, 2014-09-26 07:14:28]

tapted, PTAL for OWNERS

[tapted, 2014-09-26 11:08:08]

lgtm

[Wez, 2014-09-26 17:54:36]

The CQ bit was checked by wez@chromium.org

[commit-bot: I haz the power, 2014-09-26 17:56:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602013002/80001

[commit-bot: I haz the power, 2014-09-26 18:00:21]

Committed patchset #4 (id:80001) as 7321d0b37a5860c7619b6e790e2112a24d0e7dd2

[commit-bot: I haz the power, 2014-09-26 18:00:58]

Patchset 4 (id:??) landed as
https://crrev.com/c0c8e095397d530fd31f7a77d6e39f39bd72b436
Cr-Commit-Position: refs/heads/master@{#296975}