[unseasoned-pdf] Block 100% loading on PDF layout
Blocks 100% loading progress until PDF page layout has been negotiated between the internal plugin and the UI extension. This fixes ExtensionApiTest.TemporaryAddressSpoof by avoiding a race between mouse events and layout negotiation. Bug: 1237952 Change-Id: Ia8466e6ec4b6aeb66034a273b48afa41e4754f9a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3119844 Reviewed-by: Hui Yingst <nigi@chromium.org> Commit-Queue: K. Moon <kmoon@chromium.org> Cr-Commit-Position: refs/heads/main@{#916154}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
92223e58dc
commit
76d6ade99f
@ -412,6 +412,7 @@ if (enable_pdf) {
|
||||
":ppapi_migration",
|
||||
"//base",
|
||||
"//ppapi/cpp:objects",
|
||||
"//testing/gmock",
|
||||
"//testing/gtest",
|
||||
"//third_party/blink/public/common:headers",
|
||||
"//ui/gfx:geometry_skia",
|
||||
|
@ -327,7 +327,6 @@ void PdfViewPluginBase::DocumentLoadComplete() {
|
||||
SendAttachments();
|
||||
SendBookmarks();
|
||||
SendMetadata();
|
||||
SendLoadingProgress(/*percentage=*/100);
|
||||
|
||||
if (accessibility_state_ == AccessibilityState::kPending)
|
||||
LoadAccessibility();
|
||||
@ -390,7 +389,6 @@ void PdfViewPluginBase::DocumentLoadProgress(uint32_t available,
|
||||
if (progress <= last_progress_sent_ + 1)
|
||||
return;
|
||||
|
||||
last_progress_sent_ = progress;
|
||||
SendLoadingProgress(progress);
|
||||
}
|
||||
|
||||
@ -568,6 +566,7 @@ void PdfViewPluginBase::ConsumeSaveToken(const std::string& token) {
|
||||
|
||||
void PdfViewPluginBase::SendLoadingProgress(double percentage) {
|
||||
DCHECK(percentage == -1 || (percentage >= 0 && percentage <= 100));
|
||||
last_progress_sent_ = percentage;
|
||||
|
||||
base::Value message(base::Value::Type::DICTIONARY);
|
||||
message.SetStringKey("type", "loadProgress");
|
||||
@ -1149,6 +1148,10 @@ void PdfViewPluginBase::HandleViewportMessage(const base::Value& message) {
|
||||
// TODO(crbug.com/1013800): Eliminate need to get document size from here.
|
||||
document_size_ = engine()->ApplyDocumentLayout(layout_options);
|
||||
OnGeometryChanged(zoom_, device_scale_);
|
||||
|
||||
// Send 100% loading progress only after initial layout negotiated.
|
||||
if (last_progress_sent_ < 100)
|
||||
SendLoadingProgress(/*percentage=*/100);
|
||||
}
|
||||
|
||||
gfx::PointF scroll_position(message.FindDoubleKey("xOffset").value(),
|
||||
|
@ -12,12 +12,14 @@
|
||||
#include "base/containers/contains.h"
|
||||
#include "base/memory/weak_ptr.h"
|
||||
#include "base/test/icu_test_util.h"
|
||||
#include "base/test/values_test_util.h"
|
||||
#include "base/time/time.h"
|
||||
#include "base/values.h"
|
||||
#include "pdf/accessibility_structs.h"
|
||||
#include "pdf/buildflags.h"
|
||||
#include "pdf/content_restriction.h"
|
||||
#include "pdf/document_attachment_info.h"
|
||||
#include "pdf/document_layout.h"
|
||||
#include "pdf/document_metadata.h"
|
||||
#include "pdf/pdf_engine.h"
|
||||
#include "pdf/ppapi_migration/callback.h"
|
||||
@ -34,6 +36,8 @@ namespace chrome_pdf {
|
||||
|
||||
namespace {
|
||||
|
||||
using ::testing::ElementsAre;
|
||||
using ::testing::IsEmpty;
|
||||
using ::testing::SaveArg;
|
||||
|
||||
// Keep it in-sync with the `kFinalFallbackName` returned by
|
||||
@ -231,6 +235,8 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
|
||||
|
||||
MOCK_METHOD(void, UserMetricsRecordAction, (const std::string&), (override));
|
||||
|
||||
void clear_sent_messages() { sent_messages_.clear(); }
|
||||
|
||||
const std::vector<base::Value>& sent_messages() const {
|
||||
return sent_messages_;
|
||||
}
|
||||
@ -332,13 +338,6 @@ base::Value CreateExpectedNoMetadataResponse() {
|
||||
return message;
|
||||
}
|
||||
|
||||
base::Value CreateExpectedLoadingProgressResponse() {
|
||||
base::Value message(base::Value::Type::DICTIONARY);
|
||||
message.SetStringKey("type", "loadProgress");
|
||||
message.SetDoubleKey("progress", 100);
|
||||
return message;
|
||||
}
|
||||
|
||||
base::Value CreateSaveRequestMessage(PdfViewPluginBase::SaveRequestType type,
|
||||
const std::string& token) {
|
||||
base::Value message(base::Value::Type::DICTIONARY);
|
||||
@ -375,8 +374,8 @@ class PdfViewPluginBaseTest : public testing::Test {
|
||||
class PdfViewPluginBaseWithEngineTest : public PdfViewPluginBaseTest {
|
||||
public:
|
||||
void SetUp() override {
|
||||
std::unique_ptr<TestPDFiumEngine> engine =
|
||||
std::make_unique<TestPDFiumEngine>(&fake_plugin_);
|
||||
auto engine =
|
||||
std::make_unique<testing::NiceMock<TestPDFiumEngine>>(&fake_plugin_);
|
||||
fake_plugin_.InitializeEngine(std::move(engine));
|
||||
}
|
||||
};
|
||||
@ -486,7 +485,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
fake_plugin_.accessibility_state());
|
||||
|
||||
// Check all the sent messages.
|
||||
ASSERT_EQ(5u, fake_plugin_.sent_messages().size());
|
||||
ASSERT_EQ(4u, fake_plugin_.sent_messages().size());
|
||||
EXPECT_EQ(CreateExpectedFormTextFieldFocusChangeResponse(),
|
||||
fake_plugin_.sent_messages()[0]);
|
||||
EXPECT_EQ(CreateExpectedAttachmentsResponse(),
|
||||
@ -495,8 +494,6 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
CreateExpectedBookmarksResponse(fake_plugin_.engine()->GetBookmarks()),
|
||||
fake_plugin_.sent_messages()[2]);
|
||||
EXPECT_EQ(CreateExpectedMetadataResponse(), fake_plugin_.sent_messages()[3]);
|
||||
EXPECT_EQ(CreateExpectedLoadingProgressResponse(),
|
||||
fake_plugin_.sent_messages()[4]);
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
@ -528,7 +525,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
fake_plugin_.accessibility_state());
|
||||
|
||||
// Check all the sent messages.
|
||||
ASSERT_EQ(5u, fake_plugin_.sent_messages().size());
|
||||
ASSERT_EQ(4u, fake_plugin_.sent_messages().size());
|
||||
EXPECT_EQ(CreateExpectedFormTextFieldFocusChangeResponse(),
|
||||
fake_plugin_.sent_messages()[0]);
|
||||
EXPECT_EQ(CreateExpectedAttachmentsResponse(),
|
||||
@ -537,8 +534,6 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
CreateExpectedBookmarksResponse(fake_plugin_.engine()->GetBookmarks()),
|
||||
fake_plugin_.sent_messages()[2]);
|
||||
EXPECT_EQ(CreateExpectedMetadataResponse(), fake_plugin_.sent_messages()[3]);
|
||||
EXPECT_EQ(CreateExpectedLoadingProgressResponse(),
|
||||
fake_plugin_.sent_messages()[4]);
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
@ -562,7 +557,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
fake_plugin_.document_load_state());
|
||||
|
||||
// Check all the sent messages.
|
||||
ASSERT_EQ(5u, fake_plugin_.sent_messages().size());
|
||||
ASSERT_EQ(4u, fake_plugin_.sent_messages().size());
|
||||
EXPECT_EQ(CreateExpectedFormTextFieldFocusChangeResponse(),
|
||||
fake_plugin_.sent_messages()[0]);
|
||||
EXPECT_EQ(CreateExpectedAttachmentsResponse(),
|
||||
@ -571,8 +566,6 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
|
||||
CreateExpectedBookmarksResponse(fake_plugin_.engine()->GetBookmarks()),
|
||||
fake_plugin_.sent_messages()[2]);
|
||||
EXPECT_EQ(CreateExpectedMetadataResponse(), fake_plugin_.sent_messages()[3]);
|
||||
EXPECT_EQ(CreateExpectedLoadingProgressResponse(),
|
||||
fake_plugin_.sent_messages()[4]);
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithoutDocInfoTest, DocumentLoadCompletePostMessages) {
|
||||
@ -590,13 +583,11 @@ TEST_F(PdfViewPluginBaseWithoutDocInfoTest, DocumentLoadCompletePostMessages) {
|
||||
|
||||
// Check the sent messages when the document doesn't have any metadata,
|
||||
// attachments or bookmarks.
|
||||
ASSERT_EQ(3u, fake_plugin_.sent_messages().size());
|
||||
ASSERT_EQ(2u, fake_plugin_.sent_messages().size());
|
||||
EXPECT_EQ(CreateExpectedFormTextFieldFocusChangeResponse(),
|
||||
fake_plugin_.sent_messages()[0]);
|
||||
EXPECT_EQ(CreateExpectedNoMetadataResponse(),
|
||||
fake_plugin_.sent_messages()[1]);
|
||||
EXPECT_EQ(CreateExpectedLoadingProgressResponse(),
|
||||
fake_plugin_.sent_messages()[2]);
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseTest, DocumentLoadFailedWithNotifiedRenderFrame) {
|
||||
@ -837,6 +828,66 @@ TEST_F(PdfViewPluginBaseTest, HandleSetBackgroundColorMessage) {
|
||||
EXPECT_EQ(kNewBackgroundColor, fake_plugin_.GetBackgroundColor());
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithEngineTest, HandleViewportMessageInitially) {
|
||||
auto* engine = static_cast<TestPDFiumEngine*>(fake_plugin_.engine());
|
||||
EXPECT_CALL(*engine, ApplyDocumentLayout(DocumentLayout::Options()));
|
||||
|
||||
fake_plugin_.HandleMessage(base::test::ParseJson(R"({
|
||||
"type": "viewport",
|
||||
"userInitiated": false,
|
||||
"zoom": 1,
|
||||
"layoutOptions": {
|
||||
"defaultPageOrientation": 0,
|
||||
"twoUpViewEnabled": false,
|
||||
},
|
||||
"xOffset": 0,
|
||||
"yOffset": 0,
|
||||
"pinchPhase": 0,
|
||||
})"));
|
||||
|
||||
EXPECT_THAT(fake_plugin_.sent_messages(), ElementsAre(base::test::IsJson(R"({
|
||||
"type": "loadProgress",
|
||||
"progress": 100.0,
|
||||
})")));
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithEngineTest, HandleViewportMessageSubsequently) {
|
||||
auto* engine = static_cast<TestPDFiumEngine*>(fake_plugin_.engine());
|
||||
|
||||
fake_plugin_.HandleMessage(base::test::ParseJson(R"({
|
||||
"type": "viewport",
|
||||
"userInitiated": false,
|
||||
"zoom": 1,
|
||||
"layoutOptions": {
|
||||
"defaultPageOrientation": 0,
|
||||
"twoUpViewEnabled": false,
|
||||
},
|
||||
"xOffset": 0,
|
||||
"yOffset": 0,
|
||||
"pinchPhase": 0,
|
||||
})"));
|
||||
fake_plugin_.clear_sent_messages();
|
||||
|
||||
DocumentLayout::Options two_up_options;
|
||||
two_up_options.set_page_spread(DocumentLayout::PageSpread::kTwoUpOdd);
|
||||
EXPECT_CALL(*engine, ApplyDocumentLayout(two_up_options));
|
||||
|
||||
fake_plugin_.HandleMessage(base::test::ParseJson(R"({
|
||||
"type": "viewport",
|
||||
"userInitiated": false,
|
||||
"zoom": 1,
|
||||
"layoutOptions": {
|
||||
"defaultPageOrientation": 0,
|
||||
"twoUpViewEnabled": true,
|
||||
},
|
||||
"xOffset": 0,
|
||||
"yOffset": 0,
|
||||
"pinchPhase": 0,
|
||||
})"));
|
||||
|
||||
EXPECT_THAT(fake_plugin_.sent_messages(), IsEmpty());
|
||||
}
|
||||
|
||||
TEST_F(PdfViewPluginBaseWithEngineTest, GetContentRestrictions) {
|
||||
auto* engine = static_cast<TestPDFiumEngine*>(fake_plugin_.engine());
|
||||
static constexpr int kContentRestrictionCutPaste =
|
||||
|
@ -16,6 +16,7 @@
|
||||
#include "pdf/document_metadata.h"
|
||||
#include "pdf/pdf_engine.h"
|
||||
#include "pdf/pdfium/pdfium_engine.h"
|
||||
#include "testing/gmock/include/gmock/gmock.h"
|
||||
#include "third_party/blink/public/common/input/web_mouse_event.h"
|
||||
|
||||
namespace blink {
|
||||
@ -40,6 +41,11 @@ class TestPDFiumEngine : public PDFiumEngine {
|
||||
|
||||
~TestPDFiumEngine() override;
|
||||
|
||||
MOCK_METHOD(gfx::Size,
|
||||
ApplyDocumentLayout,
|
||||
(const DocumentLayout::Options&),
|
||||
(override));
|
||||
|
||||
// Sets a scaled mouse event for testing.
|
||||
bool HandleInputEvent(const blink::WebInputEvent& scaled_event) override;
|
||||
|
||||
|
Reference in New Issue
Block a user