Cleanup createNewWindow

* Use LocalDOMWindow rather than LocalFrame/Document for most things.
* Change probe::WindowOpen() to take a KURL rather than a url String,
  to avoid an unnecessary CompleteURL() call.
* Remove an obsolete TODO, which refers to a Frame being remote when
  at a point where it is known to be local.

Change-Id: I0860485758fa1ff12843bf965e03c9816e5f2390
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414277
Commit-Queue: Nate Chapin <japhet@chromium.org>
Auto-Submit: Nate Chapin <japhet@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808601}

third_party/blink/renderer/core/inspector/inspector_page_agent.cc

@@ -1051,13 +1051,11 @@ void InspectorPageAgent::PageLayoutInvalidated(bool resized) {
     client_->PageLayoutInvalidated(resized);
 }
 
-void InspectorPageAgent::WindowOpen(Document* document,
-                                    const String& url,
+void InspectorPageAgent::WindowOpen(const KURL& url,
                                     const AtomicString& window_name,
                                     const WebWindowFeatures& window_features,
                                     bool user_gesture) {
-  KURL completed_url = url.IsEmpty() ? BlankURL() : document->CompleteURL(url);
-  GetFrontend()->windowOpen(completed_url.GetString(), window_name,
+  GetFrontend()->windowOpen(url.IsEmpty() ? BlankURL() : url, window_name,
                             GetEnabledWindowFeatures(window_features),
                             user_gesture);
   GetFrontend()->flush();

third_party/blink/renderer/core/inspector/inspector_page_agent.h

@@ -201,8 +201,7 @@ class CORE_EXPORT InspectorPageAgent final
   void Did(const probe::UpdateLayout&);
   void Will(const probe::RecalculateStyle&);
   void Did(const probe::RecalculateStyle&);
-  void WindowOpen(Document*,
-                  const String&,
+  void WindowOpen(const KURL&,
                   const AtomicString&,
                   const WebWindowFeatures&,
                   bool);

third_party/blink/renderer/core/page/create_window.cc

@@ -226,15 +226,16 @@ static void MaybeLogWindowOpen(LocalFrame& opener_frame) {
 Frame* CreateNewWindow(LocalFrame& opener_frame,
                        FrameLoadRequest& request,
                        const AtomicString& frame_name) {
+  LocalDOMWindow& opener_window = *opener_frame.DomWindow();
   DCHECK(request.GetResourceRequest().RequestorOrigin() ||
-         opener_frame.GetDocument()->Url().IsEmpty());
+         opener_window.Url().IsEmpty());
   DCHECK_EQ(kNavigationPolicyCurrentTab, request.GetNavigationPolicy());
 
   // Exempting window.open() from this check here is necessary to support a
   // special policy that will be removed in Chrome 88.
   // See https://crbug.com/937569
   if (!request.IsWindowOpen() &&
-      opener_frame.GetDocument()->PageDismissalEventBeingDispatched() !=
+      opener_window.document()->PageDismissalEventBeingDispatched() !=
           Document::kNoDismissal) {
     return nullptr;
   }
@@ -242,61 +243,57 @@ Frame* CreateNewWindow(LocalFrame& opener_frame,
   request.SetFrameType(mojom::RequestContextFrameType::kAuxiliary);
 
   const KURL& url = request.GetResourceRequest().Url();
-  if (url.ProtocolIsJavaScript() &&
-      opener_frame.DomWindow()->GetContentSecurityPolicyForCurrentWorld()) {
+  auto* csp_for_world = opener_window.GetContentSecurityPolicyForCurrentWorld();
+  if (url.ProtocolIsJavaScript() && csp_for_world) {
     String script_source = DecodeURLEscapeSequences(
         url.GetString(), DecodeURLMode::kUTF8OrIsomorphic);
 
-    if (!opener_frame.DomWindow()
-             ->GetContentSecurityPolicyForCurrentWorld()
-             ->AllowInline(ContentSecurityPolicy::InlineType::kNavigation,
-                           nullptr /* element */, script_source,
-                           String() /* nonce */,
-                           opener_frame.DomWindow()->Url(), OrdinalNumber())) {
+    if (!csp_for_world->AllowInline(
+            ContentSecurityPolicy::InlineType::kNavigation,
+            nullptr /* element */, script_source, String() /* nonce */,
+            opener_window.Url(), OrdinalNumber())) {
       return nullptr;
     }
   }
 
-  if (!opener_frame.DomWindow()->GetSecurityOrigin()->CanDisplay(url)) {
-    opener_frame.DomWindow()->AddConsoleMessage(
-        MakeGarbageCollected<ConsoleMessage>(
-            mojom::ConsoleMessageSource::kSecurity,
-            mojom::ConsoleMessageLevel::kError,
-            "Not allowed to load local resource: " + url.ElidedString()));
+  if (!opener_window.GetSecurityOrigin()->CanDisplay(url)) {
+    opener_window.AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+        mojom::blink::ConsoleMessageSource::kSecurity,
+        mojom::blink::ConsoleMessageLevel::kError,
+        "Not allowed to load local resource: " + url.ElidedString()));
     return nullptr;
   }
 
   const WebWindowFeatures& features = request.GetWindowFeatures();
   request.SetNavigationPolicy(NavigationPolicyForCreateWindow(features));
-  probe::WindowOpen(opener_frame.GetDocument(), url, frame_name, features,
+  probe::WindowOpen(&opener_window, url, frame_name, features,
                     LocalFrame::HasTransientUserActivation(&opener_frame));
 
   // Sandboxed frames cannot open new auxiliary browsing contexts.
-  if (opener_frame.DomWindow()->IsSandboxed(
+  if (opener_window.IsSandboxed(
           network::mojom::blink::WebSandboxFlags::kPopups)) {
     // FIXME: This message should be moved off the console once a solution to
     // https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
-    opener_frame.DomWindow()->AddConsoleMessage(
-        MakeGarbageCollected<ConsoleMessage>(
-            mojom::ConsoleMessageSource::kSecurity,
-            mojom::ConsoleMessageLevel::kError,
-            "Blocked opening '" + url.ElidedString() +
-                "' in a new window because the request was made in a sandboxed "
-                "frame whose 'allow-popups' permission is not set."));
+    opener_window.AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+        mojom::blink::ConsoleMessageSource::kSecurity,
+        mojom::blink::ConsoleMessageLevel::kError,
+        "Blocked opening '" + url.ElidedString() +
+            "' in a new window because the request was made in a sandboxed "
+            "frame whose 'allow-popups' permission is not set."));
     return nullptr;
   }
 
-  bool propagate_sandbox = opener_frame.DomWindow()->IsSandboxed(
-      network::mojom::blink::WebSandboxFlags::
-          kPropagatesToAuxiliaryBrowsingContexts);
+  bool propagate_sandbox =
+      opener_window.IsSandboxed(network::mojom::blink::WebSandboxFlags::
+                                    kPropagatesToAuxiliaryBrowsingContexts);
   network::mojom::blink::WebSandboxFlags sandbox_flags =
-      propagate_sandbox ? opener_frame.DomWindow()->GetSandboxFlags()
+      propagate_sandbox ? opener_window.GetSandboxFlags()
                         : network::mojom::blink::WebSandboxFlags::kNone;
-  bool not_sandboxed = opener_frame.DomWindow()->GetSandboxFlags() ==
+  bool not_sandboxed = opener_window.GetSandboxFlags() ==
                        network::mojom::blink::WebSandboxFlags::kNone;
   FeaturePolicyFeatureState opener_feature_state =
-      (not_sandboxed || propagate_sandbox) ? opener_frame.GetSecurityContext()
-                                                 ->GetFeaturePolicy()
+      (not_sandboxed || propagate_sandbox) ? opener_window.GetSecurityContext()
+                                                 .GetFeaturePolicy()
                                                  ->GetFeatureState()
                                            : FeaturePolicyFeatureState();
 
@@ -316,16 +313,6 @@ Frame* CreateNewWindow(LocalFrame& opener_frame,
   if (!page)
     return nullptr;
 
-  auto* new_local_frame = DynamicTo<LocalFrame>(page->MainFrame());
-  if (request.GetShouldSendReferrer() == kMaybeSendReferrer) {
-    // TODO(japhet): Does network::mojom::ReferrerPolicy need to be proagated
-    // for RemoteFrames?
-    if (new_local_frame) {
-      new_local_frame->DomWindow()->SetReferrerPolicy(
-          opener_frame.DomWindow()->GetReferrerPolicy());
-    }
-  }
-
   if (page == old_page) {
     Frame* frame = &opener_frame.Tree().Top();
     if (!opener_frame.CanNavigate(*frame))
@@ -338,6 +325,9 @@ Frame* CreateNewWindow(LocalFrame& opener_frame,
   DCHECK(page->MainFrame());
   LocalFrame& frame = *To<LocalFrame>(page->MainFrame());
 
+  if (request.GetShouldSendReferrer() == kMaybeSendReferrer)
+    frame.DomWindow()->SetReferrerPolicy(opener_window.GetReferrerPolicy());
+
   page->SetWindowFeatures(features);
 
   frame.View()->SetCanHaveScrollbars(features.scrollbars_visible);

third_party/blink/renderer/core/probe/core_probes.pidl

@@ -142,7 +142,7 @@ interface CoreProbes {
   void PseudoElementDestroyed([Keep] PseudoElement*);
   void DidCreateAnimation(Document*, unsigned);
   void AnimationPlayStateChanged(Document*, Animation*, Animation::AnimationPlayState old_play_state, Animation::AnimationPlayState new_play_state);
-  void WindowOpen([Keep] Document*, const String& url, const AtomicString& window_name, const WebWindowFeatures& window_features, bool user_gestrue);
+  void WindowOpen(ExecutionContext*, const KURL& url, const AtomicString& window_name, const WebWindowFeatures& window_features, bool user_gestrue);
   void ConsoleMessageAdded(ExecutionContext*, ConsoleMessage*);
   void InspectorIssueAdded(CoreProbeSink*, InspectorIssue*);
   void WillRunJavaScriptDialog(LocalFrame* frame);