Reland "Copy attributes of old DocumentLoader on javascript: URL & XSLT commits"

This is a reland of 55f04b52

Original change's description:
> Copy attributes of old DocumentLoader on javascript: URL & XSLT commits
>
> When javascript: URL evaluates into a string, or XSLT document change
> happens, we will commit a new document but won't notify the browser of
> the change of documents, as no URL/origin change happens, essentially
> treating it as just a content update within the previously committed
> document.
>
> However, we actually create a new DocumentLoader and Document for the
> commit, and not all attributes of the old document are actually carried
> over to the new document. This CL copies most attributes from the
> previous document through DocumentLoader::CreateParamsToClone(), to
> ensure future navigations within the new document won't result in
> confusion in the browser. Some examples of things that are now copied to
> the new document: HistoryItem, http_method, http_status_code,
> devtools_navigation_token (so XSLT document transformations show up
> on devtools), etc.
>
> Bug: 1151954, 1048106
> Change-Id: Iefd620c7c3896f2f1ae2fdcc60404361363124eb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2589206
> Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Nate Chapin <japhet@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#843047}

Bug: 1151954
Bug: 1048106
Change-Id: I045cd84d088999cb903c0899ebc40cb72219ab3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628267
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843355}

content/browser/renderer_host/render_frame_host_impl.cc

