Skip to content

Commit 436b4f4

Browse files
kmoon-workChromium LUCI CQ
kmoon-work
authored and
Chromium LUCI CQ
committed
[unseasoned-pdf] Move DidOpen() to PdfViewPluginBase
Consolidates DidOpen() implementations in PdfViewPluginBase. The (unseasoned) PdfViewWebPlugin implementation diverges unnecessarily from the (Peppery) OutOfProcessInstance implementation. Also restricts PdfViewPluginBase::document_load_state_ visibility, since this field only needs to be accessible for testing now. Bug: 1099022 Change-Id: Id5be4726e2ff208454f85db9d0c2afb3deb8fb26 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3200180 Auto-Submit: K. Moon <kmoon@chromium.org> Commit-Queue: Hui Yingst <nigi@chromium.org> Reviewed-by: Hui Yingst <nigi@chromium.org> Cr-Commit-Position: refs/heads/main@{#927709}
1 parent 13e2334 commit 436b4f4

File tree

7 files changed

+33
-51
lines changed

7 files changed

+33
-51
lines changed

‎pdf/out_of_process_instance.cc‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -680,18 +680,6 @@ void OutOfProcessInstance::StopFind() {
680680
PdfViewPluginBase::StopFind();
681681
}
682682

683-
void OutOfProcessInstance::DidOpen(std::unique_ptr<UrlLoader> loader,
684-
int32_t result) {
685-
if (result == PP_OK) {
686-
if (!engine()->HandleDocumentLoad(std::move(loader), GetURL())) {
687-
set_document_load_state(DocumentLoadState::kLoading);
688-
DocumentLoadFailed();
689-
}
690-
} else if (result != PP_ERROR_ABORTED) { // Can happen in tests.
691-
DocumentLoadFailed();
692-
}
693-
}
694-
695683
void OutOfProcessInstance::SendMessage(base::Value message) {
696684
PostMessage(VarFromValue(message));
697685
}

‎pdf/out_of_process_instance.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ class OutOfProcessInstance : public PdfViewPluginBase,
119119
// PdfViewPluginBase:
120120
base::WeakPtr<PdfViewPluginBase> GetWeakPtr() override;
121121
std::unique_ptr<UrlLoader> CreateUrlLoaderInternal() override;
122-
void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result) override;
123122
void SendMessage(base::Value message) override;
124123
void SaveAs() override;
125124
void InitImageData(const gfx::Size& size) override;

‎pdf/pdf_view_plugin_base.cc‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,9 +1617,21 @@ void PdfViewPluginBase::HistogramCustomCounts(const char* name,
16171617
base::UmaHistogramCustomCounts(name, sample, min, max, bucket_count);
16181618
}
16191619

1620+
void PdfViewPluginBase::DidOpen(std::unique_ptr<UrlLoader> loader,
1621+
int32_t result) {
1622+
if (result == kSuccess) {
1623+
if (!engine()->HandleDocumentLoad(std::move(loader), GetURL())) {
1624+
document_load_state_ = DocumentLoadState::kLoading;
1625+
DocumentLoadFailed();
1626+
}
1627+
} else if (result != kErrorAborted) {
1628+
DocumentLoadFailed();
1629+
}
1630+
}
1631+
16201632
void PdfViewPluginBase::DidOpenPreview(std::unique_ptr<UrlLoader> loader,
16211633
int32_t result) {
1622-
DCHECK_EQ(result, Result::kSuccess);
1634+
DCHECK_EQ(result, kSuccess);
16231635
preview_client_ = std::make_unique<PreviewModeClient>(this);
16241636
preview_engine_ = std::make_unique<PDFiumEngine>(
16251637
preview_client_.get(), PDFiumFormFiller::ScriptOption::kNoJavaScript);
@@ -1666,7 +1678,7 @@ void PdfViewPluginBase::ProcessPreviewPageInfo(const std::string& url,
16661678

16671679
void PdfViewPluginBase::LoadAvailablePreviewPage() {
16681680
if (preview_pages_info_.empty() ||
1669-
document_load_state() != DocumentLoadState::kComplete ||
1681+
document_load_state_ != DocumentLoadState::kComplete ||
16701682
preview_document_load_state_ == DocumentLoadState::kLoading) {
16711683
return;
16721684
}

‎pdf/pdf_view_plugin_base.h‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ class PdfViewPluginBase : public PDFEngine::Client,
156156
return notified_browser_about_unsupported_feature_;
157157
}
158158

159+
DocumentLoadState document_load_state_for_testing() const {
160+
return document_load_state_;
161+
}
162+
159163
protected:
160164
struct BackgroundPart {
161165
gfx::Rect location;
@@ -193,9 +197,6 @@ class PdfViewPluginBase : public PDFEngine::Client,
193197
// frame's origin.
194198
virtual std::unique_ptr<UrlLoader> CreateUrlLoaderInternal() = 0;
195199

196-
// Handles `LoadUrl()` result.
197-
virtual void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result) = 0;
198-
199200
bool HandleInputEvent(const blink::WebInputEvent& event);
200201

201202
// Handles `postMessage()` calls from the embedder.
@@ -389,11 +390,6 @@ class PdfViewPluginBase : public PDFEngine::Client,
389390

390391
bool stop_scrolling() const { return stop_scrolling_; }
391392

392-
DocumentLoadState document_load_state() const { return document_load_state_; }
393-
void set_document_load_state(DocumentLoadState state) {
394-
document_load_state_ = state;
395-
}
396-
397393
AccessibilityState accessibility_state() const {
398394
return accessibility_state_;
399395
}
@@ -489,6 +485,9 @@ class PdfViewPluginBase : public PDFEngine::Client,
489485
int32_t max,
490486
uint32_t bucket_count);
491487

488+
// Handles `LoadUrl()` result.
489+
void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result);
490+
492491
// Handles `LoadUrl()` result for print preview.
493492
void DidOpenPreview(std::unique_ptr<UrlLoader> loader, int32_t result);
494493

