OOPIF: Replicate allowFullscreen flag.

OOPIF: Replicate allowFullscreen flag.

A frame is allowed enter fullscreen if the allowFullscreen attribute
is present in all of its ancestor <iframe> elements.  For OOPIF, this
attribute needs to be replicated: when a parent frame updates
allowFullscreen for a remote child frame, the child's RenderFrame
needs to learn about this.  Additionally, in a case like
A-embed-B-embed-C, when A updates allowFullscreen for B, C's process
also needs to be notified, since this affects C's fullscreen
calculation.  Therefore, B's RenderFrameProxy in C's process also needs
to receive this update.

This CL replicates the allowFullscreen flag by adding it to
WebFrameOwnerProperties, which are already replicated to the child's
RenderFrame.  It also adds logic to replicate WebFrameOwnerProperties
to the child's proxies as well, so that it can handle allowFullscreen
calculation across multiple remote ancestors.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/788f45b374a8ad32e8a9a6eb910ded1360f33caf
Cr-Commit-Position: refs/heads/master@{#395496}

[alexmos, 2016-04-30 05:26:07]

Description was changed from

==========
OOPIF: Replicate allowFullscreen flag.

BUG=550497
==========

to

==========
OOPIF: Replicate allowFullscreen flag.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[alexmos, 2016-05-02 17:16:27]

Description was changed from

==========
OOPIF: Replicate allowFullscreen flag.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
OOPIF: Replicate allowFullscreen flag.

A frame is allowed enter fullscreen if the allowFullscreen attribute
is present in all of its ancestor <iframe> elements.  For OOPIF, this
attribute needs to be replicated: when a parent frame updates
allowFullscreen for a remote child frame, the child's RenderFrame
needs to learn about this.  Additionally, in a case like
A-embed-B-embed-C, when A updates allowFullscreen for B, C's process
also needs to be notified, since this affects C's fullscreen
calculation.  Therefore, B's RenderFrameProxy in C's process also needs
to receive this update.

This CL replicates the allowFullscreen flag by adding it to
WebFrameOwnerProperties, which are already replicated to the child's
RenderFrame.  It also adds logic to replicate WebFrameOwnerProperties
to the child's proxies as well, so that it can handle allowFullscreen
calculation across multiple remote ancestors.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[alexmos, 2016-05-02 17:44:05]

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

[alexmos, 2016-05-02 17:44:06]

Charlie, can you please take a look at content?  
Daniel, can you please take a look at Blink?

AllowFullscreen is a bit interesting because it seems to clearly belong to
WebFrameOwnerProperties, but it also needs to be replicated to proxies, which
isn't done for WebFrameOwnerProperties today.  (The latter matters for a case
like A-embed-B-embed-C, where C needs to learn if A updates B's allowFullscreen
bit, since it needs to check the bit for all its ancestors.)  So it seems the
choices here were to either (1) add allowFullscreen to WebFrameOwnerProperties,
and replicate WebFrameOwnerProperties to proxies as well as the child's
RenderFrame, or (2) add allowFullscreen to FrameReplicationState and replicate
it via a separate IPC.

I went with (1) since that seemed a bit cleaner.  Although that means that we
might be sending unnecessary updates to proxies whenever margins/scrolling
change, that should be a rare enough case to not matter, and it helps
consistency.  This also allows removing WebFrameOwnerProperties from the
createProvisional path (since the properties will now be in sync before the
remote-to-local navigation starts).  (2) would require a bunch more plumbing for
createChildFrame, as well as adding a separate IPC for allowFullscreen dynamic
updates, which again should be rare enough that I thought it makes more sense to
just reuse the WebFrameOwnerProperties IPC.  

I'm also hopeful that with this change, some of the sandbox flags plumbing could
eventually be simplified to use WebFrameOwnerProperties too.  Let me know what
you think.

