Use black for resize gutter in tab fullscreen mode.

Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991, 607415
Committed: https://crrev.com/41663d972e8f55b6f2876eb59329c41465157b7e
Cr-Commit-Position: refs/heads/master@{#393112}

[jbauman, 2016-04-19 02:55:11]

jbauman@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2016-04-25 22:17:58]

https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
File content/browser/renderer_host/delegated_frame_host.cc (right):

https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
content/browser/renderer_host/delegated_frame_host.cc:264:
client_->DelegatedFrameHostOverrideGutterColor(&color);
nit: i think maybe passing the color and returning a possibly-changed color is a
bit nicer and may produce nicer codegen? no pointers, RVO, etc.

then this would be

return client_->DFHGetGutterColor(color);

https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
content/browser/renderer_host/render_widget_host_view_aura.cc:2855: // the
initial switch to fullscreen mode look better by using black as the
Doesn't this make a black flash if the page is white, instead of just having the
white background spread everywhere and then the content re-layouts on top of it?

[danakj, 2016-04-25 22:19:22]

When I f11 on gmail or codereview on linux m49, I get a dark grey gutter, which
causes visible flashing as its immediately replaced by white. This seems like
something page authors should be responsible for?

[jbauman, 2016-05-07 01:31:09]

Description was changed from

==========
Use black for resize gutter in fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.


BUG=602991
==========

to

==========
Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991,607415
==========

[jbauman, 2016-05-07 01:35:23]

PTAL

On 2016/04/25 22:17:58, danakj wrote:
>
https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
> File content/browser/renderer_host/delegated_frame_host.cc (right):
> 
>
https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
> content/browser/renderer_host/delegated_frame_host.cc:264:
> client_->DelegatedFrameHostOverrideGutterColor(&color);
> nit: i think maybe passing the color and returning a possibly-changed color is
a
> bit nicer and may produce nicer codegen? no pointers, RVO, etc.
> 
> then this would be
> 
> return client_->DFHGetGutterColor(color);

Done.
> 
>
https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
> File content/browser/renderer_host/render_widget_host_view_aura.cc (right):
> 
>
https://codereview.chromium.org/1884043002/diff/100001/content/browser/render...
> content/browser/renderer_host/render_widget_host_view_aura.cc:2855: // the
> initial switch to fullscreen mode look better by using black as the
> Doesn't this make a black flash if the page is white, instead of just having
the
> white background spread everywhere and then the content re-layouts on top of
it?

> When I f11 on gmail or codereview on linux m49, I get a dark grey gutter,
which
> causes visible flashing as its immediately replaced by white. This seems like
> something page authors should be responsible for?

This patch only affects tab fullscreen mode, where the page tries to make an
element on it fullscreen. Since the element's background color is often not the
same as the page's background color (black youtube video vs. white page, e.g.)
that causes the flash.

This patch shouldn't affect browser fullscreen mode, which is what happens when
you hit F11. I've changed the comment to make that more clear.

[danakj, 2016-05-10 21:52:51]

I see, so this makes youtube better. But can we maybe use the element's actual
background color?

[jbauman, 2016-05-11 00:14:19]

On 2016/05/10 21:52:51, danakj wrote:
> I see, so this makes youtube better. But can we maybe use the element's actual
> background color?

For now I think just using black makes sense (at least it would prevent a
regression in M51), though I've filed
https://bugs.chromium.org/p/chromium/issues/detail?id=610895 for switching to
use the element's background color. Ideally we'd use a resize lock here and
avoid drawing the gutter at all (assuming a responsive page).

[danakj, 2016-05-11 00:30:43]

OK Thanks for filing the bug. LGTM.

Btw no RWHVAndroid?

[jbauman, 2016-05-11 00:59:14]

jbauman@chromium.org changed reviewers:
+ piman@chromium.org

[jbauman, 2016-05-11 00:59:15]

+piman for content/ OWNERS review

> Btw no RWHVAndroid?

Yeah, that doesn't use DelegatedFrameHost. I don't know if they even use the
background color as a gutter when switching to fullscreen.

[danakj, 2016-05-11 01:00:40]

On Tue, May 10, 2016 at 5:59 PM, <jbauman@chromium.org> wrote:

> +piman for content/ OWNERS review
>
> > Btw no RWHVAndroid?
>
> Yeah, that doesn't use DelegatedFrameHost. I don't know if they even use
> the
> background color as a gutter when switching to fullscreen.
>

Oh ya...

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2016-05-11 01:26:31]

lgtm

[jbauman, 2016-05-11 01:31:47]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2016-05-11 01:32:15]

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

[commit-bot: I haz the power, 2016-05-11 02:23:29]

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

[commit-bot: I haz the power, 2016-05-11 02:23:30]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jbauman, 2016-05-11 23:29:38]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2016-05-11 23:29:39]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1884043002/#ps180001
(title: " ")

[commit-bot: I haz the power, 2016-05-11 23:30:31]

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

[commit-bot: I haz the power, 2016-05-12 00:13:19]

Description was changed from

==========
Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991,607415
==========

to

==========
Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991,607415
==========

[commit-bot: I haz the power, 2016-05-12 00:13:20]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-05-12 00:14:43]

Description was changed from

==========
Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991,607415
==========

to

==========
Use black for resize gutter in tab fullscreen mode.

This avoids the possibility of flashing white in the gutter area, which
is more obtrusive than flashing black.

BUG=602991,607415

Committed: https://crrev.com/41663d972e8f55b6f2876eb59329c41465157b7e
Cr-Commit-Position: refs/heads/master@{#393112}
==========

[commit-bot: I haz the power, 2016-05-12 00:14:44]

Patchset 10 (id:??) landed as
https://crrev.com/41663d972e8f55b6f2876eb59329c41465157b7e
Cr-Commit-Position: refs/heads/master@{#393112}