Factor ExclusiveAccessView out to SubtleNotificationView.

Factor ExclusiveAccessView out to SubtleNotificationView.

This makes the subtle white-on-black fullscreen notification code
available for general use, by removing all fullscreen-specific code from
the view.

Planned for use as a dialog that tells you the new shortcut key for Back
and Forward now that Backspace and Shift+Backspace no longer work.

BUG=610039
Committed: https://crrev.com/238b3392aa0f9cb91aeff8cb147810030b1775af
Cr-Commit-Position: refs/heads/master@{#394662}

[Matt Giuca, 2016-05-18 03:12:48]

mgiuca@chromium.org changed reviewers:
+ msw@chromium.org

[Matt Giuca, 2016-05-18 03:12:48]

msw: Hi, here's a somewhat meaty refactor required for code re-use of the
fullscreen notification bubble.

(It's not clear there is long-term reuse opportunity of this class, since the
planned use of it will be temporary, but I think it still helps with breaking
down the complexity in ExclusiveAccessBubbleViews.)

I hate to have to do this, but we're hoping to have this in M52 (feature request
came up at the last minute), so I would appreciate if we could save any nits
(other than trivial ones) for a follow-up CL.

[Matt Giuca, 2016-05-18 03:12:57]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-18 03:13:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979193002/1

[msw, 2016-05-18 18:06:01]

Looking now; sorry for the delay.

[msw, 2016-05-18 18:38:32]

nice; lgtm with nits.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/exclusive_access_bubble_views.cc (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:60: view_ = new
SubtleNotificationView(base::string16(), base::string16(), this);
nit: maybe remove the string args for the ctor?

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/subtle_notification_view.cc (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:33: // Class containing the
exit instruction text. Contains fancy styling on the
nit: remove references to 'exit' (except in examples)

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:62: // Spacing around the
escape key name.
nit: remove 'escape'

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:185: // TODO(yzshen): Change
to use the new views bubble, BubbleDelegateView.
nit: remove/reassign TODOs? (somewhat stale at this point...)

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:197: // We set layout
manager to nullptr to prevent the widget from sizing its
nit: this is probably unnecessary w/fade animation; add a TODO to remove.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/subtle_notification_view.h (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:36: // the bubble will block
mouse events (required if there is a clickable link);
nit: s/block/handle/

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:45: // Clickable hint text
for exiting fullscreen mode. (Non-simplified mode
nit: remove "for exiting fullscreen mode"?

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:48: // Instruction for
exiting fullscreen / mouse lock. Only present if there is
nit: remove "for exiting fullscreen / mouse lock"?

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:50: InstructionView*
exit_instruction_;
nit: rename |instruction_view_| or similar (not exit related)?

[Matt Giuca, 2016-05-19 01:50:32]

> Looking now; sorry for the delay.

Sorry, didn't mean to rush you *that* much. I really appreciate the sub-24h
review of such a complex CL.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/exclusive_access_bubble_views.cc (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:60: view_ = new
SubtleNotificationView(base::string16(), base::string16(), this);
On 2016/05/18 18:38:31, msw wrote:
> nit: maybe remove the string args for the ctor?

Done.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/subtle_notification_view.cc (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:33: // Class containing the
exit instruction text. Contains fancy styling on the
On 2016/05/18 18:38:32, msw wrote:
> nit: remove references to 'exit' (except in examples)

Done.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:62: // Spacing around the
escape key name.
On 2016/05/18 18:38:32, msw wrote:
> nit: remove 'escape'

Done.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:185: // TODO(yzshen): Change
to use the new views bubble, BubbleDelegateView.
On 2016/05/18 18:38:32, msw wrote:
> nit: remove/reassign TODOs? (somewhat stale at this point...)

Just removed. I think this works fine as it is (I'm not sure what those plans
mentioned were about).

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.cc:197: // We set layout
manager to nullptr to prevent the widget from sizing its
On 2016/05/18 18:38:31, msw wrote:
> nit: this is probably unnecessary w/fade animation; add a TODO to remove.

OK. Note, I'm a little hesitant to remove too much about the sliding animation
because the UX designers have hinted that they're likely to go back to a slide
animation :/

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/subtle_notification_view.h (right):

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:36: // the bubble will block
mouse events (required if there is a clickable link);
On 2016/05/18 18:38:32, msw wrote:
> nit: s/block/handle/

Intercept (compromise). I was trying to get across the idea that mouse events
won't go on to the underlying surface.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:45: // Clickable hint text
for exiting fullscreen mode. (Non-simplified mode
On 2016/05/18 18:38:32, msw wrote:
> nit: remove "for exiting fullscreen mode"?

Done.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:48: // Instruction for
exiting fullscreen / mouse lock. Only present if there is
On 2016/05/18 18:38:32, msw wrote:
> nit: remove "for exiting fullscreen / mouse lock"?

Done.

https://codereview.chromium.org/1979193002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/subtle_notification_view.h:50: InstructionView*
exit_instruction_;
On 2016/05/18 18:38:32, msw wrote:
> nit: rename |instruction_view_| or similar (not exit related)?

Done.

[Matt Giuca, 2016-05-19 01:50:40]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-05-19 01:50:40]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/1979193002/#ps80001
(title: "Respond to review.")

[commit-bot: I haz the power, 2016-05-19 01:51:01]

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

[commit-bot: I haz the power, 2016-05-19 01:51:02]

This CL has an open dependency (Issue 1995893002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[Matt Giuca, 2016-05-19 02:52:43]

Patchset #6 (id:100001) has been deleted

[Matt Giuca, 2016-05-19 02:52:48]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-05-19 02:52:49]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/1979193002/#ps120001
(title: "Avoid dependency patch set.")

[commit-bot: I haz the power, 2016-05-19 02:53:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1979193002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1979193002/120001

[commit-bot: I haz the power, 2016-05-19 04:00:51]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-05-19 04:01:55]

Description was changed from

==========
Factor ExclusiveAccessView out to SubtleNotificationView.

This makes the subtle white-on-black fullscreen notification code
available for general use, by removing all fullscreen-specific code from
the view.

Planned for use as a dialog that tells you the new shortcut key for Back
and Forward now that Backspace and Shift+Backspace no longer work.

BUG=610039
==========

to

==========
Factor ExclusiveAccessView out to SubtleNotificationView.

This makes the subtle white-on-black fullscreen notification code
available for general use, by removing all fullscreen-specific code from
the view.

Planned for use as a dialog that tells you the new shortcut key for Back
and Forward now that Backspace and Shift+Backspace no longer work.

BUG=610039

Committed: https://crrev.com/238b3392aa0f9cb91aeff8cb147810030b1775af
Cr-Commit-Position: refs/heads/master@{#394662}
==========

[commit-bot: I haz the power, 2016-05-19 04:01:56]

Patchset 6 (id:??) landed as
https://crrev.com/238b3392aa0f9cb91aeff8cb147810030b1775af
Cr-Commit-Position: refs/heads/master@{#394662}