Also, I'm not planning to land this until I have a followup fullscreen rendering
CL ready to land as well (I'm still working on cleaning that one up for review),
so that we won't expose OOPIF users to broken fullscreen experience.

https://codereview.chromium.org/1938753002/diff/60001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (left):

https://codereview.chromium.org/1938753002/diff/60001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:1581: // proxies.
This isn't true for allowFullscreen, so I modified WebFrameOwnerProperties to
also be sent to proxies.

https://codereview.chromium.org/1938753002/diff/60001/content/browser/frame_h...
File content/browser/frame_host/render_frame_proxy_host.cc (right):

https://codereview.chromium.org/1938753002/diff/60001/content/browser/frame_h...
content/browser/frame_host/render_frame_proxy_host.cc:199: Send(new
FrameMsg_SetFrameOwnerProperties(
On one hand this probably makes sense to send as part of NewFrameProxy above
(where it would require converting IPC params to a struct).  One the other, it
doesn't really make sense for main frames, and I can't think of a race condition
where this might matter.

[Charlie Reis, 2016-05-03 16:40:44]

Cool.  A few high level questions before I look more deeply.

First, I'm guessing it's fundamental that the renderer process needs to know
this value?  You could imagine just letting the browser process make the call if
fullscreen should work or not, but that probably wouldn't work if the page can
query whether it's allowed to go fullscreen.

Second, will we end up verifying the frame actually has the fullscreen bit in
the browser process as well?  We don't want a subframe process lying and
claiming its parent granted it the bit.

Third, I'm curious about having to replicate WebFrameOwnerProperties to proxies.
 Maybe there's some benefits to consistency and the createProvisional path that
make it worthwhile anyway, but it seems like it might not be necessary.  What if
we only told the LocalFrames true if *all* of their ancestors have the bit, and
false if any ancestors are missing it?  Does the renderer process actually need
to look at the value on the RemoteFrames?

[alexmos, 2016-05-03 17:33:04]

Great questions!

On 2016/05/03 16:40:44, Charlie Reis wrote:
> Cool.  A few high level questions before I look more deeply.
> 
> First, I'm guessing it's fundamental that the renderer process needs to know
> this value?  You could imagine just letting the browser process make the call
if
> fullscreen should work or not, but that probably wouldn't work if the page can
> query whether it's allowed to go fullscreen.

Yes, a web page can directly query this bit via document.fullscreenEnabled,
which is very convenient for my test. :)

> 
> Second, will we end up verifying the frame actually has the fullscreen bit in
> the browser process as well?  We don't want a subframe process lying and
> claiming its parent granted it the bit.

Indeed, I forgot to mention this in the review message, but I was planning on
enforcing this bit in RenderFrameHostImpl::OnToggleFullscreen and killing the
renderer if it tries to lie.  I was going to do it in a separate easy-to-revert
CL though.


> 
> Third, I'm curious about having to replicate WebFrameOwnerProperties to
proxies.
>  Maybe there's some benefits to consistency and the createProvisional path
that
> make it worthwhile anyway, but it seems like it might not be necessary.  What
if
> we only told the LocalFrames true if *all* of their ancestors have the bit,
and
> false if any ancestors are missing it?  Does the renderer process actually
need
> to look at the value on the RemoteFrames?

Yes, this question also came up in my design doc [1].  My concern with that
approach is that it might complicate dealing with dynamic updates (calculating
the set of all affected frames when this bit is updated).  E.g., if we have
multiple levels of embedding, say A(B,C(A2,D,E(F(G,C2)))), and A updates this
bit for its child C, that now needs to figure out which FrameOwners need this
bit recalculated (frames C,D,E,F,G,C2) and send them all a message with the
recalculated bit.  Blink might also need to change to synchronously propagate
allowFullscreen changes to local descendants (matters for A2 above), as opposed
to a simple ancestor walk it uses currently. For cases like C2, we'll need to
avoid sending a duplicate update to that SiteInstance, since Blink will have to
walk through local descendants anyway.  That said, this approach should
certainly work too, and I can explore it more - it could be better than I think.
:)

[1]
https://docs.google.com/document/d/1PkpVsDiGlpsGv4vbViHoa2zmzrjCWSy2ASwGl3KR0FE/

[alexmos, 2016-05-11 00:15:09]

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
File content/browser/frame_host/render_frame_proxy_host.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
content/browser/frame_host/render_frame_proxy_host.cc:202: if
(frame_tree_node_->parent() && should_send_properties) {
Just a note that after discussing this with Charlie last week, I went ahead and
added this optimization in PS6 to only send WebFrameOwnerProperties to new
proxies when they're different from default values.  No need for this IPC in the
default case, as WebRemoteFrameImpl::createRemoteChild already initializes the
FrameOwner with default values.

[dcheng, 2016-05-12 05:32:32]

Blink plumbing lg. I wonder if we can add some documentation somewhere that
describes how the replication flow works: I like the direction this CL takes,
but it's also hard to keep the mental model of replication and how/when things
are replicated / when they take effect in my head.

[Charlie Reis, 2016-05-13 21:18:42]

Ok, sounds like there's enough reasons to notify proxies rather than seek out
all affected children (assuming the frequency of changes stays relatively low).

content/ LGTM!

On 2016/05/02 17:44:06, alexmos wrote:
> Charlie, can you please take a look at content?  
> Daniel, can you please take a look at Blink?
> 
> AllowFullscreen is a bit interesting because it seems to clearly belong to
> WebFrameOwnerProperties, but it also needs to be replicated to proxies, which
> isn't done for WebFrameOwnerProperties today.  (The latter matters for a case
> like A-embed-B-embed-C, where C needs to learn if A updates B's
allowFullscreen
> bit, since it needs to check the bit for all its ancestors.)  So it seems the
> choices here were to either (1) add allowFullscreen to
WebFrameOwnerProperties,
> and replicate WebFrameOwnerProperties to proxies as well as the child's
> RenderFrame, or (2) add allowFullscreen to FrameReplicationState and replicate
> it via a separate IPC.
> 
> I went with (1) since that seemed a bit cleaner.  Although that means that we
> might be sending unnecessary updates to proxies whenever margins/scrolling
> change, that should be a rare enough case to not matter, and it helps
> consistency.

Sanity check: It's sent when scroll mode changes, not scroll position, right? 
We wouldn't want to notify all proxies any time the user scrolled.  :)

>  This also allows removing WebFrameOwnerProperties from the
> createProvisional path (since the properties will now be in sync before the
> remote-to-local navigation starts).  (2) would require a bunch more plumbing
for
> createChildFrame, as well as adding a separate IPC for allowFullscreen dynamic
> updates, which again should be rare enough that I thought it makes more sense
to
> just reuse the WebFrameOwnerProperties IPC.  
> 
> I'm also hopeful that with this change, some of the sandbox flags plumbing
could
> eventually be simplified to use WebFrameOwnerProperties too.  Let me know what
> you think.

Yeah, that could be nice cleanup later on, since it also seems like a fairly
rare event.

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
content/browser/frame_host/render_frame_host_manager.cc:966: // parent.
Maybe it's worth mentioning that this is only currently needed for the
allowFullscreen property, since that can be queried on RemoteFrames?  That's
useful to keep track of if we later decide to remove this and notify the
affected children when a frame changes its value.

(I agree that would be more complex, but it's an option we have to cut down on
IPCs if needed.)

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
File content/browser/frame_host/render_frame_proxy_host.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
content/browser/frame_host/render_frame_proxy_host.cc:202: if
(frame_tree_node_->parent() && should_send_properties) {
On 2016/05/11 00:15:09, alexmos wrote:
> Just a note that after discussing this with Charlie last week, I went ahead
and
> added this optimization in PS6 to only send WebFrameOwnerProperties to new
> proxies when they're different from default values.  No need for this IPC in
the
> default case, as WebRemoteFrameImpl::createRemoteChild already initializes the
> FrameOwner with default values.

Acknowledged.

https://codereview.chromium.org/1938753002/diff/100001/content/browser/site_p...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/site_p...
content/browser/site_per_process_browsertest.cc:6576:
EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)));
Sanity check: This won't be racy, right?  The script call above will tell the
browser about the change, which sends IPCs to each of the renderers.  I think
we're guaranteed that should finish before the ExecuteScript in this lambda
reaches the renderer?

