Propagate scrolling/marginwidth/marginheight property values to child frame.

third_party/WebKit/Source/web/WebLocalFrameImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 183 matching lines @@
 #include "public/platform/WebSuspendableTask.h"
 #include "public/platform/WebURLError.h"
 #include "public/platform/WebVector.h"
 #include "public/web/WebAutofillClient.h"
 #include "public/web/WebConsoleMessage.h"
 #include "public/web/WebDOMEvent.h"
 #include "public/web/WebDocument.h"
 #include "public/web/WebFindOptions.h"
 #include "public/web/WebFormElement.h"
 #include "public/web/WebFrameClient.h"
+#include "public/web/WebFrameOwnerProperties.h"
 #include "public/web/WebHistoryItem.h"
 #include "public/web/WebIconURL.h"
 #include "public/web/WebInputElement.h"
 #include "public/web/WebKit.h"
 #include "public/web/WebNode.h"
 #include "public/web/WebPerformance.h"
 #include "public/web/WebPlugin.h"
 #include "public/web/WebPrintParams.h"
 #include "public/web/WebPrintPresetOptions.h"
 #include "public/web/WebRange.h"
@@ skipping 1534 matching lines @@
     return frame;
 }
 
 PassRefPtrWillBeRawPtr<LocalFrame> WebLocalFrameImpl::createChildFrame(const FrameLoadRequest& request,
     const AtomicString& name, HTMLFrameOwnerElement* ownerElement)
 {
     ASSERT(m_client);
     WebTreeScopeType scope = frame()->document() == ownerElement->treeScope()
         ? WebTreeScopeType::Document
         : WebTreeScopeType::Shadow;
-    WebLocalFrameImpl* webframeChild = toWebLocalFrameImpl(m_client->createChildFrame(this, scope, name, static_cast<WebSandboxFlags>(ownerElement->sandboxFlags())));
+    WebFrameOwnerProperties ownerProperties(ownerElement->scrollingMode(), ownerElement->marginWidth(), ownerElement->marginHeight());
+    WebLocalFrameImpl* webframeChild = toWebLocalFrameImpl(m_client->createChildFrame(this, scope, name, static_cast<WebSandboxFlags>(ownerElement->sandboxFlags()), ownerProperties));
     if (!webframeChild)
         return nullptr;
 
     // FIXME: Using subResourceAttributeName as fallback is not a perfect
     // solution. subResourceAttributeName returns just one attribute name. The
     // element might not have the attribute, and there might be other attributes
     // which can identify the element.
     RefPtrWillBeRawPtr<LocalFrame> child = webframeChild->initializeCoreFrame(frame()->host(), ownerElement, name, ownerElement->getAttribute(ownerElement->subResourceAttributeName()));
     // Initializing the core frame may cause the new child to be detached, since
     // it may dispatch a load event in the parent.
@@ skipping 219 matching lines @@
 static void ensureFrameLoaderHasCommitted(FrameLoader& frameLoader)
 {
     // Internally, Blink uses CommittedMultipleRealLoads to track whether the
     // next commit should create a new history item or not. Ensure we have
     // reached that state.
     if (frameLoader.stateMachine()->committedMultipleRealLoads())
         return;
     frameLoader.stateMachine()->advanceTo(FrameLoaderStateMachine::CommittedMultipleRealLoads);
 }
 
-void WebLocalFrameImpl::initializeToReplaceRemoteFrame(WebRemoteFrame* oldWebFrame, const WebString& name, WebSandboxFlags flags)
+void WebLocalFrameImpl::initializeToReplaceRemoteFrame(WebRemoteFrame* oldWebFrame, const WebString& name, WebSandboxFlags flags, const WebFrameOwnerProperties& frameOwnerProperties)
 {
     Frame* oldFrame = toCoreFrame(oldWebFrame);
     // Note: this *always* temporarily sets a frame owner, even for main frames!
     // When a core Frame is created with no owner, it attempts to set itself as
     // the main frame of the Page. However, this is a provisional frame, and may
     // disappear, so Page::m_mainFrame can't be updated just yet.
-    OwnPtrWillBeRawPtr<FrameOwner> tempOwner = RemoteBridgeFrameOwner::create(nullptr, SandboxNone);
+    OwnPtrWillBeRawPtr<FrameOwner> tempOwner = RemoteBridgeFrameOwner::create(nullptr, SandboxNone, WebFrameOwnerProperties());
     RefPtrWillBeRawPtr<LocalFrame> frame = LocalFrame::create(m_frameLoaderClientImpl.get(), oldFrame->host(), tempOwner.get());
     frame->setOwner(oldFrame->owner());
-    if (frame->owner() && !frame->owner()->isLocal())
-        toRemoteBridgeFrameOwner(frame->owner())->setSandboxFlags(static_cast<SandboxFlags>(flags));
+    if (frame->owner() && !frame->owner()->isLocal()) {
+        RemoteBridgeFrameOwner* remoteOwner = toRemoteBridgeFrameOwner(frame->owner());

[dcheng, 2015/10/14 07:20:43]

Maybe this should just call WebFrame::setReplicatedFrameOwnerProperties?

[dcheng, 2015/10/14 07:57:08]

Also, can we add a comment as to why this is necessary? I assume that this is
because a remote frame doesn't get the notifications that would update these
flags/properties, so we need to sync it up?

If that's the case, then how do we ensure these are still in sync when the frame
is actually swapped in before committing the provisional load?

[lazyboy, 2015/10/14 20:05:29]

Yes.
in A-embed B-embed C scenario, if we navigate C to A's site instance, then
in A's process during remote->local, it doesn't have any idea about the frame
owner properties for the frame. This will ensure we have them right.
Added comment.
Can you elaborate this one?

[lazyboy, 2015/10/14 20:05:29]

WebFrame::setFrameOwnerProperties needs core Frame for |frame| which is not set
until line 2021, causes a crash.

[dcheng, 2015/10/15 06:06:53]

We prepare a local frame when we have a provisional navigation. Between the time
the local frame is prepared and the time it's actually committed, a lot of other
stuff can run. If these properties are changed between these two times, then it
seems like we wouldn't get the correct properties.
Can we move this block to line 2022 then?

[lazyboy, 2015/10/15 16:56:38]

OK.
If I understand correctly, if these properties are dynamically
set using JavaScript during that time, we'll be off, right?
Now since these properties currently do not take effect immediately
and we actually have to navigate the frame to see its effect, we
should be fine IMO. Note that this is true for non --site-per-process
too.
Moved, I'll ask Alex if it's OK to move setSandboxFlags there too.

[alexmos, 2015/10/16 17:17:01]

I think this should be ok.  This needs to happens before
Document::initSecurityContext gets called, which shouldn't happen before
frame->init().

+        remoteOwner->setSandboxFlags(static_cast<SandboxFlags>(flags));
+        remoteOwner->setScrollingMode(frameOwnerProperties.scrollingMode);
+        remoteOwner->setMarginWidth(frameOwnerProperties.marginWidth);
+        remoteOwner->setMarginHeight(frameOwnerProperties.marginHeight);
+    }
+
     frame->tree().setName(name);
     setParent(oldWebFrame->parent());
     setOpener(oldWebFrame->opener());
     setCoreFrame(frame);
     // We must call init() after m_frame is assigned because it is referenced
     // during init(). Note that this may dispatch JS events; the frame may be
     // detached after init() returns.
     frame->init();
 }
 
@@ skipping 178 matching lines @@
 }
 
 WebSandboxFlags WebLocalFrameImpl::effectiveSandboxFlags() const
 {
     if (!frame())
         return WebSandboxFlags::None;
     return static_cast<WebSandboxFlags>(frame()->loader().effectiveSandboxFlags());
 }
 
 } // namespace blink