/
cherry-pick-1231134.patch
163 lines (141 loc) · 7.55 KB
/
cherry-pick-1231134.patch
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Lei Zhang <thestig@chromium.org>
Date: Tue, 10 Aug 2021 21:38:36 +0000
Subject: Do more class validity checks in PrintViewManagerBase.
PrintViewManagerBase runs a nested loop. In some situations,
PrintViewManagerBase and related classes like PrintViewManager and
PrintPreviewHandler can get deleted while the nested loop is running.
When this happens, the nested loop exists to a PrintViewManagerBase
that is no longer valid.
Use base::WeakPtrs liberally to check for this condition and exit
safely.
(cherry picked from commit a2cb1fb333d2faacb2fe1380f8d2621b5ee6af7e)
Bug: 1231134
Change-Id: I21ec131574331ce973d22594c11e70088147e149
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3057880
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#906269}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3086110
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4515@{#2024}
Cr-Branched-From: 488fc70865ddaa05324ac00a54a6eb783b4bc41c-refs/heads/master@{#885287}
diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc
index b7a2fa5a10b2461e83d78fca63e3165d5eb1350f..9f11d5053ff19e4ccfdbe54e7f59e311da3c6e0a 100644
--- a/chrome/browser/printing/print_view_manager.cc
+++ b/chrome/browser/printing/print_view_manager.cc
@@ -91,7 +91,11 @@ bool PrintViewManager::PrintForSystemDialogNow(
DCHECK(!on_print_dialog_shown_callback_);
on_print_dialog_shown_callback_ = std::move(dialog_shown_callback);
is_switching_to_system_dialog_ = true;
+
+ auto weak_this = weak_factory_.GetWeakPtr();
DisconnectFromCurrentPrintJob();
+ if (!weak_this)
+ return false;
// Don't print / print preview crashed tabs.
if (IsCrashed())
diff --git a/chrome/browser/printing/print_view_manager.h b/chrome/browser/printing/print_view_manager.h
index 8770b7bc60cb28d70cde3af8303939bf7ac27892..16c13724c397fade0eb64f9b5ec26c7bf51570ac 100644
--- a/chrome/browser/printing/print_view_manager.h
+++ b/chrome/browser/printing/print_view_manager.h
@@ -130,6 +130,11 @@ class PrintViewManager : public PrintViewManagerBase,
WEB_CONTENTS_USER_DATA_KEY_DECL();
+ // Keep this last so that all weak pointers will be invalidated at the
+ // beginning of destruction. Note that PrintViewManagerBase has its own
+ // base::WeakPtrFactory as well, but PrintViewManager should use this one.
+ base::WeakPtrFactory<PrintViewManager> weak_factory_{this};
+
DISALLOW_COPY_AND_ASSIGN(PrintViewManager);
};
diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc
index 75908fc9978db020d752e7fc7211d1a075c11ffb..b896f69f3ec272001c5c6c0071bc23a3c9134d70 100644
--- a/chrome/browser/printing/print_view_manager_base.cc
+++ b/chrome/browser/printing/print_view_manager_base.cc
@@ -192,7 +192,10 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh,
bool silent,
base::Value settings,
CompletionCallback callback) {
+ auto weak_this = weak_ptr_factory_.GetWeakPtr();
DisconnectFromCurrentPrintJob();
+ if (!weak_this)
+ return false;
// Don't print / print preview crashed tabs.
if (IsCrashed())
@@ -625,6 +628,8 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {
// or in DidPrintDocument(). The check is done in
// ShouldQuitFromInnerMessageLoop().
// BLOCKS until all the pages are received. (Need to enable recursive task)
+ // WARNING: Do not do any work after RunInnerMessageLoop() returns, as `this`
+ // may have gone away.
if (!RunInnerMessageLoop()) {
// This function is always called from DisconnectFromCurrentPrintJob() so we
// know that the job will be stopped/canceled in any case.
@@ -651,8 +656,11 @@ bool PrintViewManagerBase::CreateNewPrintJob(
DCHECK(query);
if (callback_.is_null()) {
+ auto weak_this = weak_ptr_factory_.GetWeakPtr();
// Disconnect the current |print_job_| only when calling window.print()
DisconnectFromCurrentPrintJob();
+ if (!weak_this)
+ return false;
}
// We can't print if there is no renderer.
@@ -681,7 +689,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
void PrintViewManagerBase::DisconnectFromCurrentPrintJob() {
// Make sure all the necessary rendered page are done. Don't bother with the
// return value.
+ auto weak_this = weak_ptr_factory_.GetWeakPtr();
bool result = RenderAllMissingPagesNow();
+ if (!weak_this)
+ return;
// Verify that assertion.
if (print_job_ && print_job_->document() &&
@@ -763,7 +774,10 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
quit_inner_loop_ = run_loop.QuitClosure();
+ auto weak_this = weak_ptr_factory_.GetWeakPtr();
run_loop.Run();
+ if (!weak_this)
+ return false;
// If the inner-loop quit closure is still set then we timed out.
bool success = !quit_inner_loop_;
diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h
index 095d9dfcc3ee14b646b63c29e406434c1623704a..ed4c0ec98bb84753828d0649799077edc9796317 100644
--- a/chrome/browser/printing/print_view_manager_base.h
+++ b/chrome/browser/printing/print_view_manager_base.h
@@ -115,6 +115,8 @@ class PrintViewManagerBase : public content::NotificationObserver,
// Makes sure the current print_job_ has all its data before continuing, and
// disconnect from it.
+ // WARNING: `this` may not be alive after DisconnectFromCurrentPrintJob()
+ // returns.
void DisconnectFromCurrentPrintJob();
// Manages the low-level talk to the printer.
@@ -168,6 +170,7 @@ class PrintViewManagerBase : public content::NotificationObserver,
// Requests the RenderView to render all the missing pages for the print job.
// No-op if no print job is pending. Returns true if at least one page has
// been requested to the renderer.
+ // WARNING: `this` may not be alive after RenderAllMissingPagesNow() returns.
bool RenderAllMissingPagesNow();
// Checks that synchronization is correct with |print_job_| based on |cookie|.
@@ -201,6 +204,7 @@ class PrintViewManagerBase : public content::NotificationObserver,
// while the blocking inner message loop is running. This is useful in cases
// where the RenderView is about to be destroyed while a printing job isn't
// finished.
+ // WARNING: `this` may not be alive after RunInnerMessageLoop() returns.
bool RunInnerMessageLoop();
// In the case of Scripted Printing, where the renderer is controlling the
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
index 2cea700c82790f696775ece3080662232326aabc..ab89b180e6f9586d6280d884f126fb46b278be04 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
@@ -864,9 +864,12 @@ void PrintPreviewHandler::HandleShowSystemDialog(
if (!initiator)
return;
+ auto weak_this = weak_factory_.GetWeakPtr();
auto* print_view_manager = PrintViewManager::FromWebContents(initiator);
print_view_manager->PrintForSystemDialogNow(base::BindOnce(
&PrintPreviewHandler::ClosePreviewDialog, weak_factory_.GetWeakPtr()));
+ if (!weak_this)
+ return;
// Cancel the pending preview request if exists.
print_preview_ui()->OnCancelPendingPreviewRequest();