https://codereview.chromium.org/1938753002/diff/100001/content/test/data/fram...
File content/test/data/frame_tree/page_with_one_frame.html (right):

https://codereview.chromium.org/1938753002/diff/100001/content/test/data/fram...
content/test/data/frame_tree/page_with_one_frame.html:16: <iframe id="child0"
src="/cross-site/baz.com/title1.html" allowFullscreen></iframe>
As mentioned on the other CL, we might want a fullscreen test page with this
flag, since this bit is unrelated to how this file is used in other tests.

[alexmos, 2016-05-14 01:12:37]

Thanks for reviewing!

> Sanity check: It's sent when scroll mode changes, not scroll position, right? 
> We wouldn't want to notify all proxies any time the user scrolled.  :)

Yes, this is only sent when the scrolling mode attribute in the iframe element
changes (yes/no/auto). :)

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_manager.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/frame_...
content/browser/frame_host/render_frame_host_manager.cc:966: // parent.
On 2016/05/13 21:18:42, Charlie Reis (slow to reply) wrote:
> Maybe it's worth mentioning that this is only currently needed for the
> allowFullscreen property, since that can be queried on RemoteFrames?  That's
> useful to keep track of if we later decide to remove this and notify the
> affected children when a frame changes its value.
> 
> (I agree that would be more complex, but it's an option we have to cut down on
> IPCs if needed.)

