Skip to content

Commit

Permalink
Avoid dangling pointers in pdf_view_web_plugin_unittest.cc
Browse files Browse the repository at this point in the history
-- Re-work member destruction order so that unowned references
   are released before the owned ones.

-- Null out pointer that will go stale in OnMessage() processing.

Bug: 1401495
Change-Id: I231a607b3d784b1116f7da6784de13e8d9bdafb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4206714
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099521}
  • Loading branch information
tsepez authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent 01f4465 commit d5c895c
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions pdf/pdf_view_web_plugin_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,6 @@ class PdfViewWebPluginWithoutInitializeTest : public testing::Test {
// Allows derived classes to customize `client_ptr_` within `SetUpPlugin()`.
virtual void SetUpClient() {}

void TearDown() override { plugin_.reset(); }

void ExpectUpdateTextInputState(
blink::WebTextInputType expected_text_input_type) {
EXPECT_CALL(*client_ptr_, UpdateTextInputState)
Expand All @@ -409,11 +407,19 @@ class PdfViewWebPluginWithoutInitializeTest : public testing::Test {
});
}

void OnMessageWithEngineUpdate(const base::Value::Dict& message) {
// New engine will be created making this unowned reference stale.
engine_ptr_ = nullptr;
plugin_->OnMessage(message);
}

NiceMock<FakePdfService> pdf_service_;
mojo::AssociatedReceiver<pdf::mojom::PdfService> pdf_receiver_{&pdf_service_};

raw_ptr<FakePdfViewWebPluginClient> client_ptr_;
// Must outlive raw_ptrs below.
std::unique_ptr<PdfViewWebPlugin, PluginDeleter> plugin_;

raw_ptr<FakePdfViewWebPluginClient> client_ptr_;
raw_ptr<TestPDFiumEngine> engine_ptr_;
raw_ptr<MockPdfAccessibilityDataHandler> accessibility_data_handler_ptr_;
};
Expand Down Expand Up @@ -2208,7 +2214,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest, HandleResetPrintPreviewModeMessage) {
return engine;
});

plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/0/0/print.pdf",
"grayscale": false,
Expand All @@ -2228,7 +2234,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest,

// The UI ID of 1 in the URL is arbitrary.
// The page index value of -1, AKA `kCompletePDFIndex`, is required for PDFs.
plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/1/-1/print.pdf",
"grayscale": false,
Expand All @@ -2253,7 +2259,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest,
return engine;
});

plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/0/0/print.pdf",
"grayscale": true,
Expand All @@ -2262,7 +2268,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest,
}

TEST_F(PdfViewWebPluginPrintPreviewTest, DocumentLoadComplete) {
plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/0/0/print.pdf",
"grayscale": false,
Expand Down Expand Up @@ -2294,7 +2300,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest,
DocumentLoadProgressResetByResetPrintPreviewModeMessage) {
plugin_->DocumentLoadProgress(2, 100);

plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/123/0/print.pdf",
"grayscale": false,
Expand All @@ -2310,7 +2316,7 @@ TEST_F(PdfViewWebPluginPrintPreviewTest,

TEST_F(PdfViewWebPluginPrintPreviewTest,
DocumentLoadProgressNotResetByLoadPreviewPageMessage) {
plugin_->OnMessage(ParseMessage(R"({
OnMessageWithEngineUpdate(ParseMessage(R"({
"type": "resetPrintPreviewMode",
"url": "chrome-untrusted://print/123/0/print.pdf",
"grayscale": false,
Expand Down

0 comments on commit d5c895c

Please sign in to comment.