‎pdf/pdf_view_plugin_base_unittest.cc‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,12 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
135135
public:
136136
// Public for testing.
137137
using PdfViewPluginBase::accessibility_state;
138-
using PdfViewPluginBase::document_load_state;
139138
using PdfViewPluginBase::edit_mode;
140139
using PdfViewPluginBase::engine;
141140
using PdfViewPluginBase::full_frame;
142141
using PdfViewPluginBase::HandleMessage;
143142
using PdfViewPluginBase::InitializeEngine;
144143
using PdfViewPluginBase::LoadUrl;
145-
using PdfViewPluginBase::set_document_load_state;
146144
using PdfViewPluginBase::set_full_frame;
147145

148146
MOCK_METHOD(bool, Confirm, (const std::string&), (override));
@@ -186,8 +184,6 @@ class FakePdfViewPluginBase : public PdfViewPluginBase {
186184
(),
187185
(override));
188186

189-
MOCK_METHOD(void, DidOpen, (std::unique_ptr<UrlLoader>, int32_t), (override));
190-
191187
void SendMessage(base::Value message) override {
192188
sent_messages_.push_back(std::move(message));
193189
}
@@ -516,7 +512,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
516512

517513
ASSERT_FALSE(fake_plugin_.IsPrintPreview());
518514
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
519-
fake_plugin_.document_load_state());
515+
fake_plugin_.document_load_state_for_testing());
520516

521517
// Change the accessibility state to pending so that accessibility can be
522518
// loaded later.
@@ -534,7 +530,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
534530

535531
fake_plugin_.DocumentLoadComplete();
536532
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kComplete,
537-
fake_plugin_.document_load_state());
533+
fake_plugin_.document_load_state_for_testing());
538534
EXPECT_EQ(PdfViewPluginBase::AccessibilityState::kLoaded,
539535
fake_plugin_.accessibility_state());
540536

@@ -559,7 +555,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
559555

560556
ASSERT_FALSE(fake_plugin_.IsPrintPreview());
561557
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
562-
fake_plugin_.document_load_state());
558+
fake_plugin_.document_load_state_for_testing());
563559
ASSERT_EQ(PdfViewPluginBase::AccessibilityState::kOff,
564560
fake_plugin_.accessibility_state());
565561

@@ -574,7 +570,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
574570

575571
fake_plugin_.DocumentLoadComplete();
576572
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kComplete,
577-
fake_plugin_.document_load_state());
573+
fake_plugin_.document_load_state_for_testing());
578574
EXPECT_EQ(PdfViewPluginBase::AccessibilityState::kOff,
579575
fake_plugin_.accessibility_state());
580576

@@ -597,7 +593,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
597593

598594
ASSERT_FALSE(fake_plugin_.IsPrintPreview());
599595
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
600-
fake_plugin_.document_load_state());
596+
fake_plugin_.document_load_state_for_testing());
601597

602598
EXPECT_CALL(fake_plugin_, UserMetricsRecordAction("PDF.LoadSuccess"));
603599
EXPECT_CALL(fake_plugin_, SetFormFieldInFocus(false));
@@ -608,7 +604,7 @@ TEST_F(PdfViewPluginBaseWithDocInfoTest,
608604

609605
fake_plugin_.DocumentLoadComplete();
610606
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kComplete,
611-
fake_plugin_.document_load_state());
607+
fake_plugin_.document_load_state_for_testing());
612608