@@ -2778,6 +2778,11 @@ void RenderFrameHostImpl::DidNavigate(
   last_http_method_ = params.method;
   last_post_id_ = params.post_id;
 
+  // TODO(arthursonzogni): Stop relying on DidCommitProvisionalLoadParams. Use
+  // the NavigationRequest instead. The browser process doesn't need to rely on
+  // the renderer process.
+  last_http_status_code_ = params.http_status_code;
+
   if (did_create_new_document)
     DidCommitNewDocument(params, navigation_request);
 
@@ -8958,11 +8963,6 @@ void RenderFrameHostImpl::DidCommitNewDocument(
   DCHECK(params.embedding_token.has_value());
   SetEmbeddingToken(params.embedding_token.value());
 
-  // TODO(arthursonzogni): Stop relying on DidCommitProvisionalLoadParams. Use
-  // the NavigationRequest instead. The browser process doesn't need to rely on
-  // the renderer process.
-  last_http_status_code_ = params.http_status_code;
-
   renderer_reported_scheduler_tracked_features_ = 0;
   browser_reported_scheduler_tracked_features_ = 0;
 

third_party/blink/public/web/web_document_loader.h

@@ -116,6 +116,7 @@ class BLINK_EXPORT WebDocumentLoader {
   // Extra data associated with this DocumentLoader.
   // Setting extra data will cause any existing extra data to be deleted.
   virtual ExtraData* GetExtraData() const = 0;
+  virtual std::unique_ptr<ExtraData> TakeExtraData() = 0;
   virtual void SetExtraData(std::unique_ptr<ExtraData>) = 0;
 
   // Allows the embedder to inject a filter that will be consulted for each

third_party/blink/renderer/bindings/core/v8/script_controller.cc

@@ -257,20 +257,11 @@ void ScriptController::ExecuteJavaScriptURL(
   UseCounter::Count(window_.Get(),
                     WebFeature::kReplaceDocumentViaJavaScriptURL);
 
-  // From the browser process point of view, committing a javascript-URL, an
-  // XSLT document or a document.open are all a no-op. All the security
-  // properties of the document must be preserved.
-  auto params = std::make_unique<WebNavigationParams>();
-  params->url = window_->Url();
-  // TODO(https://crbug.com/1151954): Consider inheriting the feature-policy
-  // from the previous document. Here we might miss the one defined from the
-  // original network request.
-  params->frame_policy = FramePolicy();
-  if (auto* owner = window_->GetFrame()->Owner())
-    params->frame_policy = owner->GetFramePolicy();
-  params->frame_policy->sandbox_flags = window_->GetSandboxFlags();
-  params->origin_to_commit = window_->GetSecurityOrigin();
-
+  auto* previous_document_loader =
+      window_->GetFrame()->Loader().GetDocumentLoader();
+  DCHECK(previous_document_loader);
+  auto params =
+      previous_document_loader->CreateWebNavigationParamsToCloneDocument();
   String result = ToCoreString(v8::Local<v8::String>::Cast(v8_result));
   WebNavigationParams::FillStaticResponse(params.get(), "text/html", "UTF-8",
                                           StringUTF8Adaptor(result));

third_party/blink/renderer/core/exported/web_document_loader_impl.cc

@@ -113,6 +113,11 @@ WebDocumentLoader::ExtraData* WebDocumentLoaderImpl::GetExtraData() const {
   return extra_data_.get();
 }
 
+std::unique_ptr<WebDocumentLoader::ExtraData>
+WebDocumentLoaderImpl::TakeExtraData() {
+  return std::move(extra_data_);
+}
+
 void WebDocumentLoaderImpl::SetExtraData(
     std::unique_ptr<ExtraData> extra_data) {
   extra_data_ = std::move(extra_data);

third_party/blink/renderer/core/exported/web_document_loader_impl.h

@@ -75,6 +75,7 @@ class CORE_EXPORT WebDocumentLoaderImpl final : public DocumentLoader,
   bool ReplacesCurrentHistoryItem() const override;
   WebNavigationType GetNavigationType() const override;
   ExtraData* GetExtraData() const override;
+  std::unique_ptr<ExtraData> TakeExtraData() override;
   void SetExtraData(std::unique_ptr<ExtraData>) override;
   void SetSubresourceFilter(WebDocumentSubresourceFilter*) override;
   void SetServiceWorkerNetworkProvider(

third_party/blink/renderer/core/loader/document_loader.h

@@ -114,6 +114,12 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
                  std::unique_ptr<WebNavigationParams> navigation_params);
   ~DocumentLoader() override;
 
+  // Returns WebNavigationParams that can be used to clone DocumentLoader. Used
+  // for javascript: URL and XSLT commits, where we want to create a new
+  // Document but keep most of the property of the current DocumentLoader.
+  std::unique_ptr<WebNavigationParams>
+  CreateWebNavigationParamsToCloneDocument();
+
   static bool WillLoadUrlAsEmpty(const KURL&);
 
   LocalFrame* GetFrame() const { return frame_; }
@@ -217,6 +223,8 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
   };
   InitialScrollState& GetInitialScrollState() { return initial_scroll_state_; }
 
+  enum State { kNotStarted, kProvisional, kCommitted, kSentDidFinishLoad };
+
   void DispatchLinkHeaderPreloads(const ViewportDescription*,
                                   PreloadHelper::MediaPreloadPolicy);
 
@@ -292,7 +300,8 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
 
   void SetHistoryItemStateForCommit(HistoryItem* old_item,
                                     WebFrameLoadType,
-                                    HistoryNavigationType);
+                                    HistoryNavigationType,
+                                    CommitReason commit_reason);
 
   mojo::PendingReceiver<mojom::blink::WorkerTimingContainer>
   TakePendingWorkerTimingReceiver(int request_id);
@@ -497,7 +506,6 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
   ClientHintsPreferences client_hints_preferences_;
   InitialScrollState initial_scroll_state_;
 
-  enum State { kNotStarted, kProvisional, kCommitted, kSentDidFinishLoad };
   State state_;
 
   // Used to block the parser.

third_party/blink/renderer/core/loader/frame_loader.cc

@@ -67,6 +67,7 @@
 #include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/ignore_opens_during_unload_count_incrementer.h"
 #include "third_party/blink/renderer/core/events/page_transition_event.h"
+#include "third_party/blink/renderer/core/exported/web_document_loader_impl.h"
 #include "third_party/blink/renderer/core/exported/web_plugin_container_impl.h"
 #include "third_party/blink/renderer/core/frame/csp/content_security_policy.h"
 #include "third_party/blink/renderer/core/frame/csp/csp_source.h"
@@ -1035,6 +1036,18 @@ void FrameLoader::CommitNavigation(
         std::move(navigation_params->policy_container)));
   }
 
+  // If this is a javascript: URL or XSLT commit, we must copy the ExtraData
+  // from the previous DocumentLoader to ensure the new DocumentLoader behaves
+  // the same way as the previous one.
+  if (commit_reason == CommitReason::kXSLT ||
+      commit_reason == CommitReason::kJavascriptUrl) {
+    DCHECK(!extra_data);
+    if (auto* old_document_loader =
+            static_cast<WebDocumentLoaderImpl*>(document_loader_.Get())) {
+      extra_data = old_document_loader->TakeExtraData();
+    }
+  }
+
   base::Optional<Document::UnloadEventTiming> unload_timing;
   FrameSwapScope frame_swap_scope(frame_owner);
   {
@@ -1227,7 +1240,8 @@ void FrameLoader::CommitDocumentLoader(
     // of the previous Document.
     document_loader_->SetHistoryItemStateForCommit(
         previous_history_item, document_loader_->LoadType(),
-        DocumentLoader::HistoryNavigationType::kDifferentDocument);
+        DocumentLoader::HistoryNavigationType::kDifferentDocument,
+        commit_reason);
   }
 
   // Update the DocumentLoadTiming with the timings from the previous document

third_party/blink/renderer/core/xml/xslt_processor.cc

@@ -88,8 +88,10 @@ Document* XSLTProcessor::CreateDocumentFromSource(
   }
 
   if (frame) {
-    auto params = std::make_unique<WebNavigationParams>();
-    params->url = url;
+    auto* previous_document_loader = frame->Loader().GetDocumentLoader();
+    DCHECK(previous_document_loader);
+    std::unique_ptr<WebNavigationParams> params =
+        previous_document_loader->CreateWebNavigationParamsToCloneDocument();
     WebNavigationParams::FillStaticResponse(
         params.get(), mime_type,
         source_encoding.IsEmpty() ? "UTF-8" : source_encoding,

third_party/blink/renderer/modules/media_controls/media_controls_orientation_lock_delegate_test.cc

@@ -1491,45 +1491,4 @@ TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
   EXPECT_TRUE(DelegateWillUnlockFullscreen());
 }
 
-TEST_F(MediaControlsOrientationLockAndRotateToFullscreenDelegateTest,
-       DetachBeforeChangeLockToAnyOrientation) {
-  // Naturally portrait device, initially portrait, with landscape video.
-  natural_orientation_is_portrait_ = true;
-  ASSERT_NO_FATAL_FAILURE(
-      RotateScreenTo(mojom::blink::ScreenOrientation::kPortraitPrimary, 0));
-  InitVideo(640, 480);
-  SetIsAutoRotateEnabledByUser(true);
-
-  // Initially inline, unlocked orientation.
-  ASSERT_FALSE(Video().IsFullscreen());
-  CheckStatePendingFullscreen();
-  ASSERT_FALSE(DelegateWillUnlockFullscreen());
-
-  // Simulate user clicking on media controls fullscreen button.
-  SimulateEnterFullscreen();
-  EXPECT_TRUE(Video().IsFullscreen());
-
-  // MediaControlsOrientationLockDelegate should lock to landscape.
-  CheckStateMaybeLockedFullscreen();
-  EXPECT_EQ(device::mojom::ScreenOrientationLockType::LANDSCAPE,
-            DelegateOrientationLock());
-
-  // This will trigger a screen orientation change to landscape.
-  ASSERT_NO_FATAL_FAILURE(
-      RotateScreenTo(mojom::blink::ScreenOrientation::kLandscapePrimary, 90));
-
-  // Rotate the device to match.
-  RotateDeviceTo(90 /* landscape primary */);
-
-  // And immediately detach the document by synchronously navigating.
-  // One easy way to do this is to replace the document with a JavaScript URL.
-  GetFrame().GetSettings()->SetScriptEnabled(true);
-  FrameLoadRequest request(GetFrame().DomWindow(),
-                           ResourceRequest("javascript:'Hello, world!'"));
-  GetFrame().Navigate(request, WebFrameLoadType::kStandard);
-
-  // We should not crash after the unlock delay.
-  test::RunDelayedTasks(GetUnlockDelay());
-}
-
 }  // namespace blink

third_party/blink/web_tests/http/tests/devtools/network/network-xsl-content-expected.txt

@@ -2,11 +2,15 @@ Tests XSL stylsheet content. http://crbug.com/603806
 
 http://127.0.0.1:8000/devtools/network/resources/xml-with-stylesheet.xml
 resource.type: document
-resource.content: <?xml version="1.0"?>
-<?xml-stylesheet type="text/xsl" href="xml-with-stylesheet.xsl"?>
-<foo>
-  <bar>someData</bar>
-</foo>
+resource.content: <?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head><title/></head>
+<body>
+<pre>
+    The Data: someData</pre>
+</body>
+</html>
 
 http://127.0.0.1:8000/devtools/network/resources/xml-with-stylesheet.xsl
 resource.type: stylesheet