Move willEnterFullscreen to WebRemoteFrame.

Move willEnterFullscreen to WebRemoteFrame.

This CL cleans up some of the Blink plumbing for OOPIF fullscreen
support:
- willEnterFullscreen() moves from WebView to WebRemoteFrame.
- tracking of whether fullscreen is entered for cross-process
  ancestor moves from FullscreenController to Fullscreen.

BUG=550497
TEST=no behavior change. This functionality is covered by SitePerProcessInteractiveBrowserTest.Fullscreen*

Committed: https://crrev.com/5b9bce08ba348bf5e77fd693b6d463c7363777ff
Cr-Commit-Position: refs/heads/master@{#397249}

[alexmos, 2016-05-27 21:14:28]

alexmos@chromium.org changed reviewers:
+ dcheng@chromium.org

[alexmos, 2016-05-27 21:14:29]

Daniel, can you please take a look?  This is the cleanup we discussed in
https://codereview.chromium.org/1914643005/diff/220001/third_party/WebKit/Sou...

Plumbing the forCrossProcessAncestor bit through requestFullscreen also allowed
me to remove the user gesture stuff from willEnterFullscreen, which is nice.  I
thought tracking this bit in Fullscreen is better, so I moved it there - that
also allows removing it from did(Enter|Exit)FullScreenForElement.

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
I thought about whether we need to maintain this on the fullscreen element stack
for nested fullscreen, but actually I don't think that will be necessary:  if
this flag is true and we've entered fullscreen for an element in an OOPIF, that
means nothing in this frame is visible, which means that you can't really enter
fullscreen for a different element in this frame, since there's no way to obtain
a user gesture.

[dcheng, 2016-05-28 19:59:10]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/27 at 21:14:29, alexmos wrote:
> I thought about whether we need to maintain this on the fullscreen element
stack for nested fullscreen, but actually I don't think that will be necessary: 
if this flag is true and we've entered fullscreen for an element in an OOPIF,
that means nothing in this frame is visible, which means that you can't really
enter fullscreen for a different element in this frame, since there's no way to
obtain a user gesture.

How does this interact with multimonitor though?

[alexmos, 2016-05-31 16:56:26]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/28 19:59:10, dcheng wrote:
> On 2016/05/27 at 21:14:29, alexmos wrote:
> > I thought about whether we need to maintain this on the fullscreen element
> stack for nested fullscreen, but actually I don't think that will be
necessary: 
> if this flag is true and we've entered fullscreen for an element in an OOPIF,
> that means nothing in this frame is visible, which means that you can't really
> enter fullscreen for a different element in this frame, since there's no way
to
> obtain a user gesture.
> 
> How does this interact with multimonitor though?

I don't think that affects it.  As part of HTML fullscreen, the whole
WebContents goes fullscreen on a particular monitor.  I don't see how to get
into a situation with some element on the page being fullscreen and something
else on the page still visible on a different monitor.

[dcheng, 2016-05-31 17:50:23]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/31 at 16:56:25, alexmos wrote:
> On 2016/05/28 19:59:10, dcheng wrote:
> > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > I thought about whether we need to maintain this on the fullscreen element
> > stack for nested fullscreen, but actually I don't think that will be
necessary: 
> > if this flag is true and we've entered fullscreen for an element in an
OOPIF,
> > that means nothing in this frame is visible, which means that you can't
really
> > enter fullscreen for a different element in this frame, since there's no way
to
> > obtain a user gesture.
> > 
> > How does this interact with multimonitor though?
> 
> I don't think that affects it.  As part of HTML fullscreen, the whole
WebContents goes fullscreen on a particular monitor.  I don't see how to get
into a situation with some element on the page being fullscreen and something
else on the page still visible on a different monitor.

What if you fullcsreen on your secondary monitor and then move the original
browser window to your primary monitor? Do we kick you out of fullscreen if you
try to interact with the browser window that's currently in fullscreen? What if
the browser window spans multiple monitors? Which monitor does it pick?

[alexmos, 2016-05-31 18:08:17]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/31 17:50:23, dcheng wrote:
> On 2016/05/31 at 16:56:25, alexmos wrote:
> > On 2016/05/28 19:59:10, dcheng wrote:
> > > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > > I thought about whether we need to maintain this on the fullscreen
element
> > > stack for nested fullscreen, but actually I don't think that will be
> necessary: 
> > > if this flag is true and we've entered fullscreen for an element in an
> OOPIF,
> > > that means nothing in this frame is visible, which means that you can't
> really
> > > enter fullscreen for a different element in this frame, since there's no
way
> to
> > > obtain a user gesture.
> > > 
> > > How does this interact with multimonitor though?
> > 
> > I don't think that affects it.  As part of HTML fullscreen, the whole
> WebContents goes fullscreen on a particular monitor.  I don't see how to get
> into a situation with some element on the page being fullscreen and something
> else on the page still visible on a different monitor.
> 
> What if you fullcsreen on your secondary monitor and then move the original
> browser window to your primary monitor? Do we kick you out of fullscreen if
you
> try to interact with the browser window that's currently in fullscreen? 

You can't drag the browser window onto the primary monitor, since none of its
controls are visible in fullscreen.  You might be able to do it via the window
manager -- I tried this in Unity on Linux in workspace switcher mode, and that
allowed me to drag the fullscreened window to the primary monitor or even
position it in between (looks really weird, and is probably a bug in existing
fullscreen code).  That doesn't kick you out of fullscreen, but also doesn't
make anything else visible (the window rect is the same), and focusing back to
fullscreened window resizes it to fill one of the monitors.

> What if
> the browser window spans multiple monitors? Which monitor does it pick?

On Linux, it seems to pick the monitor which has more of the window area
visible.  Mac didn't seem to allow me to have a window spanning multiple
monitors, snapping to one or the other when you tried to position the window in
between.  I haven't tried Windows but I'd imagine it'd match Linux.

[dcheng, 2016-05-31 18:50:18]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/31 at 18:08:17, alexmos wrote:
> On 2016/05/31 17:50:23, dcheng wrote:
> > On 2016/05/31 at 16:56:25, alexmos wrote:
> > > On 2016/05/28 19:59:10, dcheng wrote:
> > > > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > > > I thought about whether we need to maintain this on the fullscreen
element
> > > > stack for nested fullscreen, but actually I don't think that will be
> > necessary: 
> > > > if this flag is true and we've entered fullscreen for an element in an
> > OOPIF,
> > > > that means nothing in this frame is visible, which means that you can't
> > really
> > > > enter fullscreen for a different element in this frame, since there's no
way
> > to
> > > > obtain a user gesture.
> > > > 
> > > > How does this interact with multimonitor though?
> > > 
> > > I don't think that affects it.  As part of HTML fullscreen, the whole
> > WebContents goes fullscreen on a particular monitor.  I don't see how to get
> > into a situation with some element on the page being fullscreen and
something
> > else on the page still visible on a different monitor.
> > 
> > What if you fullcsreen on your secondary monitor and then move the original
> > browser window to your primary monitor? Do we kick you out of fullscreen if
you
> > try to interact with the browser window that's currently in fullscreen? 
> 
> You can't drag the browser window onto the primary monitor, since none of its
controls are visible in fullscreen.  You might be able to do it via the window
manager -- I tried this in Unity on Linux in workspace switcher mode, and that
allowed me to drag the fullscreened window to the primary monitor or even
position it in between (looks really weird, and is probably a bug in existing
fullscreen code).  That doesn't kick you out of fullscreen, but also doesn't
make anything else visible (the window rect is the same), and focusing back to
fullscreened window resizes it to fill one of the monitors.
> 
> > What if
> > the browser window spans multiple monitors? Which monitor does it pick?
> 
> On Linux, it seems to pick the monitor which has more of the window area
visible.  Mac didn't seem to allow me to have a window spanning multiple
monitors, snapping to one or the other when you tried to position the window in
between.  I haven't tried Windows but I'd imagine it'd match Linux.

What happens if we fix https://crbug.com/161068 so user gestures follow
postMessage, even across processes?

(It's not clear to me that this bug will ever be fixed for cross process cases,
but it's also not clear to me that this will always be a safe assumption to make
in fullscreen code)

[alexmos, 2016-05-31 23:35:18]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/31 18:50:18, dcheng wrote:
> On 2016/05/31 at 18:08:17, alexmos wrote:
> > On 2016/05/31 17:50:23, dcheng wrote:
> > > On 2016/05/31 at 16:56:25, alexmos wrote:
> > > > On 2016/05/28 19:59:10, dcheng wrote:
> > > > > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > > > > I thought about whether we need to maintain this on the fullscreen
> element
> > > > > stack for nested fullscreen, but actually I don't think that will be
> > > necessary: 
> > > > > if this flag is true and we've entered fullscreen for an element in an
> > > OOPIF,
> > > > > that means nothing in this frame is visible, which means that you
can't
> > > really
> > > > > enter fullscreen for a different element in this frame, since there's
no
> way
> > > to
> > > > > obtain a user gesture.
> > > > > 
> > > > > How does this interact with multimonitor though?
> > > > 
> > > > I don't think that affects it.  As part of HTML fullscreen, the whole
> > > WebContents goes fullscreen on a particular monitor.  I don't see how to
get
> > > into a situation with some element on the page being fullscreen and
> something
> > > else on the page still visible on a different monitor.
> > > 
> > > What if you fullcsreen on your secondary monitor and then move the
original
> > > browser window to your primary monitor? Do we kick you out of fullscreen
if
> you
> > > try to interact with the browser window that's currently in fullscreen? 
> > 
> > You can't drag the browser window onto the primary monitor, since none of
its
> controls are visible in fullscreen.  You might be able to do it via the window
> manager -- I tried this in Unity on Linux in workspace switcher mode, and that
> allowed me to drag the fullscreened window to the primary monitor or even
> position it in between (looks really weird, and is probably a bug in existing
> fullscreen code).  That doesn't kick you out of fullscreen, but also doesn't
> make anything else visible (the window rect is the same), and focusing back to
> fullscreened window resizes it to fill one of the monitors.
> > 
> > > What if
> > > the browser window spans multiple monitors? Which monitor does it pick?
> > 
> > On Linux, it seems to pick the monitor which has more of the window area
> visible.  Mac didn't seem to allow me to have a window spanning multiple
> monitors, snapping to one or the other when you tried to position the window
in
> between.  I haven't tried Windows but I'd imagine it'd match Linux.
> 
> What happens if we fix https://crbug.com/161068 so user gestures follow
> postMessage, even across processes?
> 
> (It's not clear to me that this bug will ever be fixed for cross process
cases,
> but it's also not clear to me that this will always be a safe assumption to
make
> in fullscreen code)

Hmm, that's a really good point, I didn't realize that user gestures can follow
postMessage.  If that bug gets fixed for the cross-process case, then yeah, you
could click on something on the fullscreened child frame, which triggers a
postMessage to parent, which fullscreens a different element.

OK, so then the question is whether or not to wait for cross-process fix for
161068 to do this.  (I don't know how I'd write a realistic test without it.) 
Regardless, for the purposes of this CL, would it be ok to add a TODO here and
mention 161068?  Making this part of the FS element stack will involve a bunch
more plumbing changes, which I don't want to conflate with other changes here.

[dcheng, 2016-06-01 00:29:36]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/05/31 at 23:35:18, alexmos wrote:
> On 2016/05/31 18:50:18, dcheng wrote:
> > On 2016/05/31 at 18:08:17, alexmos wrote:
> > > On 2016/05/31 17:50:23, dcheng wrote:
> > > > On 2016/05/31 at 16:56:25, alexmos wrote:
> > > > > On 2016/05/28 19:59:10, dcheng wrote:
> > > > > > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > > > > > I thought about whether we need to maintain this on the fullscreen
> > element
> > > > > > stack for nested fullscreen, but actually I don't think that will be
> > > > necessary: 
> > > > > > if this flag is true and we've entered fullscreen for an element in
an
> > > > OOPIF,
> > > > > > that means nothing in this frame is visible, which means that you
can't
> > > > really
> > > > > > enter fullscreen for a different element in this frame, since
there's no
> > way
> > > > to
> > > > > > obtain a user gesture.
> > > > > > 
> > > > > > How does this interact with multimonitor though?
> > > > > 
> > > > > I don't think that affects it.  As part of HTML fullscreen, the whole
> > > > WebContents goes fullscreen on a particular monitor.  I don't see how to
get
> > > > into a situation with some element on the page being fullscreen and
> > something
> > > > else on the page still visible on a different monitor.
> > > > 
> > > > What if you fullcsreen on your secondary monitor and then move the
original
> > > > browser window to your primary monitor? Do we kick you out of fullscreen
if
> > you
> > > > try to interact with the browser window that's currently in fullscreen? 
> > > 
> > > You can't drag the browser window onto the primary monitor, since none of
its
> > controls are visible in fullscreen.  You might be able to do it via the
window
> > manager -- I tried this in Unity on Linux in workspace switcher mode, and
that
> > allowed me to drag the fullscreened window to the primary monitor or even
> > position it in between (looks really weird, and is probably a bug in
existing
> > fullscreen code).  That doesn't kick you out of fullscreen, but also doesn't
> > make anything else visible (the window rect is the same), and focusing back
to
> > fullscreened window resizes it to fill one of the monitors.
> > > 
> > > > What if
> > > > the browser window spans multiple monitors? Which monitor does it pick?
> > > 
> > > On Linux, it seems to pick the monitor which has more of the window area
> > visible.  Mac didn't seem to allow me to have a window spanning multiple
> > monitors, snapping to one or the other when you tried to position the window
in
> > between.  I haven't tried Windows but I'd imagine it'd match Linux.
> > 
> > What happens if we fix https://crbug.com/161068 so user gestures follow
> > postMessage, even across processes?
> > 
> > (It's not clear to me that this bug will ever be fixed for cross process
cases,
> > but it's also not clear to me that this will always be a safe assumption to
make
> > in fullscreen code)
> 
> Hmm, that's a really good point, I didn't realize that user gestures can
follow postMessage.  If that bug gets fixed for the cross-process case, then
yeah, you could click on something on the fullscreened child frame, which
triggers a postMessage to parent, which fullscreens a different element.
> 
> OK, so then the question is whether or not to wait for cross-process fix for
161068 to do this.  (I don't know how I'd write a realistic test without it.) 
Regardless, for the purposes of this CL, would it be ok to add a TODO here and
mention 161068?  Making this part of the FS element stack will involve a bunch
more plumbing changes, which I don't want to conflate with other changes here.

sgtm. we might want to make a note on the fullscreen bug too: we believe that
this is currently impossible, but who knows what other weird edge cases are out
there...

[alexmos, 2016-06-01 16:50:13]

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Fullscreen.h (right):

https://codereview.chromium.org/2021513002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Fullscreen.h:129: bool
m_forCrossProcessAncestor;
On 2016/06/01 00:29:36, dcheng wrote:
> On 2016/05/31 at 23:35:18, alexmos wrote:
> > On 2016/05/31 18:50:18, dcheng wrote:
> > > On 2016/05/31 at 18:08:17, alexmos wrote:
> > > > On 2016/05/31 17:50:23, dcheng wrote:
> > > > > On 2016/05/31 at 16:56:25, alexmos wrote:
> > > > > > On 2016/05/28 19:59:10, dcheng wrote:
> > > > > > > On 2016/05/27 at 21:14:29, alexmos wrote:
> > > > > > > > I thought about whether we need to maintain this on the
fullscreen
> > > element
> > > > > > > stack for nested fullscreen, but actually I don't think that will
be
> > > > > necessary: 
> > > > > > > if this flag is true and we've entered fullscreen for an element
in
> an
> > > > > OOPIF,
> > > > > > > that means nothing in this frame is visible, which means that you
> can't
> > > > > really
> > > > > > > enter fullscreen for a different element in this frame, since
> there's no
> > > way
> > > > > to
> > > > > > > obtain a user gesture.
> > > > > > > 
> > > > > > > How does this interact with multimonitor though?
> > > > > > 
> > > > > > I don't think that affects it.  As part of HTML fullscreen, the
whole
> > > > > WebContents goes fullscreen on a particular monitor.  I don't see how
to
> get
> > > > > into a situation with some element on the page being fullscreen and
> > > something
> > > > > else on the page still visible on a different monitor.
> > > > > 
> > > > > What if you fullcsreen on your secondary monitor and then move the
> original
> > > > > browser window to your primary monitor? Do we kick you out of
fullscreen
> if
> > > you
> > > > > try to interact with the browser window that's currently in
fullscreen? 
> > > > 
> > > > You can't drag the browser window onto the primary monitor, since none
of
> its
> > > controls are visible in fullscreen.  You might be able to do it via the
> window
> > > manager -- I tried this in Unity on Linux in workspace switcher mode, and
> that
> > > allowed me to drag the fullscreened window to the primary monitor or even
> > > position it in between (looks really weird, and is probably a bug in
> existing
> > > fullscreen code).  That doesn't kick you out of fullscreen, but also
doesn't
> > > make anything else visible (the window rect is the same), and focusing
back
> to
> > > fullscreened window resizes it to fill one of the monitors.
> > > > 
> > > > > What if
> > > > > the browser window spans multiple monitors? Which monitor does it
pick?
> > > > 
> > > > On Linux, it seems to pick the monitor which has more of the window area
> > > visible.  Mac didn't seem to allow me to have a window spanning multiple
> > > monitors, snapping to one or the other when you tried to position the
window
> in
> > > between.  I haven't tried Windows but I'd imagine it'd match Linux.
> > > 
> > > What happens if we fix https://crbug.com/161068 so user gestures follow
> > > postMessage, even across processes?
> > > 
> > > (It's not clear to me that this bug will ever be fixed for cross process
> cases,
> > > but it's also not clear to me that this will always be a safe assumption
to
> make
> > > in fullscreen code)
> > 
> > Hmm, that's a really good point, I didn't realize that user gestures can
> follow postMessage.  If that bug gets fixed for the cross-process case, then
> yeah, you could click on something on the fullscreened child frame, which
> triggers a postMessage to parent, which fullscreens a different element.
> > 
> > OK, so then the question is whether or not to wait for cross-process fix for
> 161068 to do this.  (I don't know how I'd write a realistic test without it.) 
> Regardless, for the purposes of this CL, would it be ok to add a TODO here and
> mention 161068?  Making this part of the FS element stack will involve a bunch
> more plumbing changes, which I don't want to conflate with other changes here.
> 
> sgtm. we might want to make a note on the fullscreen bug too: we believe that
> this is currently impossible, but who knows what other weird edge cases are
out
> there...

Done.  I'll add a note to the bug when this lands.

[dcheng, 2016-06-01 20:43:15]

lgtm

[alexmos, 2016-06-01 21:09:49]

alexmos@chromium.org changed reviewers:
+ creis@chromium.org

[alexmos, 2016-06-01 21:09:51]

Charlie, can you please review the trivial change in render_frame_proxy.cc for
content/ OWNERS?

[Charlie Reis, 2016-06-01 21:14:35]

content/ LGTM.

[alexmos, 2016-06-01 21:15:20]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2016-06-01 21:15:58]

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

