Commit f8ffcc9d authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Fix a case where a newly open()ed window incorrectly shows its url as about:blank

This case is specific to when window.open() is used, then the opener
write()s into the newly opened document and adds a <meta refresh> tag
with the refresh url set to the opener's url.

When document.write() is called from another document, the targeted
document's url is updated to the caller's url. Therefore, the act of
writing the <meta> into the newly opened window causes the opener and
the opened window to have the same url. Because the <meta refresh>'s
url matches the current one, it is flagged as a reload, rather than as
a redirect to a new url. However, the browser process has not seen a
commit in the newly opened window, so it gets confused and refuses to
update the url for a reload when a reload shouldn't be possible.

This CL detects that case and ensures that it is treated as a
non-reload navigation, which ensures the state sent to the browser
process is coherent.

Bug: 1112815
Test: http/tests/history/document-write-meta-refresh-in-opened-window.html
Change-Id: Ie665bdf40435728bf6bd4fae5ed8ffda99014800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359377
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800614}
parent 81954b9b
...@@ -358,7 +358,8 @@ void FrameLoader::DidExplicitOpen() { ...@@ -358,7 +358,8 @@ void FrameLoader::DidExplicitOpen() {
// Calling document.open counts as committing the first real document load. // Calling document.open counts as committing the first real document load.
if (!state_machine_.CommittedFirstRealDocumentLoad()) if (!state_machine_.CommittedFirstRealDocumentLoad())
state_machine_.AdvanceTo(FrameLoaderStateMachine::kCommittedFirstRealLoad); state_machine_.AdvanceTo(FrameLoaderStateMachine::kCommittedFirstRealLoad);
has_loaded_non_empty_document_ = true; if (empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty)
empty_document_status_ = EmptyDocumentStatus::kOnlyEmptyButExplicitlyOpened;
// Only model a document.open() as part of a navigation if its parent is not // Only model a document.open() as part of a navigation if its parent is not
// done or in the process of completing. // done or in the process of completing.
...@@ -510,8 +511,10 @@ WebFrameLoadType FrameLoader::DetermineFrameLoadType( ...@@ -510,8 +511,10 @@ WebFrameLoadType FrameLoader::DetermineFrameLoadType(
// both in the renderer and in the browser. // both in the renderer and in the browser.
if (frame_load_type == WebFrameLoadType::kStandard || if (frame_load_type == WebFrameLoadType::kStandard ||
frame_load_type == WebFrameLoadType::kReplaceCurrentItem) { frame_load_type == WebFrameLoadType::kReplaceCurrentItem) {
if (frame_->Tree().Parent() && !has_loaded_non_empty_document_) if (frame_->Tree().Parent() &&
empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty) {
return WebFrameLoadType::kReplaceCurrentItem; return WebFrameLoadType::kReplaceCurrentItem;
}
if (!frame_->Tree().Parent() && !Client()->BackForwardLength()) { if (!frame_->Tree().Parent() && !Client()->BackForwardLength()) {
if (Opener() && url.IsEmpty()) if (Opener() && url.IsEmpty())
return WebFrameLoadType::kReplaceCurrentItem; return WebFrameLoadType::kReplaceCurrentItem;
...@@ -1036,7 +1039,7 @@ void FrameLoader::CommitNavigation( ...@@ -1036,7 +1039,7 @@ void FrameLoader::CommitNavigation(
tls_version_warning_origins_.clear(); tls_version_warning_origins_.clear();
if (!DocumentLoader::WillLoadUrlAsEmpty(navigation_params->url)) if (!DocumentLoader::WillLoadUrlAsEmpty(navigation_params->url))
has_loaded_non_empty_document_ = true; empty_document_status_ = EmptyDocumentStatus::kNonEmpty;
// TODO(dgozman): navigation type should probably be passed by the caller. // TODO(dgozman): navigation type should probably be passed by the caller.
// It seems incorrect to pass |false| for |have_event| and then use // It seems incorrect to pass |false| for |have_event| and then use
......
...@@ -229,7 +229,12 @@ class CORE_EXPORT FrameLoader final { ...@@ -229,7 +229,12 @@ class CORE_EXPORT FrameLoader final {
bool HasAccessedInitialDocument() { return has_accessed_initial_document_; } bool HasAccessedInitialDocument() { return has_accessed_initial_document_; }
void SetDidLoadNonEmptyDocument() { has_loaded_non_empty_document_ = true; } void SetDidLoadNonEmptyDocument() {
empty_document_status_ = EmptyDocumentStatus::kNonEmpty;
}
bool HasLoadedNonEmptyDocument() {
return empty_document_status_ == EmptyDocumentStatus::kNonEmpty;
}
static bool NeedsHistoryItemRestore(WebFrameLoadType type); static bool NeedsHistoryItemRestore(WebFrameLoadType type);
...@@ -310,7 +315,13 @@ class CORE_EXPORT FrameLoader final { ...@@ -310,7 +315,13 @@ class CORE_EXPORT FrameLoader final {
bool detached_; bool detached_;
bool committing_navigation_ = false; bool committing_navigation_ = false;
bool has_accessed_initial_document_ = false; bool has_accessed_initial_document_ = false;
bool has_loaded_non_empty_document_ = false;
enum class EmptyDocumentStatus {
kOnlyEmpty,
kOnlyEmptyButExplicitlyOpened,
kNonEmpty
};
EmptyDocumentStatus empty_document_status_ = EmptyDocumentStatus::kOnlyEmpty;
WebScopedVirtualTimePauser virtual_time_pauser_; WebScopedVirtualTimePauser virtual_time_pauser_;
......
...@@ -105,9 +105,14 @@ void HttpRefreshScheduler::NavigateTask() { ...@@ -105,9 +105,14 @@ void HttpRefreshScheduler::NavigateTask() {
request.SetInputStartTime(refresh->input_timestamp); request.SetInputStartTime(refresh->input_timestamp);
request.SetClientRedirectReason(refresh->reason); request.SetClientRedirectReason(refresh->reason);
// We want a new back/forward list item if the refresh timeout is > 1 second.
WebFrameLoadType load_type = WebFrameLoadType::kStandard; WebFrameLoadType load_type = WebFrameLoadType::kStandard;
if (EqualIgnoringFragmentIdentifier(document_->Url(), refresh->url)) { // If the urls match, process the refresh as a reload. However, if an initial
// empty document has its url modified via document.open() and the refresh is
// to that url, it will confuse the browser process to report it as a reload
// in a frame where there hasn't actually been a navigation yet. Therefore,
// don't treat as a reload if all this frame has ever seen is empty documents.
if (EqualIgnoringFragmentIdentifier(document_->Url(), refresh->url) &&
document_->GetFrame()->Loader().HasLoadedNonEmptyDocument()) {
request.GetResourceRequest().SetCacheMode( request.GetResourceRequest().SetCacheMode(
mojom::FetchCacheMode::kValidateCache); mojom::FetchCacheMode::kValidateCache);
load_type = WebFrameLoadType::kReload; load_type = WebFrameLoadType::kReload;
......
============== Back Forward List ==============
curr-> http://127.0.0.1:8000/history/document-write-meta-refresh-in-opened-window.html
===============================================
============== Back Forward List ==============
curr-> http://127.0.0.1:8000/history/document-write-meta-refresh-in-opened-window.html
===============================================
<script>
if (window.testRunner) {
testRunner.waitUntilDone();
testRunner.setCanOpenWindows();
testRunner.dumpAsText();
testRunner.dumpBackForwardList();
if (window.opener)
testRunner.notifyDone();
}
if (!window.opener) {
var w = window.open();
w.document.write(
"<meta http-equiv='refresh' content='0; url='" + location.toString() + ">");
w.document.close();
}
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment