Skip to content

Commit fe0287f

Browse files
Tom LukaszewiczChromium LUCI CQ
Tom Lukaszewicz
authored and
Chromium LUCI CQ
committed
[views] Eliminate OnWidgetClosing from NudgeWidgetObserver
Favor OnWidgetDestroying over OnWidgetClosing. OnWidgetClosing is only called where a Widget is closed directly, it fails to catch the case when the system destroys the NativeWidget directly. This can result in UAFs in situations where the OS directly destroys the widget. OnWidgetDestroying will be called every time a widget is destroyed and should be used instead in most cases. Bug: 1240365 Change-Id: I043635fcfcc9a52dd68b21f4f9c37539bc0d7f34 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641295 Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org> Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org> Cr-Commit-Position: refs/heads/main@{#1002735}
1 parent 64c67dd commit fe0287f

File tree

1 file changed

+8
-8
lines changed

1 file changed

+8
-8
lines changed

ash/system/power/adaptive_charging_nudge_controller_unittest.cc

+8-8
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@ class NudgeWidgetObserver : public views::WidgetObserver {
3232
widget_observation_.Observe(widget);
3333
}
3434

35-
void WaitForClose() {
35+
void WaitForDestruction() {
3636
run_loop_ = std::make_unique<base::RunLoop>();
3737
run_loop_->Run();
3838
}
3939

4040
// views::WidgetObserver:
41-
void OnWidgetClosing(views::Widget* widget) override {
41+
void OnWidgetDestroying(views::Widget* widget) override {
4242
if (run_loop_)
4343
run_loop_->Quit();
4444
}
@@ -75,8 +75,8 @@ class AdaptiveChargingNudgeControllerTest : public AshTestBase {
7575

7676
AdaptiveChargingNudgeController* GetController() { return controller_.get(); }
7777

78-
void WaitForWidgetClose(AdaptiveChargingNudgeController* controller,
79-
SystemNudge* nudge) {
78+
void WaitForWidgetDestruction(AdaptiveChargingNudgeController* controller,
79+
SystemNudge* nudge) {
8080
views::Widget* nudge_widget = nudge->widget();
8181
ASSERT_TRUE(nudge_widget);
8282
EXPECT_FALSE(nudge_widget->IsClosed());
@@ -86,12 +86,12 @@ class AdaptiveChargingNudgeControllerTest : public AshTestBase {
8686
ui::ScopedAnimationDurationScaleMode::SLOW_DURATION);
8787

8888
// Pretend the hide nudge timer has elapsed.
89-
NudgeWidgetObserver widget_close_observer(nudge_widget);
89+
NudgeWidgetObserver widget_destruction_observer(nudge_widget);
9090
controller->FireHideNudgeTimerForTesting();
9191

9292
EXPECT_TRUE(nudge_widget->GetLayer()->GetAnimator()->is_animating());
9393

94-
widget_close_observer.WaitForClose();
94+
widget_destruction_observer.WaitForDestruction();
9595
}
9696

9797
private:
@@ -109,7 +109,7 @@ TEST_F(AdaptiveChargingNudgeControllerTest, ShowsAndHidesNudge) {
109109
SystemNudge* nudge = controller->GetSystemNudgeForTesting();
110110
ASSERT_TRUE(nudge);
111111

112-
WaitForWidgetClose(controller, nudge);
112+
WaitForWidgetDestruction(controller, nudge);
113113
}
114114

115115
TEST_F(AdaptiveChargingNudgeControllerTest, NoNudgeShowForDisabledFeature) {
@@ -135,7 +135,7 @@ TEST_F(AdaptiveChargingNudgeControllerTest, NudgeShowExactlyOnce) {
135135
controller->GetNudgeDelayTimerForTesting()->FireNow();
136136
SystemNudge* nudge1 = controller->GetSystemNudgeForTesting();
137137
ASSERT_TRUE(nudge1);
138-
WaitForWidgetClose(controller, nudge1);
138+
WaitForWidgetDestruction(controller, nudge1);
139139

140140
// No nudge for the second time.
141141
controller->ShowNudge();

0 commit comments

Comments
 (0)