Done.

https://codereview.chromium.org/1938753002/diff/100001/content/browser/site_p...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/1938753002/diff/100001/content/browser/site_p...
content/browser/site_per_process_browsertest.cc:6576:
EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)));
On 2016/05/13 21:18:42, Charlie Reis (slow to reply) wrote:
> Sanity check: This won't be racy, right?  The script call above will tell the
> browser about the change, which sends IPCs to each of the renderers.  I think
> we're guaranteed that should finish before the ExecuteScript in this lambda
> reaches the renderer?

Yes, the ExecuteScript should block until it hears an ack message, which should
happen only after the FrameHostMsg_DidChangeFrameOwnerProperties is processed
and all updates are sent.  So, each renderer is guaranteed to process the
allowFullscreen update before executing the script in is_fullscreen_allowed.

https://codereview.chromium.org/1938753002/diff/100001/content/test/data/fram...
File content/test/data/frame_tree/page_with_one_frame.html (right):

https://codereview.chromium.org/1938753002/diff/100001/content/test/data/fram...
content/test/data/frame_tree/page_with_one_frame.html:16: <iframe id="child0"
src="/cross-site/baz.com/title1.html" allowFullscreen></iframe>
On 2016/05/13 21:18:42, Charlie Reis (slow to reply) wrote:
> As mentioned on the other CL, we might want a fullscreen test page with this
> flag, since this bit is unrelated to how this file is used in other tests.

Done.

[alexmos, 2016-05-14 01:17:12]

On 2016/05/12 05:32:32, dcheng wrote:
> Blink plumbing lg. I wonder if we can add some documentation somewhere that
> describes how the replication flow works: I like the direction this CL takes,
> but it's also hard to keep the mental model of replication and how/when things
> are replicated / when they take effect in my head.

That's a good idea, and I'll start putting together a doc.  This would be also a
good place to put together ideas on potential optimizations for replication
and/or simplifying plumbing for future additions, which I've been wanting to do.
 It might take a bit of time though, so is it ok not to block this CL on it?

[dcheng, 2016-05-19 22:12:07]

lgtm with one suggestion

https://codereview.chromium.org/1938753002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrame.cpp (right):

https://codereview.chromium.org/1938753002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrame.cpp:141: FrameOwner* owner =
toImplBase()->frame()->owner();
Perhaps just make this:

RemoteFrameOwner* owner = toRemoteFrameOwner(toImplBase()->frame()->owner());

