Commit 46c7d082 authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Remove all non-DocumentLoader uses of LocalDOMWindow::InstallNewDocument

LocalFrame::ForceSynchronousDocumentInstall and a few unit tests
manually replace the Document of a LocalDOMWindow. They can be switched
to go through the static-response navigation commit path relatively
easily. This establishes the rule that only DocumentLoader is allowed
to create a LocalDOMWindow or replace its Document.

Bug: 1029822
Change-Id: I84b37f35be5387556999f9bc24f7ba5fa74b4ff8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2222776
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773720}
parent ad9b5dfc
......@@ -155,6 +155,8 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
void CountUse(mojom::WebFeature feature) final;
void CountDeprecation(mojom::WebFeature feature) final;
// Initialize and install a Document for navigation. Should only be used by
// DocumentLoader.
Document* InstallNewDocument(const DocumentInit&);
// EventTarget overrides:
......
......@@ -1673,29 +1673,12 @@ void LocalFrame::ForceSynchronousDocumentInstall(
scoped_refptr<SharedBuffer> data) {
CHECK(loader_.StateMachine()->IsDisplayingInitialEmptyDocument());
DCHECK(!Client()->IsLocalFrameClientImpl());
// Any Document requires Shutdown() before detach, even the initial empty
// document.
GetDocument()->Shutdown();
DomWindow()->InstallNewDocument(
DocumentInit::Create()
.WithDocumentLoader(loader_.GetDocumentLoader())
.WithTypeFrom(mime_type));
loader_.StateMachine()->AdvanceTo(
FrameLoaderStateMachine::kCommittedFirstRealLoad);
GetDocument()->OpenForNavigation(kForceSynchronousParsing, mime_type,
AtomicString("UTF-8"));
for (const auto& segment : *data)
GetDocument()->Parser()->AppendBytes(segment.data(), segment.size());
GetDocument()->Parser()->Finish();
// Upon loading of SVGImages, log PageVisits in UseCounter.
// Do not track PageVisits for inspector, web page popups, and validation
// message overlays (the other callers of this method).
if (GetDocument()->IsSVGDocument())
loader_.GetDocumentLoader()->GetUseCounterHelper().DidCommitLoad(this);
auto params = std::make_unique<WebNavigationParams>();
WebNavigationParams::FillStaticResponse(
params.get(), mime_type, "UTF-8",
base::make_span(data->Data(), data->size()));
loader_.CommitNavigation(std::move(params), nullptr,
CommitReason::kForcedSync);
}
bool LocalFrame::IsProvisional() const {
......
......@@ -19,6 +19,7 @@
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/core/testing/sim/sim_request.h"
#include "third_party/blink/renderer/core/testing/sim/sim_test.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
namespace blink {
......@@ -83,8 +84,6 @@ class ImageDocumentTest : public testing::Test {
void CreateDocumentWithoutLoadingImage(int view_width, int view_height);
void CreateDocument(int view_width, int view_height);
DocumentParser* StartLoadingImage();
void LoadImage();
ImageDocument& GetDocument() const;
......@@ -93,10 +92,14 @@ class ImageDocumentTest : public testing::Test {
void SetPageZoom(float);
void SetWindowToViewportScalingFactor(float);
void SetForceZeroLayoutHeight(bool);
private:
Persistent<WindowToViewportScalingChromeClient> chrome_client_;
std::unique_ptr<DummyPageHolder> dummy_page_holder_;
float page_zoom_factor_ = 0.0f;
float viewport_scaling_factor_ = 0.0f;
base::Optional<bool> force_zero_layout_height_;
};
void ImageDocumentTest::CreateDocumentWithoutLoadingImage(int view_width,
......@@ -109,19 +112,29 @@ void ImageDocumentTest::CreateDocumentWithoutLoadingImage(int view_width,
dummy_page_holder_ = std::make_unique<DummyPageHolder>(
IntSize(view_width, view_height), &page_clients);
LocalFrame& frame = dummy_page_holder_->GetFrame();
frame.GetDocument()->Shutdown();
DocumentInit init =
DocumentInit::Create()
.WithDocumentLoader(frame.Loader().GetDocumentLoader())
.WithTypeFrom("image/jpeg");
frame.DomWindow()->InstallNewDocument(init);
frame.GetDocument()->SetURL(KURL("http://www.example.com/image.jpg"));
if (page_zoom_factor_)
dummy_page_holder_->GetFrame().SetPageZoomFactor(page_zoom_factor_);
if (viewport_scaling_factor_)
chrome_client_->SetScalingFactor(viewport_scaling_factor_);
if (force_zero_layout_height_.has_value()) {
dummy_page_holder_->GetPage().GetSettings().SetForceZeroLayoutHeight(
force_zero_layout_height_.value());
}
auto params = std::make_unique<WebNavigationParams>();
params->url = KURL("http://www.example.com/image.jpg");
const Vector<unsigned char>& data = JpegImage();
WebNavigationParams::FillStaticResponse(
params.get(), "image/jpeg", "UTF-8",
base::make_span(reinterpret_cast<const char*>(data.data()), data.size()));
dummy_page_holder_->GetFrame().Loader().CommitNavigation(std::move(params),
nullptr);
}
void ImageDocumentTest::CreateDocument(int view_width, int view_height) {
CreateDocumentWithoutLoadingImage(view_width, view_height);
LoadImage();
blink::test::RunPendingTasks();
}
ImageDocument& ImageDocumentTest::GetDocument() const {
......@@ -130,25 +143,23 @@ ImageDocument& ImageDocumentTest::GetDocument() const {
return *image_document;
}
DocumentParser* ImageDocumentTest::StartLoadingImage() {
DocumentParser* parser = GetDocument().ImplicitOpen(
ParserSynchronizationPolicy::kForceSynchronousParsing);
const Vector<unsigned char>& data = JpegImage();
parser->AppendBytes(reinterpret_cast<const char*>(data.data()), data.size());
return parser;
}
void ImageDocumentTest::LoadImage() {
DocumentParser* parser = StartLoadingImage();
parser->Finish();
}
void ImageDocumentTest::SetPageZoom(float factor) {
dummy_page_holder_->GetFrame().SetPageZoomFactor(factor);
page_zoom_factor_ = factor;
if (dummy_page_holder_)
dummy_page_holder_->GetFrame().SetPageZoomFactor(factor);
}
void ImageDocumentTest::SetWindowToViewportScalingFactor(float factor) {
chrome_client_->SetScalingFactor(factor);
viewport_scaling_factor_ = factor;
if (chrome_client_)
chrome_client_->SetScalingFactor(factor);
}
void ImageDocumentTest::SetForceZeroLayoutHeight(bool force) {
force_zero_layout_height_ = force;
if (dummy_page_holder_) {
dummy_page_holder_->GetPage().GetSettings().SetForceZeroLayoutHeight(force);
}
}
TEST_F(ImageDocumentTest, ImageLoad) {
......@@ -175,9 +186,8 @@ TEST_F(ImageDocumentTest, RestoreImageOnClick) {
}
TEST_F(ImageDocumentTest, InitialZoomDoesNotAffectScreenFit) {
CreateDocumentWithoutLoadingImage(20, 10);
SetPageZoom(2.f);
LoadImage();
CreateDocument(20, 10);
EXPECT_EQ(10, ImageWidth());
EXPECT_EQ(10, ImageHeight());
GetDocument().ImageClicked(4, 4);
......@@ -198,17 +208,15 @@ TEST_F(ImageDocumentTest, ZoomingDoesNotChangeRelativeSize) {
}
TEST_F(ImageDocumentTest, ImageScalesDownWithDsf) {
CreateDocumentWithoutLoadingImage(20, 30);
SetWindowToViewportScalingFactor(2.f);
LoadImage();
CreateDocument(20, 30);
EXPECT_EQ(10, ImageWidth());
EXPECT_EQ(10, ImageHeight());
}
TEST_F(ImageDocumentTest, ImageNotCenteredWithForceZeroLayoutHeight) {
CreateDocumentWithoutLoadingImage(80, 70);
GetDocument().GetPage()->GetSettings().SetForceZeroLayoutHeight(true);
LoadImage();
SetForceZeroLayoutHeight(true);
CreateDocument(80, 70);
EXPECT_FALSE(GetDocument().ShouldShrinkToFit());
EXPECT_EQ(0, GetDocument().ImageElement()->OffsetLeft());
EXPECT_EQ(0, GetDocument().ImageElement()->OffsetTop());
......@@ -217,9 +225,8 @@ TEST_F(ImageDocumentTest, ImageNotCenteredWithForceZeroLayoutHeight) {
}
TEST_F(ImageDocumentTest, ImageCenteredWithoutForceZeroLayoutHeight) {
CreateDocumentWithoutLoadingImage(80, 70);
GetDocument().GetPage()->GetSettings().SetForceZeroLayoutHeight(false);
LoadImage();
SetForceZeroLayoutHeight(false);
CreateDocument(80, 70);
EXPECT_TRUE(GetDocument().ShouldShrinkToFit());
EXPECT_EQ(15, GetDocument().ImageElement()->OffsetLeft());
EXPECT_EQ(10, GetDocument().ImageElement()->OffsetTop());
......@@ -234,9 +241,8 @@ TEST_F(ImageDocumentTest, DomInteractive) {
TEST_F(ImageDocumentTest, ImageSrcChangedBeforeFinish) {
CreateDocumentWithoutLoadingImage(80, 70);
DocumentParser* parser = StartLoadingImage();
GetDocument().ImageElement()->removeAttribute(html_names::kSrcAttr);
parser->Finish();
blink::test::RunPendingTasks();
}
#if defined(OS_ANDROID)
......@@ -246,9 +252,8 @@ TEST_F(ImageDocumentTest, ImageSrcChangedBeforeFinish) {
#endif
TEST_F(ImageDocumentTest, MAYBE(ImageCenteredAtDeviceScaleFactor)) {
CreateDocumentWithoutLoadingImage(30, 30);
SetWindowToViewportScalingFactor(1.5f);
LoadImage();
CreateDocument(30, 30);
EXPECT_TRUE(GetDocument().ShouldShrinkToFit());
GetDocument().ImageClicked(15, 27);
......
......@@ -1805,6 +1805,7 @@ void DocumentLoader::CreateParserPostCommit() {
? kAllowDeferredParsing
: kAllowAsynchronousParsing;
if (IsJavaScriptURLOrXSLTCommit() ||
commit_reason_ == CommitReason::kForcedSync ||
!Document::ThreadedParsingEnabledForTesting()) {
parsing_policy = kForceSynchronousParsing;
}
......
......@@ -1200,7 +1200,13 @@ void FrameLoader::CommitDocumentLoader(
if (commit_reason != CommitReason::kInitialization) {
// Note: this must be called after DispatchDidCommitLoad() for
// metrics to be correctly sent to the browser process.
document_loader_->GetUseCounterHelper().DidCommitLoad(frame_);
// kForcedSync is used in two broad cases: internal UI (inspector, web page
// popups, and validation message overlays) and svg images. Don't track
// page visits for the internal UI cases.
bool is_internal_ui = commit_reason == CommitReason::kForcedSync &&
document_loader_->MimeType() != "image/svg+xml";
if (!is_internal_ui)
document_loader_->GetUseCounterHelper().DidCommitLoad(frame_);
}
if (document_loader_->LoadType() == WebFrameLoadType::kBackForward) {
if (Page* page = frame_->GetPage())
......
......@@ -90,6 +90,8 @@ enum class CommitReason {
kJavascriptUrl,
// Committing a replacement document from XSLT.
kXSLT,
// Used to populate certain internal-implementation frames (e.g., overlays).
kForcedSync,
// All other navigations.
kRegular
};
......
......@@ -160,6 +160,12 @@ TEST_F(WindowPerformanceTest, NavigateAway) {
// document.
TEST(PerformanceLifetimeTest, SurviveContextSwitch) {
auto page_holder = std::make_unique<DummyPageHolder>(IntSize(800, 600));
// Emulate a new window inheriting the origin for its initial empty document
// from its opener. This is necessary to ensure window reuse below, as that
// only happens when origins match.
KURL url("https://example.com");
page_holder->GetDocument().GetSecurityContext().SetSecurityOriginForTesting(
SecurityOrigin::Create(KURL(url)));
WindowPerformance* perf =
DOMWindowPerformance::performance(*page_holder->GetFrame().DomWindow());
......@@ -175,18 +181,16 @@ TEST(PerformanceLifetimeTest, SurviveContextSwitch) {
EXPECT_NE(0U, navigation_start);
// Simulate changing the document while keeping the window.
page_holder->GetDocument().Shutdown();
page_holder->GetFrame().DomWindow()->InstallNewDocument(
DocumentInit::Create()
.WithDocumentLoader(document_loader)
.WithTypeFrom("text/html"));
std::unique_ptr<WebNavigationParams> params =
WebNavigationParams::CreateWithHTMLBuffer(SharedBuffer::Create(), url);
page_holder->GetFrame().Loader().CommitNavigation(std::move(params), nullptr);
EXPECT_EQ(perf, DOMWindowPerformance::performance(
*page_holder->GetFrame().DomWindow()));
EXPECT_EQ(timing, perf->timing());
EXPECT_EQ(&page_holder->GetFrame(), perf->GetFrame());
EXPECT_EQ(&page_holder->GetFrame(), timing->GetFrame());
EXPECT_EQ(navigation_start, timing->navigationStart());
EXPECT_LE(navigation_start, timing->navigationStart());
}
// Make sure the output entries with the same timestamps follow the insertion
......
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