Fullscreen / mouselock bubble: Use new theming even when flag disabled.

Fullscreen / mouselock bubble: Use new theming even when flag disabled.

Only applies with simplified-fullscreen-ui flag disabled, and only on
Windows, Linux and Chrome OS.

Now the notification has:
- White-on-black, and transparent background.
- Always fade-in/out instead of slide-down/up.
- New-style padding.
- No message saying "<domain> has gone full screen".
- The new message with border around "Esc", except when there is a
  clickable link.

Basically, it now looks and behaves the same as with the flag enabled,
except that for fullscreen, you can hover the mouse to the top of the
screen to re-show the bubble, and there is a clickable link to get out
of fullscreen. This is a concession to allow for users without a
keyboard to escape fullscreen (until we can implement a better
solution).

Lots of code can now be deleted, but saving that for a follow-up.

BUG=594868, 610900
Committed: https://crrev.com/90ed48601fd68e17b3cc3aa288c1926ef0fe675a
Cr-Commit-Position: refs/heads/master@{#393796}

[Matt Giuca, 2016-05-13 04:31:10]

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

[Matt Giuca, 2016-05-13 04:31:11]

I wanted to just start deleting all the code, but it looks like we have to keep
the flag-disabled state working for the time being, for the purpose of being
able to get out of fullscreen using the mouse.

This CL removes all of the other differences between the flag-disabled and
flag-enabled, just keeping the ability to click out. See
https://crbug.com/594868#c18 for screenshot of that the fullscreen bubble now
looks like when the simplified-fullscreen-ui flag is disabled.

[msw, 2016-05-13 17:08:12]

lgtm with nits

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

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:317: accelerator =
browser_fullscreen_exit_accelerator_;
nit: init |accelerator| to this value, to simplify the conditional block.

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:336:
button_view_->SetVisible(false);
nit: add a TODO to remove |button_view_| (and includes, etc.)?

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:556: int popup_top =
kSimplifiedPopupTopPx;
nit: inline below.

[Matt Giuca, 2016-05-16 04:59:45]

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

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:317: accelerator =
browser_fullscreen_exit_accelerator_;
On 2016/05/13 17:08:11, msw wrote:
> nit: init |accelerator| to this value, to simplify the conditional block.

I'm not sure how that simplifies things (it just reduces one line of code). It
means accelerator gets assigned to potentially the wrong value, then gets
corrected. And it means unnecessarily copying the string even if it's not used.
Doesn't seem good to me.

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:336:
button_view_->SetVisible(false);
On 2016/05/13 17:08:11, msw wrote:
> nit: add a TODO to remove |button_view_| (and includes, etc.)?

Done.

https://codereview.chromium.org/1972543002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:556: int popup_top =
kSimplifiedPopupTopPx;
On 2016/05/13 17:08:11, msw wrote:
> nit: inline below.

Done.

[Matt Giuca, 2016-05-16 04:59:50]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-05-16 04:59:50]

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/1972543002/#ps40001
(title: "Respond to review.")

[commit-bot: I haz the power, 2016-05-16 04:59:57]

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

[commit-bot: I haz the power, 2016-05-16 06:22:39]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-16 06:24:19]

Description was changed from

==========
Fullscreen / mouselock bubble: Use new theming even when flag disabled.

Only applies with simplified-fullscreen-ui flag disabled, and only on
Windows, Linux and Chrome OS.

Now the notification has:
- White-on-black, and transparent background.
- Always fade-in/out instead of slide-down/up.
- New-style padding.
- No message saying "<domain> has gone full screen".
- The new message with border around "Esc", except when there is a
  clickable link.

Basically, it now looks and behaves the same as with the flag enabled,
except that for fullscreen, you can hover the mouse to the top of the
screen to re-show the bubble, and there is a clickable link to get out
of fullscreen. This is a concession to allow for users without a
keyboard to escape fullscreen (until we can implement a better
solution).

Lots of code can now be deleted, but saving that for a follow-up.

BUG=594868,610900
==========

to

==========
Fullscreen / mouselock bubble: Use new theming even when flag disabled.

Only applies with simplified-fullscreen-ui flag disabled, and only on
Windows, Linux and Chrome OS.

Now the notification has:
- White-on-black, and transparent background.
- Always fade-in/out instead of slide-down/up.
- New-style padding.
- No message saying "<domain> has gone full screen".
- The new message with border around "Esc", except when there is a
  clickable link.

Basically, it now looks and behaves the same as with the flag enabled,
except that for fullscreen, you can hover the mouse to the top of the
screen to re-show the bubble, and there is a clickable link to get out
of fullscreen. This is a concession to allow for users without a
keyboard to escape fullscreen (until we can implement a better
solution).

Lots of code can now be deleted, but saving that for a follow-up.

BUG=594868,610900

Committed: https://crrev.com/90ed48601fd68e17b3cc3aa288c1926ef0fe675a
Cr-Commit-Position: refs/heads/master@{#393796}
==========

[commit-bot: I haz the power, 2016-05-16 06:24:20]

Patchset 3 (id:??) landed as
https://crrev.com/90ed48601fd68e17b3cc3aa288c1926ef0fe675a
Cr-Commit-Position: refs/heads/master@{#393796}

[Evan Stade, 2016-05-17 00:23:22]

estade@chromium.org changed reviewers:
+ estade@chromium.org

[Evan Stade, 2016-05-17 00:23:23]

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

https://codereview.chromium.org/1972543002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:28: #include
"ui/native_theme/native_theme.h"
you can remove this now

[Matt Giuca, 2016-05-17 00:58:01]

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

https://codereview.chromium.org/1972543002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/exclusive_access_bubble_views.cc:28: #include
"ui/native_theme/native_theme.h"
On 2016/05/17 00:23:23, Evan Stade wrote:
> you can remove this now

Sorry, I realised this after the CL went through. But it is getting removed in
the next CL (https://codereview.chromium.org/1971033003) which is in the CQ now.

[Evan Stade, 2016-05-17 00:59:39]

On 2016/05/17 00:58:01, Matt Giuca wrote:
>
https://codereview.chromium.org/1972543002/diff/40001/chrome/browser/ui/views...
> File chrome/browser/ui/views/exclusive_access_bubble_views.cc (right):
> 
>
https://codereview.chromium.org/1972543002/diff/40001/chrome/browser/ui/views...
> chrome/browser/ui/views/exclusive_access_bubble_views.cc:28: #include
> "ui/native_theme/native_theme.h"
> On 2016/05/17 00:23:23, Evan Stade wrote:
> > you can remove this now
> 
> Sorry, I realised this after the CL went through. But it is getting removed in
> the next CL (https://codereview.chromium.org/1971033003) which is in the CQ
now.

no worries, thanks for removing a TODO(estade)