[commit-bot: I haz the power, 2016-06-01 21:22:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-01 21:24:11]

Description was changed from

==========
Move willEnterFullscreen to WebRemoteFrame.

This CL cleans up some of the Blink plumbing for OOPIF fullscreen
support:
- willEnterFullscreen() moves from WebView to WebRemoteFrame.
- tracking of whether fullscreen is entered for cross-process
  ancestor moves from FullscreenController to Fullscreen.

BUG=550497
TEST=no behavior change. This functionality is covered by
SitePerProcessInteractiveBrowserTest.Fullscreen*
==========

to

==========
Move willEnterFullscreen to WebRemoteFrame.

This CL cleans up some of the Blink plumbing for OOPIF fullscreen
support:
- willEnterFullscreen() moves from WebView to WebRemoteFrame.
- tracking of whether fullscreen is entered for cross-process
  ancestor moves from FullscreenController to Fullscreen.

BUG=550497
TEST=no behavior change. This functionality is covered by
SitePerProcessInteractiveBrowserTest.Fullscreen*

Committed: https://crrev.com/5b9bce08ba348bf5e77fd693b6d463c7363777ff
Cr-Commit-Position: refs/heads/master@{#397249}
==========

[commit-bot: I haz the power, 2016-06-01 21:24:13]

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