owner->setScrollingMode(...);
owner->setMarginWidth(...);
etc.

[alexmos, 2016-05-19 22:56:42]

https://codereview.chromium.org/1938753002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrame.cpp (right):

https://codereview.chromium.org/1938753002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrame.cpp:141: FrameOwner* owner =
toImplBase()->frame()->owner();
On 2016/05/19 22:12:07, dcheng wrote:
> Perhaps just make this:
> 
> RemoteFrameOwner* owner = toRemoteFrameOwner(toImplBase()->frame()->owner());
> 
> owner->setScrollingMode(...);
> owner->setMarginWidth(...);
> etc.

Done.

[alexmos, 2016-05-23 20:34:05]

The CQ bit was checked by alexmos@chromium.org

[alexmos, 2016-05-23 20:34:06]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1938753002/#ps160001
(title: "Daniel's comment")

[commit-bot: I haz the power, 2016-05-23 20:34:25]

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

[commit-bot: I haz the power, 2016-05-23 22:20:42]

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

[commit-bot: I haz the power, 2016-05-23 22:20:44]

Failed to apply patch for content/browser/site_per_process_browsertest.cc:
While running git apply --index -3 -p1;
  error: patch failed: content/browser/site_per_process_browsertest.cc:6565
  Falling back to three-way merge...
  Applied patch to 'content/browser/site_per_process_browsertest.cc' with
conflicts.
  U content/browser/site_per_process_browsertest.cc

Patch:       content/browser/site_per_process_browsertest.cc
Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc
b/content/browser/site_per_process_browsertest.cc
index
e0700c5d218d6ba7f6746f007a1deb249502d169..8d892e3a04526a13e0a95436e4b2513a714e7500
100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -6565,4 +6565,100 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
DetachInUnloadHandler) {
       DepictFrameTree(root));
 }
 
+// Check that out-of-process frames correctly calculate their ability to enter
+// fullscreen. A frame is allowed enter fullscreen if the allowFullscreen
+// attribute is present in all of its ancestor <iframe> elements.  For OOPIF,
+// when a parent frame changes this attribute, the change is replicated to the
+// child frame and its proxies.
+//
+// The test checks the following cases:
+//
+// 1. Static attribute (<iframe allowfullscreen>)
+// 2. Attribute injected dynamically via JavaScript
+// 3. Multiple levels of nesting (A-embed-B-embed-C)
+// 4. Cross-site subframe navigation
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, AllowFullscreen) {
+  // Load a page with a cross-site <iframe allowFullscreen>.
+  GURL url_1(embedded_test_server()->GetURL(
+      "a.com", "/page_with_allowfullscreen_frame.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), url_1));
+
+  WebContentsImpl* contents = web_contents();
+  FrameTreeNode* root = contents->GetFrameTree()->root();
+
+  // Helper to check if a frame is allowed to go fullscreen on the renderer
+  // side.
+  auto is_fullscreen_allowed = [](FrameTreeNode* ftn) {
+    bool fullscreen_allowed = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        ftn->current_frame_host(),
+       
"window.domAutomationController.send(document.webkitFullscreenEnabled)",
+        &fullscreen_allowed));
+    return fullscreen_allowed;
+  };
+
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_TRUE(root->child_at(0)->frame_owner_properties().allowFullscreen);
+
+  // Now navigate to a page with two <iframe>'s, both without allowFullscreen.
+  GURL url_2(embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b,c)"));
+  EXPECT_TRUE(NavigateToURL(shell(), url_2));
+  EXPECT_FALSE(root->child_at(0)->frame_owner_properties().allowFullscreen);
+  EXPECT_FALSE(root->child_at(1)->frame_owner_properties().allowFullscreen);
+
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(1)));
+
+  // Dynamically enable fullscreen for first subframe and check that the
+  // fullscreen property was updated on the FrameTreeNode.
+  EXPECT_TRUE(ExecuteScript(
+      root->current_frame_host(),
+      "document.getElementById('child-0').allowFullscreen='true'"));
+  EXPECT_TRUE(root->child_at(0)->frame_owner_properties().allowFullscreen);
+
+  // Check that the first subframe is now allowed to go fullscreen.  Other
+  // frames shouldn't be affected.
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(1)));
+
+  // Now navigate to a page with two levels of nesting.
+  GURL url_3(embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b(c))"));
+  EXPECT_TRUE(NavigateToURL(shell(), url_3));
+
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(0)->child_at(0)));
+
+  // Dynamically enable fullscreen for bottom subframe.
+  EXPECT_TRUE(ExecuteScript(
+      root->child_at(0)->current_frame_host(),
+      "document.getElementById('child-0').allowFullscreen='true'"));
+
+  // This still shouldn't allow the bottom child to go fullscreen, since the
+  // top frame hasn't allowed fullscreen for the middle frame.
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_FALSE(is_fullscreen_allowed(root->child_at(0)->child_at(0)));
+
+  // Now allow fullscreen for the middle frame.
+  EXPECT_TRUE(ExecuteScript(
+      root->current_frame_host(),
+      "document.getElementById('child-0').allowFullscreen='true'"));
+
+  // All frames should be allowed to go fullscreen now.
+  EXPECT_TRUE(is_fullscreen_allowed(root));
+  EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)));
+  EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)->child_at(0)));
+
+  // Cross-site navigation should preserve the fullscreen flags.
+  NavigateFrameToURL(root->child_at(0)->child_at(0),
+                     embedded_test_server()->GetURL("d.com", "/title1.html"));
+  EXPECT_TRUE(is_fullscreen_allowed(root->child_at(0)->child_at(0)));
+}
+
 }  // namespace content

