Have Notifications appear over docked windows

Have Notifications appear over docked windows

Update AshPopupAlignmentDelegate calculations of its work area.

When windows are docked this reduces the work area for windows.
AshPopupAlignmentDelegate uses the work area to determine the positioning of
notifications. Update this calculation to include the docked region.

TEST=AshPopupAlignmentDelegateTest.DockedWindow
BUG=284574
Committed: https://crrev.com/5b4e72c6ad0924d64d5eb9a10e063bc9334e95b2
Cr-Commit-Position: refs/heads/master@{#329486}

[jonross, 2015-05-05 23:12:48]

jonross@chromium.org changed reviewers:
+ oshima@chromium.org

[jonross, 2015-05-05 23:12:49]

Hi Oshima,

Could you take a look at this change?

I've updated AshPopupAlignmentDelegate to increase its work area to include the
docked region.

Thanks.

[oshima, 2015-05-06 21:01:18]

https://codereview.chromium.org/1121893004/diff/1/ash/system/web_notification...
File ash/system/web_notification/ash_popup_alignment_delegate.cc (right):

https://codereview.chromium.org/1121893004/diff/1/ash/system/web_notification...
ash/system/web_notification/ash_popup_alignment_delegate.cc:153:
work_area_.Union(shelf_->dock_bounds());
I don't think this is correct. let me take a look.

[jonross, 2015-05-07 18:54:27]

https://codereview.chromium.org/1121893004/diff/1/ash/system/web_notification...
File ash/system/web_notification/ash_popup_alignment_delegate.cc (right):

https://codereview.chromium.org/1121893004/diff/1/ash/system/web_notification...
ash/system/web_notification/ash_popup_alignment_delegate.cc:153:
work_area_.Union(shelf_->dock_bounds());
On 2015/05/06 21:01:18, oshima wrote:
> I don't think this is correct. let me take a look.

Updated per offline discussions. ShelfLayoutManager now determines the region
available not used by the shelf.

[oshima, 2015-05-07 19:14:51]

nice. two nits.

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
File ash/system/web_notification/ash_popup_alignment_delegate.cc (right):

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
ash/system/web_notification/ash_popup_alignment_delegate.cc:138: work_area_ =
shelf_->non_shelf_bounds_in_root();
can you rename to "non_shelf_bounds_" ?

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
ash/system/web_notification/ash_popup_alignment_delegate.cc:144: }
you should no longer need this if block.

[jonross, 2015-05-07 20:58:30]

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
File ash/system/web_notification/ash_popup_alignment_delegate.cc (right):

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
ash/system/web_notification/ash_popup_alignment_delegate.cc:138: work_area_ =
shelf_->non_shelf_bounds_in_root();
On 2015/05/07 19:14:51, oshima wrote:
> can you rename to "non_shelf_bounds_" ?

Done.

https://codereview.chromium.org/1121893004/diff/40001/ash/system/web_notifica...
ash/system/web_notification/ash_popup_alignment_delegate.cc:144: }
On 2015/05/07 19:14:51, oshima wrote:
> you should no longer need this if block. 

Done.

[jonross, 2015-05-12 18:13:30]

Hi Oshima,

I have addressed your last comments. Could you take another look?

Thanks

[oshima, 2015-05-12 20:08:01]

lgtm

I somehow thought I did lgtm, sorry.

[jonross, 2015-05-12 20:08:35]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2015-05-12 20:08:51]

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

[commit-bot: I haz the power, 2015-05-12 21:06:58]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-12 21:07:55]

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

[jonross, 2015-05-12 22:18:52]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1135323002/ by jonross@chromium.org.

The reason for reverting is: varkha@ noticed that this change will cause the
virtual keyboard to obscure the notifications. Which I confirmed on my local
branch.

Reverting before the regression is released.

I'll work on a followup change that also accounts for the keyboard region..