Cleanup around LocalFrame initialization

third_party/WebKit/Source/web/WebRemoteFrameImpl.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "web/WebRemoteFrameImpl.h"
 
 #include "bindings/core/v8/WindowProxy.h"
 #include "core/dom/Fullscreen.h"
 #include "core/dom/RemoteSecurityContext.h"
 #include "core/dom/SecurityContext.h"
@@ skipping 314 matching lines @@
     WebFrame* previous_sibling,
     const WebParsedFeaturePolicy& container_policy,
     const WebFrameOwnerProperties& frame_owner_properties,
     WebFrame* opener) {
   WebLocalFrameImpl* child = WebLocalFrameImpl::Create(
       scope, client, interface_provider, interface_registry, opener);
   InsertAfter(child, previous_sibling);
   RemoteFrameOwner* owner =
       RemoteFrameOwner::Create(static_cast<SandboxFlags>(sandbox_flags),
                                container_policy, frame_owner_properties);
-  // FIXME: currently this calls LocalFrame::init() on the created LocalFrame,

[Nate Chapin, 2017/05/10 16:33:20]

I don't think this comment was correct, given that initial empty documents are
*supposed* to be invisible to content/ (unless they get scripted and
DidAccessInitialDocument gets called). Certainly, navigation notifications
should be suppressed.

-  // which may result in the browser observing two navigations to about:blank
-  // (one from the initial frame creation, and one from swapping it into the
-  // remote process).  FrameLoader might need a special initialization function
-  // for this case to avoid that duplicate navigation.
   child->InitializeCoreFrame(*GetFrame()->GetPage(), owner, name);
-  // Partially related with the above FIXME--the init() call may trigger JS
-  // dispatch. However,

[Nate Chapin, 2017/05/10 16:33:20]

This comment is now obsolete.

[dcheng, 2017/05/11 21:36:25]

Should we update the comment in LocalFrame::Init() as well?

[Nate Chapin, 2017/05/12 19:52:29]

Dropped it, it doesn't look like there's any substance besides the claim that
FrameLoader::Init() can detach the world.

-  // if the parent is remote, it should never be detached synchronously...
   DCHECK(child->GetFrame());
   return child;
 }
 
 void WebRemoteFrameImpl::InitializeCoreFrame(Page& page,
                                              FrameOwner* owner,
                                              const AtomicString& name) {
   SetCoreFrame(RemoteFrame::Create(frame_client_.Get(), page, owner));
   GetFrame()->CreateView();
   frame_->Tree().SetName(name);
@@ skipping 177 matching lines @@
 }
 
 WebRemoteFrameImpl::WebRemoteFrameImpl(WebTreeScopeType scope,
                                        WebRemoteFrameClient* client)
     : WebRemoteFrame(scope),
       frame_client_(RemoteFrameClientImpl::Create(this)),
       client_(client),
       self_keep_alive_(this) {}
 
 }  // namespace blink