613609
// Check all the sent messages.
614610
ASSERT_EQ(4u, fake_plugin_.sent_messages().size());
@@ -627,13 +623,13 @@ TEST_F(PdfViewPluginBaseWithoutDocInfoTest, DocumentLoadCompletePostMessages) {
627623

628624
ASSERT_FALSE(fake_plugin_.IsPrintPreview());
629625
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
630-
fake_plugin_.document_load_state());
626+
fake_plugin_.document_load_state_for_testing());
631627
EXPECT_CALL(fake_plugin_, UserMetricsRecordAction("PDF.LoadSuccess"));
632628
EXPECT_CALL(fake_plugin_, SetFormFieldInFocus(false));
633629

634630
fake_plugin_.DocumentLoadComplete();
635631
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kComplete,
636-
fake_plugin_.document_load_state());
632+
fake_plugin_.document_load_state_for_testing());
637633

638634
// Check the sent messages when the document doesn't have any metadata,
639635
// attachments or bookmarks.
@@ -651,15 +647,15 @@ TEST_F(PdfViewPluginBaseTest, DocumentLoadFailedWithNotifiedRenderFrame) {
651647
fake_plugin_.CreateUrlLoader();
652648

653649
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
654-
fake_plugin_.document_load_state());
650+
fake_plugin_.document_load_state_for_testing());
655651
EXPECT_TRUE(fake_plugin_.GetDidCallStartLoadingForTesting());
656652

657653
EXPECT_CALL(fake_plugin_, UserMetricsRecordAction("PDF.LoadFailure"));
658654
EXPECT_CALL(fake_plugin_, PluginDidStopLoading());
659655

660656
fake_plugin_.DocumentLoadFailed();
661657
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kFailed,
662-
fake_plugin_.document_load_state());
658+
fake_plugin_.document_load_state_for_testing());
663659
EXPECT_FALSE(fake_plugin_.GetDidCallStartLoadingForTesting());
664660
}
665661

@@ -669,13 +665,13 @@ TEST_F(PdfViewPluginBaseTest, DocumentLoadFailedWithoutNotifiedRenderFrame) {
669665
EXPECT_FALSE(fake_plugin_.GetDidCallStartLoadingForTesting());
670666

671667
ASSERT_EQ(PdfViewPluginBase::DocumentLoadState::kLoading,
672-
fake_plugin_.document_load_state());
668+
fake_plugin_.document_load_state_for_testing());
673669
EXPECT_CALL(fake_plugin_, UserMetricsRecordAction("PDF.LoadFailure"));
674670
EXPECT_CALL(fake_plugin_, PluginDidStopLoading()).Times(0);
675671

676672
fake_plugin_.DocumentLoadFailed();
677673
EXPECT_EQ(PdfViewPluginBase::DocumentLoadState::kFailed,
678-
fake_plugin_.document_load_state());
674+
fake_plugin_.document_load_state_for_testing());
679675
EXPECT_FALSE(fake_plugin_.GetDidCallStartLoadingForTesting());
680676
}
681677

‎pdf/pdf_view_web_plugin.cc‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -776,17 +776,6 @@ std::unique_ptr<UrlLoader> PdfViewWebPlugin::CreateUrlLoaderInternal() {
776776
return loader;
777777
}
778778

779-
// Modeled on `OutOfProcessInstance::DidOpen()`.
780-
void PdfViewWebPlugin::DidOpen(std::unique_ptr<UrlLoader> loader,
781-
int32_t result) {
782-
if (result == Result::kSuccess) {
783-
if (!engine()->HandleDocumentLoad(std::move(loader), GetURL()))
784-
DocumentLoadFailed();
785-
} else {
786-
NOTIMPLEMENTED();
787-
}
788-
}
789-
790779
void PdfViewWebPlugin::SendMessage(base::Value message) {
791780
post_message_sender_.Post(std::move(message));
792781
}

‎pdf/pdf_view_web_plugin.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,6 @@ class PdfViewWebPlugin final : public PdfViewPluginBase,
271271
// PdfViewPluginBase:
272272
base::WeakPtr<PdfViewPluginBase> GetWeakPtr() override;
273273
std::unique_ptr<UrlLoader> CreateUrlLoaderInternal() override;
274-
void DidOpen(std::unique_ptr<UrlLoader> loader, int32_t result) override;
275274
void SendMessage(base::Value message) override;
276275
void SaveAs() override;
277276
void InitImageData(const gfx::Size& size) override;

0 commit comments

Comments
 (0)