[commit-bot: I haz the power, 2016-05-23 22:20:45]

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

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

The CQ bit was checked by alexmos@chromium.org

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

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

[commit-bot: I haz the power, 2016-05-23 23:06:18]

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

[commit-bot: I haz the power, 2016-05-24 00:57:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-05-24 00:58:58]

Description was changed from

==========
OOPIF: Replicate allowFullscreen flag.

A frame is allowed enter fullscreen if the allowFullscreen attribute
is present in all of its ancestor <iframe> elements.  For OOPIF, this
attribute needs to be replicated: when a parent frame updates
allowFullscreen for a remote child frame, the child's RenderFrame
needs to learn about this.  Additionally, in a case like
A-embed-B-embed-C, when A updates allowFullscreen for B, C's process
also needs to be notified, since this affects C's fullscreen
calculation.  Therefore, B's RenderFrameProxy in C's process also needs
to receive this update.

This CL replicates the allowFullscreen flag by adding it to
WebFrameOwnerProperties, which are already replicated to the child's
RenderFrame.  It also adds logic to replicate WebFrameOwnerProperties
to the child's proxies as well, so that it can handle allowFullscreen
calculation across multiple remote ancestors.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
OOPIF: Replicate allowFullscreen flag.

A frame is allowed enter fullscreen if the allowFullscreen attribute
is present in all of its ancestor <iframe> elements.  For OOPIF, this
attribute needs to be replicated: when a parent frame updates
allowFullscreen for a remote child frame, the child's RenderFrame
needs to learn about this.  Additionally, in a case like
A-embed-B-embed-C, when A updates allowFullscreen for B, C's process
also needs to be notified, since this affects C's fullscreen
calculation.  Therefore, B's RenderFrameProxy in C's process also needs
to receive this update.

This CL replicates the allowFullscreen flag by adding it to
WebFrameOwnerProperties, which are already replicated to the child's
RenderFrame.  It also adds logic to replicate WebFrameOwnerProperties
to the child's proxies as well, so that it can handle allowFullscreen
calculation across multiple remote ancestors.

BUG=550497
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/788f45b374a8ad32e8a9a6eb910ded1360f33caf
Cr-Commit-Position: refs/heads/master@{#395496}
==========

[commit-bot: I haz the power, 2016-05-24 00:58:59]

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