已合并
【安全】【平台通用】修改合入 22 笔安全问题修复(chromium_138 26-07-06) #9935
福佑中华-大刘创建于 7月30日
【安全】【平台通用】修改合入 22 笔安全问题修复(chromium_138 26-07-06) #9935
已合并
共 51 个文件变更+1160-98
| @@ -131,8 +131,11 @@ void SimpleDevToolsProtocolClient::DispatchProtocolMessageTask( | |||
| 131 | auto it = pending_response_map_.find(*id); | 131 | auto it = pending_response_map_.find(*id); |
| 132 | if (it == pending_response_map_.cend()) { | 132 | if (it == pending_response_map_.cend()) { |
| 133 | LOG(ERROR) << "Unexpected message id=" << *id; | 133 | LOG(ERROR) << "Unexpected message id=" << *id; |
| 134 | - agent_host_->GetProcessHost()->ShutdownForBadMessage( | 134 | + if (agent_host_) { |
| 135 | - content::RenderProcessHost::CrashReportMode::GENERATE_CRASH_DUMP); | 135 | + agent_host_->GetProcessHost()->ShutdownForBadMessage( |
| 136 | + content::RenderProcessHost::CrashReportMode::GENERATE_CRASH_DUMP); | ||
| 137 | + } | ||
| 138 | + return; | ||
| 136 | } | 139 | } |
| 137 | 140 | ||
| 138 | // Result handler callback may add more callbacks, so make sure we use | 141 | // Result handler callback may add more callbacks, so make sure we use |
Mcomponents/devtools/simple_devtools_protocol_client/simple_devtools_protocol_client_unittest.cc+36-0
| @@ -13,6 +13,7 @@ | |||
| 13 | 13 | ||
| 14 | 14 | ||
| 15 | 15 | ||
| 16 | + | ||
| 16 | 17 | ||
| 17 | 18 | ||
| 18 | 19 | ||
| @@ -374,6 +375,41 @@ TEST(SimpleDevToolsProtocolClientTest, DestoroyClientInFlight) { | |||
| 374 | task_environment.RunUntilIdle(); | 375 | task_environment.RunUntilIdle(); |
| 375 | } | 376 | } |
| 376 | 377 | ||
| 378 | +class UnexpectedMessageIdSimpleDevToolsProtocolClient | ||
| 379 | + : public SimpleDevToolsProtocolClient { | ||
| 380 | + public: | ||
| 381 | + void TryIt() { | ||
| 382 | + std::string json_message = "{\"id\": 42}"; | ||
| 383 | + SimpleDevToolsProtocolClient::DispatchProtocolMessage( | ||
| 384 | + agent_host_.get(), base::as_byte_span(json_message)); | ||
| 385 | + } | ||
| 386 | + | ||
| 387 | + void DispatchProtocolMessageTask(base::DictValue message) override { | ||
| 388 | + SimpleDevToolsProtocolClient::DispatchProtocolMessageTask( | ||
| 389 | + std::move(message)); | ||
| 390 | + std::move(done_callback_).Run(); | ||
| 391 | + } | ||
| 392 | + | ||
| 393 | + void set_done_callback(base::OnceClosure callback) { | ||
| 394 | + done_callback_ = std::move(callback); | ||
| 395 | + } | ||
| 396 | + | ||
| 397 | + private: | ||
| 398 | + base::OnceClosure done_callback_; | ||
| 399 | +}; | ||
| 400 | + | ||
| 401 | +TEST(SimpleDevToolsProtocolClientTest, TryUnexpectedMessageId) { | ||
| 402 | + content::BrowserTaskEnvironment task_environment; | ||
| 403 | + | ||
| 404 | + auto client = | ||
| 405 | + std::make_unique<UnexpectedMessageIdSimpleDevToolsProtocolClient>(); | ||
| 406 | + | ||
| 407 | + base::RunLoop run_loop; | ||
| 408 | + client->set_done_callback(run_loop.QuitClosure()); | ||
| 409 | + client->TryIt(); | ||
| 410 | + run_loop.Run(); | ||
| 411 | +} | ||
| 412 | + | ||
| 377 | } // namespace | 413 | } // namespace |
| 378 | 414 | ||
| 379 | } // namespace simple_devtools_protocol_client | 415 | } // namespace simple_devtools_protocol_client |
| @@ -6068,10 +6068,16 @@ void RenderFrameHostImpl::DidCommitPageActivation( | |||
| 6068 | }); | 6068 | }); |
| 6069 | 6069 | ||
| 6070 | 6070 | ||
| 6071 | + base::WeakPtr<RenderFrameHostImpl> weak_ptr = GetWeakPtr(); | ||
| 6071 | DidCommitNavigationInternal( | 6072 | DidCommitNavigationInternal( |
| 6072 | std::move(owned_request), std::move(params), | 6073 | std::move(owned_request), std::move(params), |
| 6073 | /*same_document_params=*/nullptr, | 6074 | /*same_document_params=*/nullptr, |
| 6074 | /*did_commit_ipc_received_time=*/base::TimeTicks()); | 6075 | /*did_commit_ipc_received_time=*/base::TimeTicks()); |
| 6076 | + if (!weak_ptr) { | ||
| 6077 | + // This RFH may be deleted after DidCommitNavigationInternal due to a nested | ||
| 6078 | + // message loop. All callers should handle this. | ||
| 6079 | + return; | ||
| 6080 | + } | ||
| 6075 | 6081 | ||
| 6076 | // NOTE: Navigation metrics assume that not much work is done between | 6082 | // NOTE: Navigation metrics assume that not much work is done between |
| 6077 | // DidCommitNavigationInternal() and the end of this function. Avoid adding | 6083 | // DidCommitNavigationInternal() and the end of this function. Avoid adding |
| @@ -15570,10 +15576,17 @@ bool RenderFrameHostImpl::DidCommitNavigationInternal( | |||
| 15570 | *params, did_commit_ipc_received_time); | 15576 | *params, did_commit_ipc_received_time); |
| 15571 | 15577 | ||
| 15572 | // TODO(crbug.com/40150370): Do not pass |params| to DidNavigate(). | 15578 | // TODO(crbug.com/40150370): Do not pass |params| to DidNavigate(). |
| 15573 | - NavigationRequest* raw_navigation_request = navigation_request.get(); | 15579 | + FrameTreeNode* frame_tree_node = navigation_request->frame_tree_node(); |
| 15574 | - raw_navigation_request->frame_tree_node()->navigator().DidNavigate( | 15580 | + base::WeakPtr<RenderFrameHostImpl> weak_ptr = GetWeakPtr(); |
| 15581 | + frame_tree_node->navigator().DidNavigate( | ||
| 15575 | this, *params, std::move(navigation_request), | 15582 | this, *params, std::move(navigation_request), |
| 15576 | is_same_document_navigation); | 15583 | is_same_document_navigation); |
| 15584 | + if (!weak_ptr) { | ||
| 15585 | + // This RFH may be deleted after DidNavigate due to a nested message loop. | ||
| 15586 | + // That occurs before the navigation has actually committed, so return false | ||
| 15587 | + // to indicate that the commit did not succeed. | ||
| 15588 | + return false; | ||
| 15589 | + } | ||
| 15577 | 15590 | ||
| 15578 | // Run any deferred shared storage operations from response headers now that | 15591 | // Run any deferred shared storage operations from response headers now that |
| 15579 | // commit has occurred. | 15592 | // commit has occurred. |
| @@ -6379,6 +6379,35 @@ TEST_F(InputMethodStateAuraTest, GetCaretBounds) { | |||
| 6379 | EXPECT_EQ(measured_rect, text_input_client()->GetCaretBounds()); | 6379 | EXPECT_EQ(measured_rect, text_input_client()->GetCaretBounds()); |
| 6380 | } | 6380 | } |
| 6381 | } | 6381 | } |
| 6382 | +TEST_F(InputMethodStateAuraTest, EditContextBoundsClamped) { | ||
| 6383 | + RenderWidgetHostViewAura* view = tab_view(); | ||
| 6384 | + view->SetBounds(gfx::Rect(0, 0, 800, 600)); | ||
| 6385 | + ActivateViewForTextInputManager(view, ui::TEXT_INPUT_TYPE_TEXT); | ||
| 6386 | + TextInputManager* manager = GetTextInputManager(view); | ||
| 6387 | + | ||
| 6388 | + ui::mojom::TextInputState state; | ||
| 6389 | + state.type = ui::TEXT_INPUT_TYPE_TEXT; | ||
| 6390 | + state.edit_context_control_bounds = gfx::Rect(-50, -50, 100, 100); | ||
| 6391 | + state.edit_context_selection_bounds = gfx::Rect(-50, -50, 100, 100); | ||
| 6392 | + | ||
| 6393 | + manager->UpdateTextInputState(view, state); | ||
| 6394 | + | ||
| 6395 | + std::optional<gfx::Rect> control_bounds = manager->GetTextControlBounds(); | ||
| 6396 | + std::optional<gfx::Rect> selection_bounds = manager->GetTextSelectionBounds(); | ||
| 6397 | + | ||
| 6398 | + EXPECT_TRUE(control_bounds.has_value()); | ||
| 6399 | + EXPECT_TRUE(selection_bounds.has_value()); | ||
| 6400 | + | ||
| 6401 | + // Expected adjusted bounds in view local space is (0, 0, 100, 100). | ||
| 6402 | + gfx::Rect expected_local_bounds = gfx::Rect(0, 0, 100, 100); | ||
| 6403 | + | ||
| 6404 | + gfx::Rect expected_bounds = gfx::Rect( | ||
| 6405 | + view->TransformPointToRootCoordSpace(expected_local_bounds.origin()), | ||
| 6406 | + expected_local_bounds.size()); | ||
| 6407 | + | ||
| 6408 | + EXPECT_EQ(control_bounds.value(), expected_bounds); | ||
| 6409 | + EXPECT_EQ(selection_bounds.value(), expected_bounds); | ||
| 6410 | +} | ||
| 6382 | 6411 | ||
| 6383 | // This test is for composition character bounds. | 6412 | // This test is for composition character bounds. |
| 6384 | TEST_F(InputMethodStateAuraTest, GetCompositionCharacterBounds) { | 6413 | TEST_F(InputMethodStateAuraTest, GetCompositionCharacterBounds) { |
| @@ -6387,6 +6416,7 @@ TEST_F(InputMethodStateAuraTest, GetCompositionCharacterBounds) { | |||
| 6387 | EXPECT_FALSE(text_input_client()->GetCompositionCharacterBounds(0, &bound)); | 6416 | EXPECT_FALSE(text_input_client()->GetCompositionCharacterBounds(0, &bound)); |
| 6388 | for (auto index : active_view_sequence_) { | 6417 | for (auto index : active_view_sequence_) { |
| 6389 | ActivateViewForTextInputManager(views_[index], ui::TEXT_INPUT_TYPE_TEXT); | 6418 | ActivateViewForTextInputManager(views_[index], ui::TEXT_INPUT_TYPE_TEXT); |
| 6419 | + views_[index]->SetBounds(gfx::Rect(0, 0, 800, 600)); | ||
| 6390 | // Simulate an IPC to set character bounds for the view. | 6420 | // Simulate an IPC to set character bounds for the view. |
| 6391 | views_[index]->ImeCompositionRangeChanged( | 6421 | views_[index]->ImeCompositionRangeChanged( |
| 6392 | gfx::Range(), {{gfx::Rect(1, 2, 3, 4 + index)}}); | 6422 | gfx::Range(), {{gfx::Rect(1, 2, 3, 4 + index)}}); |
| @@ -39,6 +39,19 @@ bool ShouldUpdateTextInputState(const ui::mojom::TextInputState& old_state, | |||
| 39 | 39 | ||
| 40 | } | 40 | } |
| 41 | 41 | ||
| 42 | +// We want to validate with the viewport's rect. However this lookup can be | ||
| 43 | +// invoked on a `RenderWidgetHostViewChildFrame` which has been disconnected | ||
| 44 | +// from the viewport. In such a case we return the requested size of the child | ||
| 45 | +// view | ||
| 46 | +gfx::Rect GetViewportRect(RenderWidgetHostViewBase* view) { | ||
| 47 | + auto* root_view = view->GetRootView(); | ||
| 48 | + if (root_view) { | ||
| 49 | + return gfx::Rect(root_view->GetVisibleViewportSize()); | ||
| 50 | + } else { | ||
| 51 | + return gfx::Rect(view->GetRequestedRendererSize()); | ||
| 52 | + } | ||
| 53 | +} | ||
| 54 | + | ||
| 42 | } // namespace | 55 | } // namespace |
| 43 | 56 | ||
| 44 | TextInputManager::TextInputManager() : active_view_(nullptr) {} | 57 | TextInputManager::TextInputManager() : active_view_(nullptr) {} |
| @@ -159,8 +172,9 @@ const std::optional<gfx::Rect> TextInputManager::GetTextControlBounds() const { | |||
| 159 | auto control_bounds = state->edit_context_control_bounds.value(); | 172 | auto control_bounds = state->edit_context_control_bounds.value(); |
| 160 | auto new_top_left = | 173 | auto new_top_left = |
| 161 | active_view_->TransformPointToRootCoordSpace(control_bounds.origin()); | 174 | active_view_->TransformPointToRootCoordSpace(control_bounds.origin()); |
| 162 | - return std::optional<gfx::Rect>( | 175 | + control_bounds.set_origin(new_top_left); |
| 163 | - gfx::Rect(new_top_left, control_bounds.size())); | 176 | + control_bounds.AdjustToFit(GetViewportRect(active_view_)); |
| 177 | + return control_bounds; | ||
| 164 | } | 178 | } |
| 165 | 179 | ||
| 166 | const std::optional<gfx::Rect> TextInputManager::GetTextSelectionBounds() | 180 | const std::optional<gfx::Rect> TextInputManager::GetTextSelectionBounds() |
| @@ -172,8 +186,9 @@ const std::optional<gfx::Rect> TextInputManager::GetTextSelectionBounds() | |||
| 172 | auto selection_bounds = state->edit_context_selection_bounds.value(); | 186 | auto selection_bounds = state->edit_context_selection_bounds.value(); |
| 173 | auto new_top_left = | 187 | auto new_top_left = |
| 174 | active_view_->TransformPointToRootCoordSpace(selection_bounds.origin()); | 188 | active_view_->TransformPointToRootCoordSpace(selection_bounds.origin()); |
| 175 | - return std::optional<gfx::Rect>( | 189 | + selection_bounds.set_origin(new_top_left); |
| 176 | - gfx::Rect(new_top_left, selection_bounds.size())); | 190 | + selection_bounds.AdjustToFit(GetViewportRect(active_view_)); |
| 191 | + return selection_bounds; | ||
| 177 | } | 192 | } |
| 178 | 193 | ||
| 179 | void TextInputManager::UpdateTextInputState( | 194 | void TextInputManager::UpdateTextInputState( |
| @@ -385,11 +400,16 @@ void TextInputManager::ImeCompositionRangeChanged( | |||
| 385 | if (character_bounds.has_value()) { | 400 | if (character_bounds.has_value()) { |
| 386 | composition_range_info_map_[view].character_bounds.clear(); | 401 | composition_range_info_map_[view].character_bounds.clear(); |
| 387 | 402 | ||
| 403 | + gfx::Rect viewport_rect = GetViewportRect(view); | ||
| 388 | // The values for the bounds should be converted to root view's coordinates | 404 | // The values for the bounds should be converted to root view's coordinates |
| 389 | // before being stored. | 405 | // before being stored. |
| 390 | for (auto& rect : character_bounds.value()) { | 406 | for (auto& rect : character_bounds.value()) { |
| 407 | + gfx::Rect clamped_rect = rect; | ||
| 408 | + clamped_rect.set_origin( | ||
| 409 | + view->TransformPointToRootCoordSpace(clamped_rect.origin())); | ||
| 410 | + clamped_rect.AdjustToFit(viewport_rect); | ||
| 391 | composition_range_info_map_[view].character_bounds.emplace_back( | 411 | composition_range_info_map_[view].character_bounds.emplace_back( |
| 392 | - view->TransformPointToRootCoordSpace(rect.origin()), rect.size()); | 412 | + clamped_rect); |
| 393 | } | 413 | } |
| 394 | 414 | ||
| 395 | composition_range_info_map_[view].range.set_start(range.start()); | 415 | composition_range_info_map_[view].range.set_start(range.start()); |
| @@ -0,0 +1,82 @@ | |||
| 1 | +// Copyright 2026 The Chromium Authors | ||
| 2 | +// Use of this source code is governed by a BSD-style license that can be | ||
| 3 | +// found in the LICENSE file. | ||
| 4 | + | ||
| 5 | + | ||
| 6 | + | ||
| 7 | + | ||
| 8 | + | ||
| 9 | + | ||
| 10 | + | ||
| 11 | + | ||
| 12 | + | ||
| 13 | +namespace content { | ||
| 14 | + | ||
| 15 | +class TextInputManagerTest : public RenderViewHostTestHarness { | ||
| 16 | + public: | ||
| 17 | + TextInputManagerTest() = default; | ||
| 18 | + ~TextInputManagerTest() override = default; | ||
| 19 | +}; | ||
| 20 | + | ||
| 21 | +// Test that ImeCompositionRangeChanged clamps out-of-bounds character bounds. | ||
| 22 | +TEST_F(TextInputManagerTest, ImeCompositionRangeChanged_Clamped) { | ||
| 23 | + RenderWidgetHostViewBase* view = | ||
| 24 | + static_cast<RenderWidgetHostViewBase*>(rvh()->GetWidget()->GetView()); | ||
| 25 | + | ||
| 26 | + TextInputManager* manager = view->GetTextInputManager(); | ||
| 27 | + ASSERT_TRUE(manager); | ||
| 28 | + | ||
| 29 | + // Make the view active in TextInputManager. | ||
| 30 | + ui::mojom::TextInputState state; | ||
| 31 | + state.type = ui::TEXT_INPUT_TYPE_TEXT; | ||
| 32 | + manager->UpdateTextInputState(view, state); | ||
| 33 | + | ||
| 34 | + view->SetBounds(gfx::Rect(0, 0, 800, 600)); | ||
| 35 | + | ||
| 36 | + // Simulate an IPC to set character bounds that are out of bounds (negative | ||
| 37 | + // origin). Rect: x=-10, y=-10, w=50, h=50 | ||
| 38 | + manager->ImeCompositionRangeChanged(view, gfx::Range(0, 1), | ||
| 39 | + {{gfx::Rect(-10, -10, 50, 50)}}); | ||
| 40 | + | ||
| 41 | + const TextInputManager::CompositionRangeInfo* info = | ||
| 42 | + manager->GetCompositionRangeInfo(); | ||
| 43 | + | ||
| 44 | + ASSERT_TRUE(info); | ||
| 45 | + ASSERT_EQ(info->character_bounds.size(), 1u); | ||
| 46 | + | ||
| 47 | + // Should be clamped to fit in (0, 0, 800, 600) -> (0, 0, 50, 50) | ||
| 48 | + gfx::Rect expected_bounds(0, 0, 50, 50); | ||
| 49 | + EXPECT_EQ(info->character_bounds[0], expected_bounds); | ||
| 50 | +} | ||
| 51 | + | ||
| 52 | +// Test that ImeCompositionRangeChanged does not clamp in-bounds character | ||
| 53 | +// bounds. | ||
| 54 | +TEST_F(TextInputManagerTest, ImeCompositionRangeChanged_InBounds) { | ||
| 55 | + RenderWidgetHostViewBase* view = | ||
| 56 | + static_cast<RenderWidgetHostViewBase*>(rvh()->GetWidget()->GetView()); | ||
| 57 | + | ||
| 58 | + TextInputManager* manager = view->GetTextInputManager(); | ||
| 59 | + ASSERT_TRUE(manager); | ||
| 60 | + | ||
| 61 | + // Make the view active in TextInputManager. | ||
| 62 | + ui::mojom::TextInputState state; | ||
| 63 | + state.type = ui::TEXT_INPUT_TYPE_TEXT; | ||
| 64 | + manager->UpdateTextInputState(view, state); | ||
| 65 | + | ||
| 66 | + view->SetBounds(gfx::Rect(0, 0, 800, 600)); | ||
| 67 | + | ||
| 68 | + // In bounds rect: x=10, y=10, w=50, h=50 | ||
| 69 | + manager->ImeCompositionRangeChanged(view, gfx::Range(0, 1), | ||
| 70 | + {{gfx::Rect(10, 10, 50, 50)}}); | ||
| 71 | + | ||
| 72 | + const TextInputManager::CompositionRangeInfo* info = | ||
| 73 | + manager->GetCompositionRangeInfo(); | ||
| 74 | + | ||
| 75 | + ASSERT_TRUE(info); | ||
| 76 | + ASSERT_EQ(info->character_bounds.size(), 1u); | ||
| 77 | + | ||
| 78 | + gfx::Rect expected_bounds(10, 10, 50, 50); | ||
| 79 | + EXPECT_EQ(info->character_bounds[0], expected_bounds); | ||
| 80 | +} | ||
| 81 | + | ||
| 82 | +} // namespace content | ||
| @@ -986,10 +986,13 @@ void ServiceWorkerContextCore::RemoveLiveVersion(int64_t id) { | |||
| 986 | auto it = live_versions_.find(id); | 986 | auto it = live_versions_.find(id); |
| 987 | CHECK(it != live_versions_.end()); | 987 | CHECK(it != live_versions_.end()); |
| 988 | ServiceWorkerVersion* version = it->second; | 988 | ServiceWorkerVersion* version = it->second; |
| 989 | + // Erase from the map before notifying observers to prevent re-entrancy: | ||
| 990 | + // synchronous observers could otherwise look up this version via | ||
| 991 | + // `GetLiveVersion()` and resurrect it with a new `scoped_refptr`. | ||
| 992 | + live_versions_.erase(it); | ||
| 989 | 993 | ||
| 990 | if (version->running_status() != blink::EmbeddedWorkerStatus::kStopped) { | 994 | if (version->running_status() != blink::EmbeddedWorkerStatus::kStopped) { |
| 991 | - // Notify all observers that this live version is stopped, as it will | 995 | + // Notify all observers that this live version is stopped. |
| 992 | - // be removed from |live_versions_|. | ||
| 993 | observer_list_->Notify(FROM_HERE, | 996 | observer_list_->Notify(FROM_HERE, |
| 994 | &ServiceWorkerContextCoreObserver::OnStopped, id); | 997 | &ServiceWorkerContextCoreObserver::OnStopped, id); |
| 995 | for (auto& observer : sync_observer_list_->observers) { | 998 | for (auto& observer : sync_observer_list_->observers) { |
| @@ -1011,8 +1014,6 @@ void ServiceWorkerContextCore::RemoveLiveVersion(int64_t id) { | |||
| 1011 | 1014 | ||
| 1012 | observer_list_->Notify( | 1015 | observer_list_->Notify( |
| 1013 | FROM_HERE, &ServiceWorkerContextCoreObserver::OnLiveVersionDestroyed, id); | 1016 | FROM_HERE, &ServiceWorkerContextCoreObserver::OnLiveVersionDestroyed, id); |
| 1014 | - | ||
| 1015 | - live_versions_.erase(it); | ||
| 1016 | } | 1017 | } |
| 1017 | 1018 | ||
| 1018 | std::vector<ServiceWorkerRegistrationInfo> | 1019 | std::vector<ServiceWorkerRegistrationInfo> |
| @@ -159,6 +159,14 @@ class ServiceWorkerVersionTest | |||
| 159 | return version->ping_controller_.IsActivated(); | 159 | return version->ping_controller_.IsActivated(); |
| 160 | } | 160 | } |
| 161 | 161 | ||
| 162 | + void StopTimeoutTimer(ServiceWorkerVersion* version) const { | ||
| 163 | + version->StopTimeoutTimer(); | ||
| 164 | + } | ||
| 165 | + | ||
| 166 | + void ResetWorkerHost(ServiceWorkerVersion* version) const { | ||
| 167 | + version->worker_host_.reset(); | ||
| 168 | + } | ||
| 169 | + | ||
| 162 | void NotifyScriptEvaluationStart(ServiceWorkerVersion* version) { | 170 | void NotifyScriptEvaluationStart(ServiceWorkerVersion* version) { |
| 163 | version->OnScriptEvaluationStart(); | 171 | version->OnScriptEvaluationStart(); |
| 164 | } | 172 | } |
| @@ -2187,5 +2195,80 @@ TEST_P(ServiceWorkerVersionTest, NoUsbEventHandler) { | |||
| 2187 | EXPECT_FALSE(version_->has_usb_event_handlers()); | 2195 | EXPECT_FALSE(version_->has_usb_event_handlers()); |
| 2188 | } | 2196 | } |
| 2189 | 2197 | ||
| 2198 | +// A synchronous observer that checks whether the `ServiceWorkerVersion` is | ||
| 2199 | +// still discoverable in `ServiceWorkerContextCore` while `OnStoppedSync()` is | ||
| 2200 | +// executing. Part of the regression test for https://crbug.com/513424000. | ||
| 2201 | +class SyncObserverForTest : public ServiceWorkerContextObserverSynchronous { | ||
| 2202 | + public: | ||
| 2203 | + SyncObserverForTest(ServiceWorkerContextWrapper* wrapper, | ||
| 2204 | + int64_t expected_version_id) | ||
| 2205 | + : wrapper_(wrapper), expected_version_id_(expected_version_id) {} | ||
| 2206 | + | ||
| 2207 | + void OnStoppedSync( | ||
| 2208 | + int64_t version_id, | ||
| 2209 | + const GURL& scope, | ||
| 2210 | + const blink::ServiceWorkerToken& service_worker_token) override { | ||
| 2211 | + if (version_id != expected_version_id_) { | ||
| 2212 | + return; | ||
| 2213 | + } | ||
| 2214 | + | ||
| 2215 | + called_ = true; | ||
| 2216 | + // Verify that the version is erased from `live_versions_` before observers | ||
| 2217 | + // are notified. If it were still in the map, looking it up would wrap a raw | ||
| 2218 | + // pointer undergoing destruction into a new `scoped_refptr`, causing a | ||
| 2219 | + // double free. | ||
| 2220 | + EXPECT_FALSE(wrapper_->GetLiveVersion(version_id)); | ||
| 2221 | + } | ||
| 2222 | + | ||
| 2223 | + bool called() const { return called_; } | ||
| 2224 | + | ||
| 2225 | + private: | ||
| 2226 | + raw_ptr<ServiceWorkerContextWrapper> wrapper_; | ||
| 2227 | + int64_t expected_version_id_; | ||
| 2228 | + bool called_ = false; | ||
| 2229 | +}; | ||
| 2230 | + | ||
| 2231 | +// Verifies that during the destruction of a `ServiceWorkerVersion`, synchronous | ||
| 2232 | +// observers notified via `OnStoppedSync()` cannot re-acquire a reference to the | ||
| 2233 | +// version being destroyed (e.g. via `GetLiveVersion()`). This ensures the | ||
| 2234 | +// version is erased from the live map prior to observer notifications, | ||
| 2235 | +// preventing double-free and use-after-free vulnerabilities. | ||
| 2236 | +// Regression test for https://crbug.com/513424000. | ||
| 2237 | +TEST_P(ServiceWorkerVersionTest, RemoveLiveVersion_NoResurrection) { | ||
| 2238 | + // Ensure the version starts with exactly 1 reference, representing the sole | ||
| 2239 | + // reference held by `version_`. | ||
| 2240 | + EXPECT_TRUE(version_->HasOneRef()); | ||
| 2241 | + | ||
| 2242 | + // Initiate starting the worker. This transitions `running_status` to | ||
| 2243 | + // `kStarting`, which ensures that when the version is destroyed, | ||
| 2244 | + // `RemoveLiveVersion()` will notify `OnStoppedSync()` observers. | ||
| 2245 | + version_->StartWorker(ServiceWorkerMetrics::EventType::UNKNOWN, | ||
| 2246 | + base::DoNothing()); | ||
| 2247 | + EXPECT_EQ(blink::EmbeddedWorkerStatus::kStarting, version_->running_status()); | ||
| 2248 | + | ||
| 2249 | + // Attach a synchronous observer to intercept `OnStoppedSync()`. | ||
| 2250 | + SyncObserverForTest sync_observer(helper_->context_wrapper(), | ||
| 2251 | + version_->version_id()); | ||
| 2252 | + helper_->context_wrapper()->AddSyncObserver(&sync_observer); | ||
| 2253 | + | ||
| 2254 | + // Drop all internal references (e.g., pending timer callbacks and worker | ||
| 2255 | + // hosts) that were created by `StartWorker()`, so that `version_` remains the | ||
| 2256 | + // only reference. | ||
| 2257 | + StopTimeoutTimer(version_.get()); | ||
| 2258 | + ResetWorkerHost(version_.get()); | ||
| 2259 | + registration_.reset(); | ||
| 2260 | + EXPECT_TRUE(version_->HasOneRef()); | ||
| 2261 | + | ||
| 2262 | + // Trigger destruction of the version. As part of destruction, | ||
| 2263 | + // `RemoveLiveVersion()` is invoked. The observer verifies that the version is | ||
| 2264 | + // erased from the live map prior to the observer notification. | ||
| 2265 | + version_.reset(); | ||
| 2266 | + EXPECT_TRUE(sync_observer.called()); | ||
| 2267 | + | ||
| 2268 | + // Unregister the observer before it goes out of scope to prevent dangling | ||
| 2269 | + // pointer crashes during fixture teardown. | ||
| 2270 | + helper_->context_wrapper()->RemoveSyncObserver(&sync_observer); | ||
| 2271 | +} | ||
| 2272 | + | ||
| 2190 | } // namespace service_worker_version_unittest | 2273 | } // namespace service_worker_version_unittest |
| 2191 | } // namespace content | 2274 | } // namespace content |
| @@ -15131,6 +15131,41 @@ TEST_F(BidderWorkletTest, CrossOrigin) { | |||
| 15131 | BidderWorklet::SignalsOriginRelation::kCrossOriginSignals, 5); | 15131 | BidderWorklet::SignalsOriginRelation::kCrossOriginSignals, 5); |
| 15132 | } | 15132 | } |
| 15133 | 15133 | ||
| 15134 | +TEST_F(BidderWorkletTest, MicroTaskTiming) { | ||
| 15135 | + const char kScript[] = R"( | ||
| 15136 | + function generateBid() { | ||
| 15137 | + Promise.resolve().then(() => { | ||
| 15138 | + /* If this runs at wrong time, the error will be top-level timeout, | ||
| 15139 | + * not generateBid one */ | ||
| 15140 | + while(true) {} | ||
| 15141 | + }); | ||
| 15142 | + | ||
| 15143 | + while(true) {}; | ||
| 15144 | + } | ||
| 15145 | + )"; | ||
| 15146 | + | ||
| 15147 | + mojo::Remote<mojom::BidderWorklet> bidder_worklet = CreateWorklet(); | ||
| 15148 | + AddJavascriptResponse(&url_loader_factory_, interest_group_bidding_url_, | ||
| 15149 | + kScript); | ||
| 15150 | + GenerateBid(bidder_worklet.get()); | ||
| 15151 | + generate_bid_run_loop_ = std::make_unique<base::RunLoop>(); | ||
| 15152 | + generate_bid_run_loop_->Run(); | ||
| 15153 | + EXPECT_EQ(0u, bids_.size()); | ||
| 15154 | + EXPECT_THAT(bid_errors_, | ||
| 15155 | + testing::ElementsAre( | ||
| 15156 | + "https://url.test/ execution of `generateBid` timed out.")); | ||
| 15157 | + | ||
| 15158 | + // Second run should have the same behavior, not different one due to | ||
| 15159 | + // wrong timing of utask execution. | ||
| 15160 | + GenerateBid(bidder_worklet.get()); | ||
| 15161 | + generate_bid_run_loop_ = std::make_unique<base::RunLoop>(); | ||
| 15162 | + generate_bid_run_loop_->Run(); | ||
| 15163 | + EXPECT_EQ(0u, bids_.size()); | ||
| 15164 | + EXPECT_THAT(bid_errors_, | ||
| 15165 | + testing::ElementsAre( | ||
| 15166 | + "https://url.test/ execution of `generateBid` timed out.")); | ||
| 15167 | +} | ||
| 15168 | + | ||
| 15134 | class BidderWorkletRealTimeReportingEnabledTest : public BidderWorkletTest { | 15169 | class BidderWorkletRealTimeReportingEnabledTest : public BidderWorkletTest { |
| 15135 | public: | 15170 | public: |
| 15136 | BidderWorkletRealTimeReportingEnabledTest() { | 15171 | BidderWorkletRealTimeReportingEnabledTest() { |
| @@ -190,6 +190,12 @@ v8::Local<v8::Context> ContextRecycler::GetContext() { | |||
| 190 | } | 190 | } |
| 191 | 191 | ||
| 192 | void ContextRecycler::ResetForReuse() { | 192 | void ContextRecycler::ResetForReuse() { |
| 193 | + // Make sure that microtasks get flushed as they would not on timeout. | ||
| 194 | + { | ||
| 195 | + AuctionV8Helper::TimeLimitScope time_scope(v8_helper_->GetTimeLimit()); | ||
| 196 | + v8_helper_->isolate()->PerformMicrotaskCheckpoint(); | ||
| 197 | + } | ||
| 198 | + | ||
| 193 | for (Bindings* bindings : bindings_list_) { | 199 | for (Bindings* bindings : bindings_list_) { |
| 194 | bindings->Reset(); | 200 | bindings->Reset(); |
| 195 | } | 201 | } |
| @@ -2817,6 +2817,7 @@ test("content_unittests") { | |||
| 2817 | "../browser/renderer_host/render_widget_host_view_aura_unittest.cc", | 2817 | "../browser/renderer_host/render_widget_host_view_aura_unittest.cc", |
| 2818 | "../browser/renderer_host/render_widget_host_view_base_unittest.cc", | 2818 | "../browser/renderer_host/render_widget_host_view_base_unittest.cc", |
| 2819 | "../browser/renderer_host/render_widget_host_view_child_frame_unittest.cc", | 2819 | "../browser/renderer_host/render_widget_host_view_child_frame_unittest.cc", |
| 2820 | + "../browser/renderer_host/text_input_manager_unittest.cc", | ||
| 2820 | "../browser/renderer_host/transient_allow_popup_unittest.cc", | 2821 | "../browser/renderer_host/transient_allow_popup_unittest.cc", |
| 2821 | "../browser/renderer_host/visible_time_request_trigger_unittest.cc", | 2822 | "../browser/renderer_host/visible_time_request_trigger_unittest.cc", |
| 2822 | "../browser/scheduler/browser_io_thread_delegate_unittest.cc", | 2823 | "../browser/scheduler/browser_io_thread_delegate_unittest.cc", |
| @@ -171,8 +171,16 @@ void TestRenderWidgetHostView::Destroy() { | |||
| 171 | delete this; | 171 | delete this; |
| 172 | } | 172 | } |
| 173 | 173 | ||
| 174 | +void TestRenderWidgetHostView::SetSize(const gfx::Size& size) { | ||
| 175 | + bounds_.set_size(size); | ||
| 176 | +} | ||
| 177 | + | ||
| 178 | +void TestRenderWidgetHostView::SetBounds(const gfx::Rect& rect) { | ||
| 179 | + bounds_ = rect; | ||
| 180 | +} | ||
| 181 | + | ||
| 174 | gfx::Rect TestRenderWidgetHostView::GetViewBounds() { | 182 | gfx::Rect TestRenderWidgetHostView::GetViewBounds() { |
| 175 | - return gfx::Rect(); | 183 | + return bounds_; |
| 176 | } | 184 | } |
| 177 | 185 | ||
| 178 | 186 | ||
| @@ -68,8 +68,8 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase, | |||
| 68 | 68 | ||
| 69 | // RenderWidgetHostView: | 69 | // RenderWidgetHostView: |
| 70 | void InitAsChild(gfx::NativeView parent_view) override {} | 70 | void InitAsChild(gfx::NativeView parent_view) override {} |
| 71 | - void SetSize(const gfx::Size& size) override {} | 71 | + void SetSize(const gfx::Size& size) override; |
| 72 | - void SetBounds(const gfx::Rect& rect) override {} | 72 | + void SetBounds(const gfx::Rect& rect) override; |
| 73 | gfx::NativeView GetNativeView() override; | 73 | gfx::NativeView GetNativeView() override; |
| 74 | gfx::NativeViewAccessible GetNativeViewAccessible() override; | 74 | gfx::NativeViewAccessible GetNativeViewAccessible() override; |
| 75 | ui::TextInputClient* GetTextInputClient() override; | 75 | ui::TextInputClient* GetTextInputClient() override; |
| @@ -210,6 +210,8 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase, | |||
| 210 | raw_ptr<ui::Compositor, DanglingUntriaged> compositor_ = nullptr; | 210 | raw_ptr<ui::Compositor, DanglingUntriaged> compositor_ = nullptr; |
| 211 | 211 | ||
| 212 | input::CursorManager cursor_manager_; | 212 | input::CursorManager cursor_manager_; |
| 213 | + | ||
| 214 | + gfx::Rect bounds_; | ||
| 213 | }; | 215 | }; |
| 214 | 216 | ||
| 215 | // TestRenderWidgetHostViewChildFrame ----------------------------------------- | 217 | // TestRenderWidgetHostViewChildFrame ----------------------------------------- |
| @@ -286,6 +286,20 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal( | |||
| 286 | 286 | ||
| 287 | const GURL* render_frame_host_url = nullptr; | 287 | const GURL* render_frame_host_url = nullptr; |
| 288 | if (render_frame_host) { | 288 | if (render_frame_host) { |
| 289 | + // Error pages commit with the target URL and in their parents' process, but | ||
| 290 | + // aren't actually on the page indicated by the committed URL. | ||
| 291 | + // Bail out in this case. Error pages don't use extension APIs. | ||
| 292 | + // If we had perfect timing and no race conditions, this could be a sign of | ||
| 293 | + // a bad message; however, it's possible a page commits to an error page | ||
| 294 | + // after a legitimate message is sent. | ||
| 295 | + if (render_frame_host->IsErrorDocument()) { | ||
| 296 | + constexpr char kCannotUseExtensionAPIsInErrorPages[] = | ||
| 297 | + "Cannot call extension APIs from error pages."; | ||
| 298 | + ResponseCallbackOnError(std::move(callback), | ||
| 299 | + ExtensionFunction::ResponseType::kFailed, | ||
| 300 | + kCannotUseExtensionAPIsInErrorPages); | ||
| 301 | + return; | ||
| 302 | + } | ||
| 289 | render_frame_host_url = &render_frame_host->GetLastCommittedURL(); | 303 | render_frame_host_url = &render_frame_host->GetLastCommittedURL(); |
| 290 | DCHECK_EQ(render_process_id, | 304 | DCHECK_EQ(render_process_id, |
| 291 | render_frame_host->GetProcess()->GetDeprecatedID()); | 305 | render_frame_host->GetProcess()->GetDeprecatedID()); |
| @@ -142,9 +142,15 @@ void PostMessageSupport::PostJavaScriptMessage(v8::Isolate* isolate, | |||
| 142 | v8::Local<v8::Object> target_window_proxy = | 142 | v8::Local<v8::Object> target_window_proxy = |
| 143 | target_frame->GlobalProxy(isolate); | 143 | target_frame->GlobalProxy(isolate); |
| 144 | gin::Dictionary window_object(isolate, target_window_proxy); | 144 | gin::Dictionary window_object(isolate, target_window_proxy); |
| 145 | + auto weak_this = weak_factory_.GetWeakPtr(); | ||
| 145 | v8::Local<v8::Function> post_message; | 146 | v8::Local<v8::Function> post_message; |
| 146 | if (!window_object.Get(std::string(kPostMessageName), &post_message)) | 147 | if (!window_object.Get(std::string(kPostMessageName), &post_message)) |
| 147 | return; | 148 | return; |
| 149 | + if (!weak_this) { | ||
| 150 | + // Getting the function may have executed a malicious script that destroyed | ||
| 151 | + // `this`. See https://crbug.com/516910450 | ||
| 152 | + return; | ||
| 153 | + } | ||
| 148 | 154 | ||
| 149 | v8::Local<v8::Value> args[] = { | 155 | v8::Local<v8::Value> args[] = { |
| 150 | message, | 156 | message, |
| @@ -668,6 +668,13 @@ bool Framebuffer::GetReadBufferIsMultisampledTexture() const { | |||
| 668 | : false; | 668 | : false; |
| 669 | } | 669 | } |
| 670 | 670 | ||
| 671 | +bool Framebuffer::GetReadBufferIsMultisampledRenderbuffer() const { | ||
| 672 | + const Attachment* attachment = GetReadBufferAttachment(); | ||
| 673 | + return attachment ? attachment->IsRenderbufferAttachment() && | ||
| 674 | + attachment->samples() > 1 | ||
| 675 | + : false; | ||
| 676 | +} | ||
| 677 | + | ||
| 671 | GLsizei Framebuffer::GetSamples() const { | 678 | GLsizei Framebuffer::GetSamples() const { |
| 672 | // Assume the framebuffer is complete, so return any attachment's samples. | 679 | // Assume the framebuffer is complete, so return any attachment's samples. |
| 673 | auto iter = attachments_.begin(); | 680 | auto iter = attachments_.begin(); |
| @@ -169,6 +169,7 @@ class GPU_GLES2_EXPORT Framebuffer : public base::RefCounted<Framebuffer> { | |||
| 169 | // returns 0. | 169 | // returns 0. |
| 170 | GLenum GetReadBufferTextureType() const; | 170 | GLenum GetReadBufferTextureType() const; |
| 171 | bool GetReadBufferIsMultisampledTexture() const; | 171 | bool GetReadBufferIsMultisampledTexture() const; |
| 172 | + bool GetReadBufferIsMultisampledRenderbuffer() const; | ||
| 172 | 173 | ||
| 173 | // Verify all the rules in OpenGL ES 2.0.25 4.4.5 are followed. | 174 | // Verify all the rules in OpenGL ES 2.0.25 4.4.5 are followed. |
| 174 | // Returns GL_FRAMEBUFFER_COMPLETE if there are no reasons we know we can't | 175 | // Returns GL_FRAMEBUFFER_COMPLETE if there are no reasons we know we can't |
| @@ -933,6 +933,7 @@ class GLES2DecoderImpl : public GLES2Decoder, | |||
| 933 | // If the color image is a renderbuffer, returns 0 for type. | 933 | // If the color image is a renderbuffer, returns 0 for type. |
| 934 | GLenum GetBoundReadFramebufferTextureType(); | 934 | GLenum GetBoundReadFramebufferTextureType(); |
| 935 | GLenum GetBoundReadFramebufferInternalFormat(); | 935 | GLenum GetBoundReadFramebufferInternalFormat(); |
| 936 | + bool IsBoundReadFramebufferMultisampledRenderbuffer(); | ||
| 936 | 937 | ||
| 937 | // Get the i-th draw buffer's internal format/type from the bound framebuffer. | 938 | // Get the i-th draw buffer's internal format/type from the bound framebuffer. |
| 938 | // If no framebuffer is bound, or no image is attached, or the DrawBuffers | 939 | // If no framebuffer is bound, or no image is attached, or the DrawBuffers |
| @@ -4275,6 +4276,18 @@ GLenum GLES2DecoderImpl::GetBoundReadFramebufferInternalFormat() { | |||
| 4275 | } | 4276 | } |
| 4276 | } | 4277 | } |
| 4277 | 4278 | ||
| 4279 | +bool GLES2DecoderImpl::IsBoundReadFramebufferMultisampledRenderbuffer() { | ||
| 4280 | + Framebuffer* read_framebuffer = GetBoundReadFramebuffer(); | ||
| 4281 | + // We check if the read framebuffer is a multisampled renderbuffer. | ||
| 4282 | + // We explicitly do NOT block multisampled texture attachments (with target | ||
| 4283 | + // GL_TEXTURE_2D, etc.) because they are implicit resolve textures (allocated | ||
| 4284 | + // via FramebufferTexture2DMultisampleEXT) and are safe to copy from (they are | ||
| 4285 | + // resolved on the fly). True multisampled textures | ||
| 4286 | + // (GL_TEXTURE_2D_MULTISAMPLE) are not supported by the validating decoder. | ||
| 4287 | + return read_framebuffer && | ||
| 4288 | + read_framebuffer->GetReadBufferIsMultisampledRenderbuffer(); | ||
| 4289 | +} | ||
| 4290 | + | ||
| 4278 | GLenum GLES2DecoderImpl::GetBoundColorDrawBufferType(GLint drawbuffer_i) { | 4291 | GLenum GLES2DecoderImpl::GetBoundColorDrawBufferType(GLint drawbuffer_i) { |
| 4279 | DCHECK(drawbuffer_i >= 0 && | 4292 | DCHECK(drawbuffer_i >= 0 && |
| 4280 | drawbuffer_i < static_cast<GLint>(group_->max_draw_buffers())); | 4293 | drawbuffer_i < static_cast<GLint>(group_->max_draw_buffers())); |
| @@ -13470,6 +13483,12 @@ void GLES2DecoderImpl::DoCopyTexImage2D( | |||
| 13470 | return; | 13483 | return; |
| 13471 | } | 13484 | } |
| 13472 | 13485 | ||
| 13486 | + if (IsBoundReadFramebufferMultisampledRenderbuffer()) { | ||
| 13487 | + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name, | ||
| 13488 | + "cannot copy from a multisampled framebuffer"); | ||
| 13489 | + return; | ||
| 13490 | + } | ||
| 13491 | + | ||
| 13473 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); | 13492 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); |
| 13474 | GLenum read_type = GetBoundReadFramebufferTextureType(); | 13493 | GLenum read_type = GetBoundReadFramebufferTextureType(); |
| 13475 | if (!ValidateCopyTexFormat(func_name, internal_format, | 13494 | if (!ValidateCopyTexFormat(func_name, internal_format, |
| @@ -13722,6 +13741,12 @@ void GLES2DecoderImpl::DoCopyTexSubImage2D( | |||
| 13722 | return; | 13741 | return; |
| 13723 | } | 13742 | } |
| 13724 | 13743 | ||
| 13744 | + if (IsBoundReadFramebufferMultisampledRenderbuffer()) { | ||
| 13745 | + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name, | ||
| 13746 | + "cannot copy from a multisampled framebuffer"); | ||
| 13747 | + return; | ||
| 13748 | + } | ||
| 13749 | + | ||
| 13725 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); | 13750 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); |
| 13726 | GLenum read_type = GetBoundReadFramebufferTextureType(); | 13751 | GLenum read_type = GetBoundReadFramebufferTextureType(); |
| 13727 | if (!ValidateCopyTexFormat(func_name, internal_format, | 13752 | if (!ValidateCopyTexFormat(func_name, internal_format, |
| @@ -13836,6 +13861,12 @@ void GLES2DecoderImpl::DoCopyTexSubImage3D( | |||
| 13836 | return; | 13861 | return; |
| 13837 | } | 13862 | } |
| 13838 | 13863 | ||
| 13864 | + if (IsBoundReadFramebufferMultisampledRenderbuffer()) { | ||
| 13865 | + LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name, | ||
| 13866 | + "cannot copy from a multisampled framebuffer"); | ||
| 13867 | + return; | ||
| 13868 | + } | ||
| 13869 | + | ||
| 13839 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); | 13870 | GLenum read_format = GetBoundReadFramebufferInternalFormat(); |
| 13840 | GLenum read_type = GetBoundReadFramebufferTextureType(); | 13871 | GLenum read_type = GetBoundReadFramebufferTextureType(); |
| 13841 | if (!ValidateCopyTexFormat(func_name, internal_format, | 13872 | if (!ValidateCopyTexFormat(func_name, internal_format, |
| @@ -16138,6 +16169,27 @@ void GLES2DecoderImpl::TexStorageImpl(GLenum target, | |||
| 16138 | compatibility_internal_format = format_info->decompressed_internal_format; | 16169 | compatibility_internal_format = format_info->decompressed_internal_format; |
| 16139 | } | 16170 | } |
| 16140 | 16171 | ||
| 16172 | + // TODO(zmo): We might need to emulate TexStorage using TexImage or | ||
| 16173 | + // CompressedTexImage on Mac OSX where we expose ES3 APIs when the underlying | ||
| 16174 | + // driver is lower than 4.2 and ARB_texture_storage extension doesn't exist. | ||
| 16175 | + LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER(function_name); | ||
| 16176 | + if (dimension == ContextState::k2D) { | ||
| 16177 | + api()->glTexStorage2DEXTFn(target, levels, compatibility_internal_format, | ||
| 16178 | + width, height); | ||
| 16179 | + } else { | ||
| 16180 | + api()->glTexStorage3DFn(target, levels, compatibility_internal_format, | ||
| 16181 | + width, height, depth); | ||
| 16182 | + } | ||
| 16183 | + GLenum error = LOCAL_PEEK_GL_ERROR(function_name); | ||
| 16184 | + if (error != GL_NO_ERROR) { | ||
| 16185 | + // The driver rejected the allocation. Do NOT update the decoder-side | ||
| 16186 | + // LevelInfo / immutable flag, otherwise subsequent TexSubImage bounds | ||
| 16187 | + // checks (Texture::ValidForTexture) would validate against dimensions | ||
| 16188 | + // that the driver never allocated, allowing oversized writes to be | ||
| 16189 | + // forwarded to the native driver. | ||
| 16190 | + return; | ||
| 16191 | + } | ||
| 16192 | + | ||
| 16141 | { | 16193 | { |
| 16142 | GLsizei level_width = width; | 16194 | GLsizei level_width = width; |
| 16143 | GLsizei level_height = height; | 16195 | GLsizei level_height = height; |
| @@ -16166,17 +16218,6 @@ void GLES2DecoderImpl::TexStorageImpl(GLenum target, | |||
| 16166 | texture->ApplyFormatWorkarounds(feature_info_.get()); | 16218 | texture->ApplyFormatWorkarounds(feature_info_.get()); |
| 16167 | texture->SetImmutable(true, true); | 16219 | texture->SetImmutable(true, true); |
| 16168 | } | 16220 | } |
| 16169 | - | ||
| 16170 | - // TODO(zmo): We might need to emulate TexStorage using TexImage or | ||
| 16171 | - // CompressedTexImage on Mac OSX where we expose ES3 APIs when the underlying | ||
| 16172 | - // driver is lower than 4.2 and ARB_texture_storage extension doesn't exist. | ||
| 16173 | - if (dimension == ContextState::k2D) { | ||
| 16174 | - api()->glTexStorage2DEXTFn(target, levels, compatibility_internal_format, | ||
| 16175 | - width, height); | ||
| 16176 | - } else { | ||
| 16177 | - api()->glTexStorage3DFn(target, levels, compatibility_internal_format, | ||
| 16178 | - width, height, depth); | ||
| 16179 | - } | ||
| 16180 | } | 16221 | } |
| 16181 | 16222 | ||
| 16182 | void GLES2DecoderImpl::DoTexStorage2DEXT(GLenum target, | 16223 | void GLES2DecoderImpl::DoTexStorage2DEXT(GLenum target, |
| @@ -1193,6 +1193,10 @@ TEST_P(GLES2DecoderManualInitTest, MemoryTrackerTexStorage2DEXT) { | |||
| 1193 | EXPECT_CALL(*gl_, TexStorage2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, 8, 4)) | 1193 | EXPECT_CALL(*gl_, TexStorage2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, 8, 4)) |
| 1194 | .Times(1) | 1194 | .Times(1) |
| 1195 | .RetiresOnSaturation(); | 1195 | .RetiresOnSaturation(); |
| 1196 | + EXPECT_CALL(*gl_, GetError()) | ||
| 1197 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 1198 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 1199 | + .RetiresOnSaturation(); | ||
| 1196 | cmds::TexStorage2DEXT cmd; | 1200 | cmds::TexStorage2DEXT cmd; |
| 1197 | cmd.Init(GL_TEXTURE_2D, 1, GL_RGBA8, 8, 4); | 1201 | cmd.Init(GL_TEXTURE_2D, 1, GL_RGBA8, 8, 4); |
| 1198 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); | 1202 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); |
| @@ -3606,6 +3606,10 @@ class GLES2DecoderTexStorageFormatAndTypeTest | |||
| 3606 | *gl_, TexStorage2DEXT(GL_TEXTURE_2D, kLevels, format, kWidth, kHeight)) | 3606 | *gl_, TexStorage2DEXT(GL_TEXTURE_2D, kLevels, format, kWidth, kHeight)) |
| 3607 | .Times(1) | 3607 | .Times(1) |
| 3608 | .RetiresOnSaturation(); | 3608 | .RetiresOnSaturation(); |
| 3609 | + EXPECT_CALL(*gl_, GetError()) | ||
| 3610 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 3611 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 3612 | + .RetiresOnSaturation(); | ||
| 3609 | cmds::TexStorage2DEXT cmd; | 3613 | cmds::TexStorage2DEXT cmd; |
| 3610 | cmd.Init(GL_TEXTURE_2D, kLevels, format, kWidth, kHeight); | 3614 | cmd.Init(GL_TEXTURE_2D, kLevels, format, kWidth, kHeight); |
| 3611 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); | 3615 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); |
| @@ -3672,6 +3676,10 @@ TEST_P(GLES3DecoderTest, TexStorage3DValidArgs) { | |||
| 3672 | EXPECT_CALL(*gl_, TexStorage3D(GL_TEXTURE_3D, 2, GL_RGB565, 4, 5, 6)) | 3676 | EXPECT_CALL(*gl_, TexStorage3D(GL_TEXTURE_3D, 2, GL_RGB565, 4, 5, 6)) |
| 3673 | .Times(1) | 3677 | .Times(1) |
| 3674 | .RetiresOnSaturation(); | 3678 | .RetiresOnSaturation(); |
| 3679 | + EXPECT_CALL(*gl_, GetError()) | ||
| 3680 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 3681 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 3682 | + .RetiresOnSaturation(); | ||
| 3675 | cmds::TexStorage3D cmd; | 3683 | cmds::TexStorage3D cmd; |
| 3676 | cmd.Init(GL_TEXTURE_3D, 2, GL_RGB565, 4, 5, 6); | 3684 | cmd.Init(GL_TEXTURE_3D, 2, GL_RGB565, 4, 5, 6); |
| 3677 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); | 3685 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); |
| @@ -4113,6 +4121,10 @@ TEST_P(GLES3DecoderTest, ImmutableTextureBaseLevelMaxLevelClamping) { | |||
| 4113 | kHeight, kDepth)) | 4121 | kHeight, kDepth)) |
| 4114 | .Times(1) | 4122 | .Times(1) |
| 4115 | .RetiresOnSaturation(); | 4123 | .RetiresOnSaturation(); |
| 4124 | + EXPECT_CALL(*gl_, GetError()) | ||
| 4125 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 4126 | + .WillOnce(Return(GL_NO_ERROR)) | ||
| 4127 | + .RetiresOnSaturation(); | ||
| 4116 | cmds::TexStorage3D cmd; | 4128 | cmds::TexStorage3D cmd; |
| 4117 | cmd.Init(kTarget, kLevels, kInternalFormat, kWidth, kHeight, kDepth); | 4129 | cmd.Init(kTarget, kLevels, kInternalFormat, kWidth, kHeight, kDepth); |
| 4118 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); | 4130 | EXPECT_EQ(error::kNoError, ExecuteCmd(cmd)); |
| @@ -8,6 +8,9 @@ | |||
| 8 | 8 | ||
| 9 | 9 | ||
| 10 | 10 | ||
| 11 | + | ||
| 12 | + | ||
| 13 | + | ||
| 11 | 14 | ||
| 12 | 15 | ||
| 13 | 16 | ||
| @@ -109,6 +112,15 @@ Response TargetHandler::CreateTarget( | |||
| 109 | HeadlessWebContentsImpl* web_contents_impl = HeadlessWebContentsImpl::From( | 112 | HeadlessWebContentsImpl* web_contents_impl = HeadlessWebContentsImpl::From( |
| 110 | context->CreateWebContentsBuilder().SetInitialURL(gurl).Build()); | 113 | context->CreateWebContentsBuilder().SetInitialURL(gurl).Build()); |
| 111 | 114 | ||
| 115 | + // Mark the process used so IsSuitableHost() rejects it for sites that | ||
| 116 | + // require a dedicated process. (Mirrors content::HiddenTargetManager, which | ||
| 117 | + // the headless embedder layer bypasses by handling Target.createTarget | ||
| 118 | + // itself.) | ||
| 119 | + web_contents_impl->web_contents() | ||
| 120 | + ->GetPrimaryMainFrame() | ||
| 121 | + ->GetProcess() | ||
| 122 | + ->SetIsUsed(); | ||
| 123 | + | ||
| 112 | *out_target_id = content::DevToolsAgentHost::GetOrCreateFor( | 124 | *out_target_id = content::DevToolsAgentHost::GetOrCreateFor( |
| 113 | web_contents_impl->web_contents()) | 125 | web_contents_impl->web_contents()) |
| 114 | ->GetId(); | 126 | ->GetId(); |
| @@ -10,6 +10,8 @@ | |||
| 10 | 10 | ||
| 11 | 11 | ||
| 12 | 12 | ||
| 13 | + | ||
| 14 | + | ||
| 13 | 15 | ||
| 14 | 16 | ||
| 15 | 17 | ||
| @@ -97,4 +99,52 @@ void HeadlessDevTooledBrowserTest::CustomizeHeadlessBrowserContext( | |||
| 97 | void HeadlessDevTooledBrowserTest::CustomizeHeadlessWebContents( | 99 | void HeadlessDevTooledBrowserTest::CustomizeHeadlessWebContents( |
| 98 | HeadlessWebContents::Builder& builder) {} | 100 | HeadlessWebContents::Builder& builder) {} |
| 99 | 101 | ||
| 102 | +class HeadlessCreatedTargetIsUsedProcessTest | ||
| 103 | + : public HeadlessDevTooledBrowserTest, | ||
| 104 | + public testing::WithParamInterface<bool> { | ||
| 105 | + public: | ||
| 106 | + HeadlessCreatedTargetIsUsedProcessTest() = default; | ||
| 107 | + | ||
| 108 | + bool IsHiddenTarget() const { return GetParam(); } | ||
| 109 | + | ||
| 110 | + private: | ||
| 111 | + void RunDevTooledTest() override { | ||
| 112 | + base::DictValue params; | ||
| 113 | + params.Set("url", ""); | ||
| 114 | + params.Set("hidden", IsHiddenTarget()); | ||
| 115 | + browser_devtools_client_.SendCommand( | ||
| 116 | + "Target.createTarget", std::move(params), | ||
| 117 | + base::BindOnce(&HeadlessCreatedTargetIsUsedProcessTest::OnTargetCreated, | ||
| 118 | + base::Unretained(this))); | ||
| 119 | + } | ||
| 120 | + | ||
| 121 | + void OnTargetCreated(base::DictValue result) { | ||
| 122 | + std::string target_id = DictString(result, "result.targetId"); | ||
| 123 | + ASSERT_FALSE(target_id.empty()); | ||
| 124 | + | ||
| 125 | + scoped_refptr<content::DevToolsAgentHost> agent_host = | ||
| 126 | + content::DevToolsAgentHost::GetForId(target_id); | ||
| 127 | + ASSERT_TRUE(agent_host); | ||
| 128 | + | ||
| 129 | + content::WebContents* web_contents = agent_host->GetWebContents(); | ||
| 130 | + ASSERT_TRUE(web_contents); | ||
| 131 | + | ||
| 132 | + bool is_used = | ||
| 133 | + !web_contents->GetPrimaryMainFrame()->GetProcess()->IsUnused(); | ||
| 134 | + EXPECT_EQ(is_used, IsHiddenTarget()); | ||
| 135 | + | ||
| 136 | + FinishAsynchronousTest(); | ||
| 137 | + } | ||
| 138 | +}; | ||
| 139 | + | ||
| 140 | +INSTANTIATE_TEST_SUITE_P( | ||
| 141 | + /* no prefix */, | ||
| 142 | + HeadlessCreatedTargetIsUsedProcessTest, | ||
| 143 | + testing::Bool(), | ||
| 144 | + [](const testing::TestParamInfo<bool>& info) { | ||
| 145 | + return info.param ? "hidden" : "normal"; | ||
| 146 | + }); | ||
| 147 | + | ||
| 148 | +HEADLESS_DEVTOOLED_TEST_P(HeadlessCreatedTargetIsUsedProcessTest); | ||
| 149 | + | ||
| 100 | } // namespace headless | 150 | } // namespace headless |
| @@ -1198,6 +1198,15 @@ bool H265Decoder::PerformDpbOperations(const H265SPS* sps) { | |||
| 1198 | return false; | 1198 | return false; |
| 1199 | } | 1199 | } |
| 1200 | 1200 | ||
| 1201 | + // Non-decodable RASL frames are not stored in the DPB because the picture | ||
| 1202 | + // is not actually decoded so it doesn't make sense to store it. | ||
| 1203 | + if (curr_pic_->no_rasl_output_flag_ && | ||
| 1204 | + (curr_pic_->nal_unit_type_ == H265NALU::RASL_N || | ||
| 1205 | + curr_pic_->nal_unit_type_ == H265NALU::RASL_R)) { | ||
| 1206 | + DVLOG(1) << "Skipping storing non-decodable RASL frame in DPB"; | ||
| 1207 | + return true; | ||
| 1208 | + } | ||
| 1209 | + | ||
| 1201 | // Put the current pic in the DPB. | 1210 | // Put the current pic in the DPB. |
| 1202 | dpb_.StorePicture(curr_pic_, H265Picture::kShortTermFoll); | 1211 | dpb_.StorePicture(curr_pic_, H265Picture::kShortTermFoll); |
| 1203 | return true; | 1212 | return true; |
| @@ -142,6 +142,7 @@ V4L2ImageProcessorBackend::V4L2ImageProcessorBackend( | |||
| 142 | base::SingleThreadTaskRunnerThreadMode::DEDICATED)) { | 142 | base::SingleThreadTaskRunnerThreadMode::DEDICATED)) { |
| 143 | DVLOGF(2); | 143 | DVLOGF(2); |
| 144 | DETACH_FROM_SEQUENCE(poll_sequence_checker_); | 144 | DETACH_FROM_SEQUENCE(poll_sequence_checker_); |
| 145 | + CHECK_EQ(input_memory_type_, V4L2_MEMORY_DMABUF); | ||
| 145 | DCHECK_NE(output_memory_type_, V4L2_MEMORY_USERPTR); | 146 | DCHECK_NE(output_memory_type_, V4L2_MEMORY_USERPTR); |
| 146 | 147 | ||
| 147 | VLOGF(2) << "V4L2ImageProcessorBackend constructed with input: " | 148 | VLOGF(2) << "V4L2ImageProcessorBackend constructed with input: " |
| @@ -218,14 +219,12 @@ namespace { | |||
| 218 | 219 | ||
| 219 | v4l2_memory InputStorageTypeToV4L2Memory(VideoFrame::StorageType storage_type) { | 220 | v4l2_memory InputStorageTypeToV4L2Memory(VideoFrame::StorageType storage_type) { |
| 220 | switch (storage_type) { | 221 | switch (storage_type) { |
| 221 | - case VideoFrame::STORAGE_OWNED_MEMORY: | ||
| 222 | - case VideoFrame::STORAGE_UNOWNED_MEMORY: | ||
| 223 | - case VideoFrame::STORAGE_SHMEM: | ||
| 224 | - return V4L2_MEMORY_USERPTR; | ||
| 225 | case VideoFrame::STORAGE_DMABUFS: | 222 | case VideoFrame::STORAGE_DMABUFS: |
| 226 | case VideoFrame::STORAGE_GPU_MEMORY_BUFFER: | 223 | case VideoFrame::STORAGE_GPU_MEMORY_BUFFER: |
| 227 | return V4L2_MEMORY_DMABUF; | 224 | return V4L2_MEMORY_DMABUF; |
| 228 | default: | 225 | default: |
| 226 | + VLOGF(2) << "Unsupported storage type:" | ||
| 227 | + << VideoFrame::StorageTypeToString(storage_type); | ||
| 229 | return static_cast<v4l2_memory>(0); | 228 | return static_cast<v4l2_memory>(0); |
| 230 | } | 229 | } |
| 231 | } | 230 | } |
| @@ -263,8 +262,7 @@ std::unique_ptr<ImageProcessorBackend> V4L2ImageProcessorBackend::Create( | |||
| 263 | 262 | ||
| 264 | const v4l2_memory input_memory_type = | 263 | const v4l2_memory input_memory_type = |
| 265 | InputStorageTypeToV4L2Memory(input_config.storage_type); | 264 | InputStorageTypeToV4L2Memory(input_config.storage_type); |
| 266 | - if (input_memory_type != V4L2_MEMORY_USERPTR && | 265 | + if (input_memory_type != V4L2_MEMORY_DMABUF) { |
| 267 | - input_memory_type != V4L2_MEMORY_DMABUF) { | ||
| 268 | VLOGF(2) << "Unsupported input storage type"; | 266 | VLOGF(2) << "Unsupported input storage type"; |
| 269 | return nullptr; | 267 | return nullptr; |
| 270 | } | 268 | } |
| @@ -879,25 +877,6 @@ bool V4L2ImageProcessorBackend::EnqueueInputRecord( | |||
| 879 | DCHECK(input_queue_); | 877 | DCHECK(input_queue_); |
| 880 | 878 | ||
| 881 | switch (input_memory_type_) { | 879 | switch (input_memory_type_) { |
| 882 | - case V4L2_MEMORY_USERPTR: { | ||
| 883 | - const size_t num_planes = | ||
| 884 | - GetNumPlanesOfV4L2PixFmt(input_config_.fourcc.ToV4L2PixFmt()); | ||
| 885 | - std::vector<void*> user_ptrs(num_planes); | ||
| 886 | - for (size_t i = 0; i < num_planes; ++i) { | ||
| 887 | - int bytes_used = | ||
| 888 | - VideoFrame::PlaneSize(job_record->input_frame->format(), i, | ||
| 889 | - input_config_.size) | ||
| 890 | - .GetArea(); | ||
| 891 | - buffer.SetPlaneBytesUsed(i, bytes_used); | ||
| 892 | - user_ptrs[i] = const_cast<uint8_t*>(job_record->input_frame->data(i)); | ||
| 893 | - } | ||
| 894 | - if (!std::move(buffer).QueueUserPtr(user_ptrs)) { | ||
| 895 | - VPLOGF(1) << "Failed to queue a DMABUF buffer to input queue"; | ||
| 896 | - NotifyError(); | ||
| 897 | - return false; | ||
| 898 | - } | ||
| 899 | - break; | ||
| 900 | - } | ||
| 901 | case V4L2_MEMORY_DMABUF: { | 880 | case V4L2_MEMORY_DMABUF: { |
| 902 | auto input_handle = CreateHandle(job_record->input_frame.get()); | 881 | auto input_handle = CreateHandle(job_record->input_frame.get()); |
| 903 | if (!input_handle) { | 882 | if (!input_handle) { |
| @@ -711,7 +711,7 @@ void VideoEncodeAcceleratorAdapter::FlushOnAcceleratorThread( | |||
| 711 | if (state_ == State::kFlushing && flush_support_.value()) { | 711 | if (state_ == State::kFlushing && flush_support_.value()) { |
| 712 | accelerator_->Flush( | 712 | accelerator_->Flush( |
| 713 | base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted, | 713 | base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted, |
| 714 | - base::Unretained(this))); | 714 | + weak_factory_.GetWeakPtr())); |
| 715 | } | 715 | } |
| 716 | } | 716 | } |
| 717 | 717 | ||
| @@ -989,7 +989,7 @@ void VideoEncodeAcceleratorAdapter::InitCompleted(EncoderStatus status) { | |||
| 989 | if (flush_support_.value()) { | 989 | if (flush_support_.value()) { |
| 990 | accelerator_->Flush( | 990 | accelerator_->Flush( |
| 991 | base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted, | 991 | base::BindOnce(&VideoEncodeAcceleratorAdapter::FlushCompleted, |
| 992 | - base::Unretained(this))); | 992 | + weak_factory_.GetWeakPtr())); |
| 993 | } | 993 | } |
| 994 | } | 994 | } |
| 995 | } | 995 | } |
| @@ -15,6 +15,7 @@ | |||
| 15 | 15 | ||
| 16 | 16 | ||
| 17 | 17 | ||
| 18 | + | ||
| 18 | 19 | ||
| 19 | 20 | ||
| 20 | 21 | ||
| @@ -199,6 +200,8 @@ class MEDIA_EXPORT VideoEncodeAcceleratorAdapter | |||
| 199 | VideoEncodeAccelerator::Config::EncoderType::kHardware; | 200 | VideoEncodeAccelerator::Config::EncoderType::kHardware; |
| 200 | bool supports_frame_size_change_ = false; | 201 | bool supports_frame_size_change_ = false; |
| 201 | bool supports_gpu_shared_images_ = false; | 202 | bool supports_gpu_shared_images_ = false; |
| 203 | + | ||
| 204 | + base::WeakPtrFactory<VideoEncodeAcceleratorAdapter> weak_factory_{this}; | ||
| 202 | }; | 205 | }; |
| 203 | 206 | ||
| 204 | } // namespace media | 207 | } // namespace media |
| @@ -11,12 +11,14 @@ | |||
| 11 | 11 | ||
| 12 | 12 | ||
| 13 | 13 | ||
| 14 | + | ||
| 14 | 15 | ||
| 15 | 16 | ||
| 16 | 17 | ||
| 17 | 18 | ||
| 18 | 19 | ||
| 19 | 20 | ||
| 21 | + | ||
| 20 | 22 | ||
| 21 | 23 | ||
| 22 | 24 | ||
| @@ -44,6 +46,30 @@ using ::testing::WithArgs; | |||
| 44 | 46 | ||
| 45 | namespace media { | 47 | namespace media { |
| 46 | 48 | ||
| 49 | +class FlushHoldingVideoEncodeAccelerator : public FakeVideoEncodeAccelerator { | ||
| 50 | + public: | ||
| 51 | + explicit FlushHoldingVideoEncodeAccelerator( | ||
| 52 | + scoped_refptr<base::SequencedTaskRunner> task_runner, | ||
| 53 | + FlushCallback* out_held_callback) | ||
| 54 | + : FakeVideoEncodeAccelerator(std::move(task_runner)), | ||
| 55 | + out_held_callback_(out_held_callback) {} | ||
| 56 | + | ||
| 57 | + void Flush(FlushCallback flush_callback) override { | ||
| 58 | + flush_callback_ = std::move(flush_callback); | ||
| 59 | + } | ||
| 60 | + | ||
| 61 | + bool IsFlushSupported() override { return true; } | ||
| 62 | + | ||
| 63 | + void Destroy() override { | ||
| 64 | + *out_held_callback_ = std::move(flush_callback_); | ||
| 65 | + FakeVideoEncodeAccelerator::Destroy(); | ||
| 66 | + } | ||
| 67 | + | ||
| 68 | + private: | ||
| 69 | + FlushCallback flush_callback_; | ||
| 70 | + raw_ptr<FlushCallback> out_held_callback_; | ||
| 71 | +}; | ||
| 72 | + | ||
| 47 | class VideoEncodeAcceleratorAdapterTest | 73 | class VideoEncodeAcceleratorAdapterTest |
| 48 | : public ::testing::TestWithParam<VideoPixelFormat> { | 74 | : public ::testing::TestWithParam<VideoPixelFormat> { |
| 49 | public: | 75 | public: |
| @@ -394,6 +420,98 @@ TEST_F(VideoEncodeAcceleratorAdapterTest, FlushDuringInitialize) { | |||
| 394 | RunUntilIdle(); | 420 | RunUntilIdle(); |
| 395 | } | 421 | } |
| 396 | 422 | ||
| 423 | +TEST_F(VideoEncodeAcceleratorAdapterTest, HeldFlushCallbackAfterDestroy) { | ||
| 424 | + // Delete the default vea_ allocated in SetUp since we are replacing it. | ||
| 425 | + delete vea_.ExtractAsDangling(); | ||
| 426 | + | ||
| 427 | + VideoEncodeAccelerator::FlushCallback held_flush_callback; | ||
| 428 | + auto* flush_holding_vea = | ||
| 429 | + new FlushHoldingVideoEncodeAccelerator(vea_runner_, &held_flush_callback); | ||
| 430 | + vea_ = flush_holding_vea; | ||
| 431 | + EXPECT_CALL(*gpu_factories_.get(), DoCreateVideoEncodeAccelerator()) | ||
| 432 | + .WillRepeatedly(Return(flush_holding_vea)); | ||
| 433 | + | ||
| 434 | + VideoEncoder::Options options; | ||
| 435 | + options.frame_size = gfx::Size(640, 480); | ||
| 436 | + | ||
| 437 | + // Wait 1: Initialize | ||
| 438 | + base::RunLoop init_run_loop; | ||
| 439 | + adapter()->Initialize( | ||
| 440 | + profile_, options, /*info_cb=*/base::DoNothing(), | ||
| 441 | + /*output_cb=*/base::DoNothing(), | ||
| 442 | + base::BindLambdaForTesting([&](EncoderStatus status) { | ||
| 443 | + EXPECT_TRUE(callback_runner_->RunsTasksInCurrentSequence()); | ||
| 444 | + EXPECT_TRUE(status.is_ok()); | ||
| 445 | + init_run_loop.Quit(); | ||
| 446 | + })); | ||
| 447 | + init_run_loop.Run(); | ||
| 448 | + | ||
| 449 | + auto frame = CreateGreenFrame(options.frame_size, PIXEL_FORMAT_I420, | ||
| 450 | + base::Milliseconds(1)); | ||
| 451 | + | ||
| 452 | + // Block vea_runner_ to ensure Encode and Flush are both posted before either | ||
| 453 | + // runs. | ||
| 454 | + base::WaitableEvent event; | ||
| 455 | + vea_runner_->PostTask( | ||
| 456 | + FROM_HERE, base::BindOnce( | ||
| 457 | + [](base::WaitableEvent* e) { | ||
| 458 | + base::ScopedAllowBaseSyncPrimitivesForTesting allow_wait; | ||
| 459 | + e->Wait(); | ||
| 460 | + }, | ||
| 461 | + base::Unretained(&event))); | ||
| 462 | + | ||
| 463 | + // Start Encode but don't wait yet | ||
| 464 | + base::RunLoop encode_run_loop; | ||
| 465 | + adapter()->Encode( | ||
| 466 | + frame, VideoEncoder::EncodeOptions(true), | ||
| 467 | + base::BindLambdaForTesting([&](EncoderStatus status) { | ||
| 468 | + EXPECT_TRUE(callback_runner_->RunsTasksInCurrentSequence()); | ||
| 469 | + EXPECT_TRUE(status.is_ok()); | ||
| 470 | + encode_run_loop.Quit(); | ||
| 471 | + })); | ||
| 472 | + | ||
| 473 | + bool flush_called = false; | ||
| 474 | + adapter()->Flush(base::BindLambdaForTesting([&](EncoderStatus status) { | ||
| 475 | + flush_called = true; | ||
| 476 | + EXPECT_TRUE(status.is_ok()); | ||
| 477 | + })); | ||
| 478 | + | ||
| 479 | + // Unblock vea_runner_ now that both are posted. | ||
| 480 | + event.Signal(); | ||
| 481 | + | ||
| 482 | + // Wait for vea_runner_ to process Encode and Flush, which will post | ||
| 483 | + // the Encode completion callback to the main thread. | ||
| 484 | + { | ||
| 485 | + base::RunLoop run_loop; | ||
| 486 | + vea_runner_->PostTaskAndReply(FROM_HERE, base::DoNothing(), | ||
| 487 | + run_loop.QuitClosure()); | ||
| 488 | + run_loop.Run(); | ||
| 489 | + } | ||
| 490 | + | ||
| 491 | + // Now run the main thread loop until the Encode callback executes. | ||
| 492 | + encode_run_loop.Run(); | ||
| 493 | + EXPECT_FALSE(flush_called); | ||
| 494 | + | ||
| 495 | + // Wait 4: Deletion | ||
| 496 | + vea_runner_->DeleteSoon(FROM_HERE, std::move(vae_adapter_)); | ||
| 497 | + { | ||
| 498 | + base::RunLoop run_loop; | ||
| 499 | + vea_runner_->PostTaskAndReply(FROM_HERE, base::DoNothing(), | ||
| 500 | + run_loop.QuitClosure()); | ||
| 501 | + run_loop.Run(); | ||
| 502 | + } | ||
| 503 | + | ||
| 504 | + // Wait 5: Running the held callback | ||
| 505 | + vea_runner_->PostTask(FROM_HERE, | ||
| 506 | + base::BindOnce(std::move(held_flush_callback), true)); | ||
| 507 | + { | ||
| 508 | + base::RunLoop run_loop; | ||
| 509 | + vea_runner_->PostTaskAndReply(FROM_HERE, base::DoNothing(), | ||
| 510 | + run_loop.QuitClosure()); | ||
| 511 | + run_loop.Run(); | ||
| 512 | + } | ||
| 513 | +} | ||
| 514 | + | ||
| 397 | TEST_F(VideoEncodeAcceleratorAdapterTest, InitializationError) { | 515 | TEST_F(VideoEncodeAcceleratorAdapterTest, InitializationError) { |
| 398 | VideoEncoder::Options options; | 516 | VideoEncoder::Options options; |
| 399 | options.frame_size = gfx::Size(640, 480); | 517 | options.frame_size = gfx::Size(640, 480); |
| @@ -107,10 +107,16 @@ void SpdyBuffer::ConsumeHelper(size_t consume_size, | |||
| 107 | DCHECK_GE(consume_size, 1u); | 107 | DCHECK_GE(consume_size, 1u); |
| 108 | DCHECK_LE(consume_size, GetRemainingSize()); | 108 | DCHECK_LE(consume_size, GetRemainingSize()); |
| 109 | offset_ += consume_size; | 109 | offset_ += consume_size; |
| 110 | - for (std::vector<ConsumeCallback>::const_iterator it = | 110 | + // Copy callbacks before iterating: a consume callback may cause `this` to be |
| 111 | - consume_callbacks_.begin(); it != consume_callbacks_.end(); ++it) { | 111 | + // destroyed reentrantly. Iterating a local copy keeps the iterator valid and |
| 112 | - it->Run(consume_size, consume_source); | 112 | + // keeps each callback's BindState alive (via RepeatingCallback's |
| 113 | + // scoped_refptr) even after `this` is freed. The callbacks themselves are | ||
| 114 | + // WeakPtr-bound and tolerate the receiver being gone. | ||
| 115 | + std::vector<ConsumeCallback> callbacks = consume_callbacks_; | ||
| 116 | + for (const auto& callback : callbacks) { | ||
| 117 | + callback.Run(consume_size, consume_source); | ||
| 113 | } | 118 | } |
| 119 | + // `this` may have been deleted here. | ||
| 114 | } | 120 | } |
| 115 | 121 | ||
| 116 | } // namespace net | 122 | } // namespace net |
| @@ -10,10 +10,12 @@ | |||
| 10 | 10 | ||
| 11 | 11 | ||
| 12 | 12 | ||
| 13 | + | ||
| 13 | 14 | ||
| 14 | 15 | ||
| 15 | 16 | ||
| 16 | 17 | ||
| 18 | + | ||
| 17 | 19 | ||
| 18 | 20 | ||
| 19 | 21 | ||
| @@ -138,4 +140,55 @@ TEST_F(SpdyReadQueueTest, Clear) { | |||
| 138 | EXPECT_TRUE(read_queue.IsEmpty()); | 140 | EXPECT_TRUE(read_queue.IsEmpty()); |
| 139 | } | 141 | } |
| 140 | 142 | ||
| 143 | +// Tests that calling Dequeue() reentrantly from within a consume callback | ||
| 144 | +// does not cause a use-after-free when the SpdyBuffer is destroyed during | ||
| 145 | +// the reentrant call. | ||
| 146 | +namespace { | ||
| 147 | + | ||
| 148 | +void ReentrantDequeue(SpdyReadQueue* queue, | ||
| 149 | + bool* fired, | ||
| 150 | + size_t inner_buf_len, | ||
| 151 | + size_t consume_size, | ||
| 152 | + SpdyBuffer::ConsumeSource consume_source) { | ||
| 153 | + if (*fired) { | ||
| 154 | + return; | ||
| 155 | + } | ||
| 156 | + *fired = true; | ||
| 157 | + | ||
| 158 | + std::vector<uint8_t> inner_buf(inner_buf_len); | ||
| 159 | + queue->Dequeue(inner_buf); | ||
| 160 | +} | ||
| 161 | + | ||
| 162 | +} // namespace | ||
| 163 | + | ||
| 164 | +TEST_F(SpdyReadQueueTest, ReentrantDequeue) { | ||
| 165 | + constexpr size_t kPayloadSize = 20; | ||
| 166 | + constexpr size_t kUserBufLen = 12; | ||
| 167 | + | ||
| 168 | + std::array<uint8_t, kPayloadSize> payload = {}; | ||
| 169 | + SpdyReadQueue queue; | ||
| 170 | + auto buffer = | ||
| 171 | + std::make_unique<SpdyBuffer>(base::span<const uint8_t>(payload)); | ||
| 172 | + | ||
| 173 | + bool reentry_fired = false; | ||
| 174 | + | ||
| 175 | + buffer->AddConsumeCallback(base::BindRepeating(&ReentrantDequeue, &queue, | ||
| 176 | + &reentry_fired, kUserBufLen)); | ||
| 177 | + // Add a second callback to ensure that the loop in ConsumeHelper continues | ||
| 178 | + // and attempts to access the next callback after the buffer has been deleted. | ||
| 179 | + int second_callback_called = 0; | ||
| 180 | + buffer->AddConsumeCallback(base::BindRepeating( | ||
| 181 | + [](int* counter, size_t, SpdyBuffer::ConsumeSource) { (*counter)++; }, | ||
| 182 | + &second_callback_called)); | ||
| 183 | + | ||
| 184 | + queue.Enqueue(std::move(buffer)); | ||
| 185 | + | ||
| 186 | + std::array<uint8_t, kUserBufLen> user_buf; | ||
| 187 | + size_t copied = queue.Dequeue(base::span<uint8_t>(user_buf)); | ||
| 188 | + | ||
| 189 | + EXPECT_EQ(copied, kUserBufLen); | ||
| 190 | + EXPECT_TRUE(reentry_fired); | ||
| 191 | + EXPECT_EQ(second_callback_called, 2); | ||
| 192 | +} | ||
| 193 | + | ||
| 141 | } // namespace net::test | 194 | } // namespace net::test |
| @@ -610,6 +610,8 @@ PDFiumEngine::PDFiumEngine(PDFiumEngineClient* client, | |||
| 610 | } | 610 | } |
| 611 | 611 | ||
| 612 | PDFiumEngine::~PDFiumEngine() { | 612 | PDFiumEngine::~PDFiumEngine() { |
| 613 | + in_dtor_ = true; | ||
| 614 | + | ||
| 613 | if (!client_->IsPrintPreview()) { | 615 | if (!client_->IsPrintPreview()) { |
| 614 | base::UmaHistogramLongTimes("PDF.EngineLifetime", | 616 | base::UmaHistogramLongTimes("PDF.EngineLifetime", |
| 615 | base::TimeTicks::Now() - engine_creation_time_); | 617 | base::TimeTicks::Now() - engine_creation_time_); |
| @@ -1327,6 +1327,8 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE { | |||
| 1327 | std::map<InkModeledShapeId, FPDF_PAGEOBJECT> ink_modeled_shape_map_; | 1327 | std::map<InkModeledShapeId, FPDF_PAGEOBJECT> ink_modeled_shape_map_; |
| 1328 | 1328 | ||
| 1329 | 1329 | ||
| 1330 | + bool in_dtor_ = false; | ||
| 1331 | + | ||
| 1330 | base::WeakPtrFactory<PDFiumEngine> weak_factory_{this}; | 1332 | base::WeakPtrFactory<PDFiumEngine> weak_factory_{this}; |
| 1331 | 1333 | ||
| 1332 | // Weak pointers from this factory are used to bind the ContinueFind() | 1334 | // Weak pointers from this factory are used to bind the ContinueFind() |
| @@ -146,6 +146,10 @@ void PDFiumFormFiller::Form_Invalidate(FPDF_FORMFILLINFO* param, | |||
| 146 | double bottom) { | 146 | double bottom) { |
| 147 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 147 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 148 | PDFiumEngine* engine = engine_scope.engine(); | 148 | PDFiumEngine* engine = engine_scope.engine(); |
| 149 | + if (engine->in_dtor_) { | ||
| 150 | + return; | ||
| 151 | + } | ||
| 152 | + | ||
| 149 | int page_index = engine->GetVisiblePageIndex(page); | 153 | int page_index = engine->GetVisiblePageIndex(page); |
| 150 | if (page_index == -1) { | 154 | if (page_index == -1) { |
| 151 | // This can sometime happen when the page is closed because it went off | 155 | // This can sometime happen when the page is closed because it went off |
| @@ -168,6 +172,10 @@ void PDFiumFormFiller::Form_OutputSelectedRect(FPDF_FORMFILLINFO* param, | |||
| 168 | double bottom) { | 172 | double bottom) { |
| 169 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 173 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 170 | PDFiumEngine* engine = engine_scope.engine(); | 174 | PDFiumEngine* engine = engine_scope.engine(); |
| 175 | + if (engine->in_dtor_) { | ||
| 176 | + return; | ||
| 177 | + } | ||
| 178 | + | ||
| 171 | int page_index = engine->GetVisiblePageIndex(page); | 179 | int page_index = engine->GetVisiblePageIndex(page); |
| 172 | if (page_index == -1) | 180 | if (page_index == -1) |
| 173 | return; | 181 | return; |
| @@ -222,6 +230,10 @@ FPDF_SYSTEMTIME PDFiumFormFiller::Form_GetLocalTime(FPDF_FORMFILLINFO* param) { | |||
| 222 | void PDFiumFormFiller::Form_OnChange(FPDF_FORMFILLINFO* param) { | 230 | void PDFiumFormFiller::Form_OnChange(FPDF_FORMFILLINFO* param) { |
| 223 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 231 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 224 | PDFiumEngine* engine = engine_scope.engine(); | 232 | PDFiumEngine* engine = engine_scope.engine(); |
| 233 | + if (engine->in_dtor_) { | ||
| 234 | + return; | ||
| 235 | + } | ||
| 236 | + | ||
| 225 | engine->EnteredEditMode(); | 237 | engine->EnteredEditMode(); |
| 226 | } | 238 | } |
| 227 | 239 | ||
| @@ -231,6 +243,10 @@ FPDF_PAGE PDFiumFormFiller::Form_GetPage(FPDF_FORMFILLINFO* param, | |||
| 231 | int page_index) { | 243 | int page_index) { |
| 232 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 244 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 233 | PDFiumEngine* engine = engine_scope.engine(); | 245 | PDFiumEngine* engine = engine_scope.engine(); |
| 246 | + if (engine->in_dtor_) { | ||
| 247 | + return nullptr; | ||
| 248 | + } | ||
| 249 | + | ||
| 234 | if (!engine->PageIndexInBounds(page_index)) | 250 | if (!engine->PageIndexInBounds(page_index)) |
| 235 | return nullptr; | 251 | return nullptr; |
| 236 | return engine->pages_[page_index]->GetPage(); | 252 | return engine->pages_[page_index]->GetPage(); |
| @@ -241,6 +257,10 @@ FPDF_PAGE PDFiumFormFiller::Form_GetCurrentPage(FPDF_FORMFILLINFO* param, | |||
| 241 | FPDF_DOCUMENT document) { | 257 | FPDF_DOCUMENT document) { |
| 242 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 258 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 243 | PDFiumEngine* engine = engine_scope.engine(); | 259 | PDFiumEngine* engine = engine_scope.engine(); |
| 260 | + if (engine->in_dtor_) { | ||
| 261 | + return nullptr; | ||
| 262 | + } | ||
| 263 | + | ||
| 244 | int index = engine->last_focused_page_; | 264 | int index = engine->last_focused_page_; |
| 245 | if (index == -1) { | 265 | if (index == -1) { |
| 246 | index = engine->GetMostVisiblePage(); | 266 | index = engine->GetMostVisiblePage(); |
| @@ -263,6 +283,10 @@ void PDFiumFormFiller::Form_ExecuteNamedAction(FPDF_FORMFILLINFO* param, | |||
| 263 | FPDF_BYTESTRING named_action) { | 283 | FPDF_BYTESTRING named_action) { |
| 264 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 284 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 265 | PDFiumEngine* engine = engine_scope.engine(); | 285 | PDFiumEngine* engine = engine_scope.engine(); |
| 286 | + if (engine->in_dtor_) { | ||
| 287 | + return; | ||
| 288 | + } | ||
| 289 | + | ||
| 266 | std::string action(named_action); | 290 | std::string action(named_action); |
| 267 | if (action == "Print") { | 291 | if (action == "Print") { |
| 268 | engine->client_->Print(); | 292 | engine->client_->Print(); |
| @@ -311,6 +335,10 @@ void PDFiumFormFiller::Form_OnFocusChange(FPDF_FORMFILLINFO* param, | |||
| 311 | int page_index) { | 335 | int page_index) { |
| 312 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 336 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 313 | PDFiumEngine* engine = engine_scope.engine(); | 337 | PDFiumEngine* engine = engine_scope.engine(); |
| 338 | + if (engine->in_dtor_) { | ||
| 339 | + return; | ||
| 340 | + } | ||
| 341 | + | ||
| 314 | if (!engine->PageIndexInBounds(page_index)) | 342 | if (!engine->PageIndexInBounds(page_index)) |
| 315 | return; | 343 | return; |
| 316 | 344 | ||
| @@ -330,6 +358,10 @@ void PDFiumFormFiller::Form_DoURIAction(FPDF_FORMFILLINFO* param, | |||
| 330 | FPDF_BYTESTRING uri) { | 358 | FPDF_BYTESTRING uri) { |
| 331 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 359 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 332 | PDFiumEngine* engine = engine_scope.engine(); | 360 | PDFiumEngine* engine = engine_scope.engine(); |
| 361 | + if (engine->in_dtor_) { | ||
| 362 | + return; | ||
| 363 | + } | ||
| 364 | + | ||
| 333 | engine->client_->NavigateTo(std::string(uri), | 365 | engine->client_->NavigateTo(std::string(uri), |
| 334 | WindowOpenDisposition::CURRENT_TAB); | 366 | WindowOpenDisposition::CURRENT_TAB); |
| 335 | } | 367 | } |
| @@ -342,6 +374,10 @@ void PDFiumFormFiller::Form_DoGoToAction(FPDF_FORMFILLINFO* param, | |||
| 342 | int size_of_array) { | 374 | int size_of_array) { |
| 343 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 375 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 344 | PDFiumEngine* engine = engine_scope.engine(); | 376 | PDFiumEngine* engine = engine_scope.engine(); |
| 377 | + if (engine->in_dtor_) { | ||
| 378 | + return; | ||
| 379 | + } | ||
| 380 | + | ||
| 345 | engine->ScrollToPage(page_index); | 381 | engine->ScrollToPage(page_index); |
| 346 | } | 382 | } |
| 347 | 383 | ||
| @@ -352,6 +388,10 @@ void PDFiumFormFiller::Form_DoURIActionWithKeyboardModifier( | |||
| 352 | int modifiers) { | 388 | int modifiers) { |
| 353 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 389 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 354 | PDFiumEngine* engine = engine_scope.engine(); | 390 | PDFiumEngine* engine = engine_scope.engine(); |
| 391 | + if (engine->in_dtor_) { | ||
| 392 | + return; | ||
| 393 | + } | ||
| 394 | + | ||
| 355 | bool middle_button = | 395 | bool middle_button = |
| 356 | !!(modifiers & blink::WebInputEvent::Modifiers::kMiddleButtonDown); | 396 | !!(modifiers & blink::WebInputEvent::Modifiers::kMiddleButtonDown); |
| 357 | bool alt_key = !!(modifiers & blink::WebInputEvent::Modifiers::kAltKey); | 397 | bool alt_key = !!(modifiers & blink::WebInputEvent::Modifiers::kAltKey); |
| @@ -384,6 +424,10 @@ void PDFiumFormFiller::Form_EmailTo(FPDF_FORMFILLINFO* param, | |||
| 384 | 424 | ||
| 385 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 425 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 386 | PDFiumEngine* engine = engine_scope.engine(); | 426 | PDFiumEngine* engine = engine_scope.engine(); |
| 427 | + if (engine->in_dtor_) { | ||
| 428 | + return; | ||
| 429 | + } | ||
| 430 | + | ||
| 387 | engine->client_->Email(to_str, cc_str, bcc_str, subject_str, message_str); | 431 | engine->client_->Email(to_str, cc_str, bcc_str, subject_str, message_str); |
| 388 | } | 432 | } |
| 389 | 433 | ||
| @@ -402,6 +446,10 @@ void PDFiumFormFiller::Form_SetCurrentPage(FPDF_FORMFILLINFO* param, | |||
| 402 | int page) { | 446 | int page) { |
| 403 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 447 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 404 | PDFiumEngine* engine = engine_scope.engine(); | 448 | PDFiumEngine* engine = engine_scope.engine(); |
| 449 | + if (engine->in_dtor_) { | ||
| 450 | + return; | ||
| 451 | + } | ||
| 452 | + | ||
| 405 | engine->ScrollToPage(page); | 453 | engine->ScrollToPage(page); |
| 406 | } | 454 | } |
| 407 | 455 | ||
| @@ -410,6 +458,10 @@ int PDFiumFormFiller::Form_GetCurrentPageIndex(FPDF_FORMFILLINFO* param, | |||
| 410 | FPDF_DOCUMENT document) { | 458 | FPDF_DOCUMENT document) { |
| 411 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 459 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 412 | PDFiumEngine* engine = engine_scope.engine(); | 460 | PDFiumEngine* engine = engine_scope.engine(); |
| 461 | + if (engine->in_dtor_) { | ||
| 462 | + return -1; | ||
| 463 | + } | ||
| 464 | + | ||
| 413 | return engine->GetMostVisiblePage(); | 465 | return engine->GetMostVisiblePage(); |
| 414 | } | 466 | } |
| 415 | 467 | ||
| @@ -422,6 +474,10 @@ void PDFiumFormFiller::Form_GetPageViewRect(FPDF_FORMFILLINFO* param, | |||
| 422 | double* bottom) { | 474 | double* bottom) { |
| 423 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 475 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 424 | PDFiumEngine* engine = engine_scope.engine(); | 476 | PDFiumEngine* engine = engine_scope.engine(); |
| 477 | + if (engine->in_dtor_) { | ||
| 478 | + return; | ||
| 479 | + } | ||
| 480 | + | ||
| 425 | int page_index = engine->GetVisiblePageIndex(page); | 481 | int page_index = engine->GetVisiblePageIndex(page); |
| 426 | if (!engine->PageIndexInBounds(page_index)) { | 482 | if (!engine->PageIndexInBounds(page_index)) { |
| 427 | *left = 0; | 483 | *left = 0; |
| @@ -502,6 +558,10 @@ void PDFiumFormFiller::Form_PageEvent(FPDF_FORMFILLINFO* param, | |||
| 502 | 558 | ||
| 503 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 559 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 504 | PDFiumEngine* engine = engine_scope.engine(); | 560 | PDFiumEngine* engine = engine_scope.engine(); |
| 561 | + if (engine->in_dtor_) { | ||
| 562 | + return; | ||
| 563 | + } | ||
| 564 | + | ||
| 505 | engine->UpdatePageCount(); | 565 | engine->UpdatePageCount(); |
| 506 | } | 566 | } |
| 507 | 567 | ||
| @@ -608,6 +668,10 @@ int PDFiumFormFiller::Form_Alert(IPDF_JSPLATFORM* param, | |||
| 608 | 668 | ||
| 609 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 669 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 610 | PDFiumEngine* engine = engine_scope.engine(); | 670 | PDFiumEngine* engine = engine_scope.engine(); |
| 671 | + if (engine->in_dtor_) { | ||
| 672 | + return ALERT_RESULT_CANCEL; | ||
| 673 | + } | ||
| 674 | + | ||
| 611 | std::string message_str = WideStringToString(message); | 675 | std::string message_str = WideStringToString(message); |
| 612 | if (type == ALERT_TYPE_OK) { | 676 | if (type == ALERT_TYPE_OK) { |
| 613 | engine->client_->Alert(message_str); | 677 | engine->client_->Alert(message_str); |
| @@ -624,6 +688,10 @@ int PDFiumFormFiller::Form_Alert(IPDF_JSPLATFORM* param, | |||
| 624 | void PDFiumFormFiller::Form_Beep(IPDF_JSPLATFORM* param, int type) { | 688 | void PDFiumFormFiller::Form_Beep(IPDF_JSPLATFORM* param, int type) { |
| 625 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 689 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 626 | PDFiumEngine* engine = engine_scope.engine(); | 690 | PDFiumEngine* engine = engine_scope.engine(); |
| 691 | + if (engine->in_dtor_) { | ||
| 692 | + return; | ||
| 693 | + } | ||
| 694 | + | ||
| 627 | engine->client_->Beep(); | 695 | engine->client_->Beep(); |
| 628 | } | 696 | } |
| 629 | 697 | ||
| @@ -641,6 +709,10 @@ int PDFiumFormFiller::Form_Response(IPDF_JSPLATFORM* param, | |||
| 641 | 709 | ||
| 642 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 710 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 643 | PDFiumEngine* engine = engine_scope.engine(); | 711 | PDFiumEngine* engine = engine_scope.engine(); |
| 712 | + if (engine->in_dtor_) { | ||
| 713 | + return 0; | ||
| 714 | + } | ||
| 715 | + | ||
| 644 | std::string rv = engine->client_->Prompt(question_str, default_str); | 716 | std::string rv = engine->client_->Prompt(question_str, default_str); |
| 645 | std::u16string rv_16 = base::UTF8ToUTF16(rv); | 717 | std::u16string rv_16 = base::UTF8ToUTF16(rv); |
| 646 | int rv_bytes = rv_16.size() * sizeof(char16_t); | 718 | int rv_bytes = rv_16.size() * sizeof(char16_t); |
| @@ -657,6 +729,10 @@ int PDFiumFormFiller::Form_GetFilePath(IPDF_JSPLATFORM* param, | |||
| 657 | int length) { | 729 | int length) { |
| 658 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 730 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 659 | PDFiumEngine* engine = engine_scope.engine(); | 731 | PDFiumEngine* engine = engine_scope.engine(); |
| 732 | + if (engine->in_dtor_) { | ||
| 733 | + return 0; | ||
| 734 | + } | ||
| 735 | + | ||
| 660 | std::string rv = engine->client_->GetURL(); | 736 | std::string rv = engine->client_->GetURL(); |
| 661 | 737 | ||
| 662 | // Account for the trailing null. | 738 | // Account for the trailing null. |
| @@ -686,6 +762,10 @@ void PDFiumFormFiller::Form_Mail(IPDF_JSPLATFORM* param, | |||
| 686 | 762 | ||
| 687 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 763 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 688 | PDFiumEngine* engine = engine_scope.engine(); | 764 | PDFiumEngine* engine = engine_scope.engine(); |
| 765 | + if (engine->in_dtor_) { | ||
| 766 | + return; | ||
| 767 | + } | ||
| 768 | + | ||
| 689 | engine->client_->Email(to_str, cc_str, bcc_str, subject_str, message_str); | 769 | engine->client_->Email(to_str, cc_str, bcc_str, subject_str, message_str); |
| 690 | } | 770 | } |
| 691 | 771 | ||
| @@ -703,6 +783,10 @@ void PDFiumFormFiller::Form_Print(IPDF_JSPLATFORM* param, | |||
| 703 | // Just opening it is fine for now. | 783 | // Just opening it is fine for now. |
| 704 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 784 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 705 | PDFiumEngine* engine = engine_scope.engine(); | 785 | PDFiumEngine* engine = engine_scope.engine(); |
| 786 | + if (engine->in_dtor_) { | ||
| 787 | + return; | ||
| 788 | + } | ||
| 789 | + | ||
| 706 | engine->client_->Print(); | 790 | engine->client_->Print(); |
| 707 | } | 791 | } |
| 708 | 792 | ||
| @@ -714,6 +798,10 @@ void PDFiumFormFiller::Form_SubmitForm(IPDF_JSPLATFORM* param, | |||
| 714 | std::string url_str = WideStringToString(url); | 798 | std::string url_str = WideStringToString(url); |
| 715 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 799 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 716 | PDFiumEngine* engine = engine_scope.engine(); | 800 | PDFiumEngine* engine = engine_scope.engine(); |
| 801 | + if (engine->in_dtor_) { | ||
| 802 | + return; | ||
| 803 | + } | ||
| 804 | + | ||
| 717 | engine->client_->SubmitForm(url_str, form_data, length); | 805 | engine->client_->SubmitForm(url_str, form_data, length); |
| 718 | } | 806 | } |
| 719 | 807 | ||
| @@ -721,6 +809,10 @@ void PDFiumFormFiller::Form_SubmitForm(IPDF_JSPLATFORM* param, | |||
| 721 | void PDFiumFormFiller::Form_GotoPage(IPDF_JSPLATFORM* param, int page_number) { | 809 | void PDFiumFormFiller::Form_GotoPage(IPDF_JSPLATFORM* param, int page_number) { |
| 722 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); | 810 | EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param); |
| 723 | PDFiumEngine* engine = engine_scope.engine(); | 811 | PDFiumEngine* engine = engine_scope.engine(); |
| 812 | + if (engine->in_dtor_) { | ||
| 813 | + return; | ||
| 814 | + } | ||
| 815 | + | ||
| 724 | engine->ScrollToPage(page_number); | 816 | engine->ScrollToPage(page_number); |
| 725 | } | 817 | } |
| 726 | 818 | ||
| @@ -165,6 +165,58 @@ TEST_F(CorsURLLoaderTest, ForbiddenMethods) { | |||
| 165 | } | 165 | } |
| 166 | } | 166 | } |
| 167 | 167 | ||
| 168 | +TEST_F(CorsURLLoaderTest, ForbiddenMethodOverride) { | ||
| 169 | + const struct { | ||
| 170 | + std::string header_name; | ||
| 171 | + std::string header_value; | ||
| 172 | + } kTestCases[] = { | ||
| 173 | + {"X-HTTP-Method-Override", "TRACE"}, | ||
| 174 | + {"X-HTTP-Method-Override", "TRACK"}, | ||
| 175 | + {"X-HTTP-Method-Override", "CONNECT"}, | ||
| 176 | + {"X-HTTP-Method", "TRACE"}, | ||
| 177 | + {"X-Method-Override", "TRACE"}, | ||
| 178 | + }; | ||
| 179 | + for (const auto& test_case : kTestCases) { | ||
| 180 | + SCOPED_TRACE(test_case.header_name); | ||
| 181 | + SCOPED_TRACE(test_case.header_value); | ||
| 182 | + for (const mojom::RequestMode mode : | ||
| 183 | + {mojom::RequestMode::kSameOrigin, mojom::RequestMode::kNoCors, | ||
| 184 | + mojom::RequestMode::kCors, | ||
| 185 | + mojom::RequestMode::kCorsWithForcedPreflight, | ||
| 186 | + mojom::RequestMode::kNavigate}) { | ||
| 187 | + SCOPED_TRACE(mode); | ||
| 188 | + | ||
| 189 | + ResetFactory( | ||
| 190 | + url::Origin::Create(GURL("https://example.com")) /* initiator */, | ||
| 191 | + OriginatingProcessId::browser()); | ||
| 192 | + | ||
| 193 | + ResourceRequest request; | ||
| 194 | + request.mode = mode; | ||
| 195 | + request.credentials_mode = mojom::CredentialsMode::kInclude; | ||
| 196 | + request.url = GURL("https://example.com/"); | ||
| 197 | + request.request_initiator = url::Origin::Create(request.url); | ||
| 198 | + request.method = "POST"; | ||
| 199 | + request.headers.SetHeader(test_case.header_name, test_case.header_value); | ||
| 200 | + | ||
| 201 | + BadMessageTestHelper bad_message_helper; | ||
| 202 | + CreateLoaderAndStart(request); | ||
| 203 | + if (IsNetworkLoaderStarted()) { | ||
| 204 | + RunUntilCreateLoaderAndStartCalled(); | ||
| 205 | + NotifyLoaderClientOnReceiveResponse(); | ||
| 206 | + NotifyLoaderClientOnComplete(net::OK); | ||
| 207 | + } | ||
| 208 | + RunUntilComplete(); | ||
| 209 | + | ||
| 210 | + EXPECT_FALSE(IsNetworkLoaderStarted()); | ||
| 211 | + EXPECT_FALSE(client().has_received_redirect()); | ||
| 212 | + EXPECT_FALSE(client().has_received_response()); | ||
| 213 | + EXPECT_TRUE(client().has_received_completion()); | ||
| 214 | + EXPECT_THAT(client().completion_status().error_code, | ||
| 215 | + net::test::IsError(net::ERR_INVALID_ARGUMENT)); | ||
| 216 | + } | ||
| 217 | + } | ||
| 218 | +} | ||
| 219 | + | ||
| 168 | TEST_F(CorsURLLoaderTest, SameOriginWithoutInitiator) { | 220 | TEST_F(CorsURLLoaderTest, SameOriginWithoutInitiator) { |
| 169 | ResourceRequest request; | 221 | ResourceRequest request; |
| 170 | request.mode = mojom::RequestMode::kSameOrigin; | 222 | request.mode = mojom::RequestMode::kSameOrigin; |
| @@ -39,6 +39,7 @@ | |||
| 39 | 39 | ||
| 40 | 40 | ||
| 41 | 41 | ||
| 42 | + | ||
| 42 | 43 | ||
| 43 | 44 | ||
| 44 | 45 | ||
| @@ -1323,4 +1324,23 @@ TEST_F(P2PSocketUdpWithInterceptorTest, ReceivePacketDelayed) { | |||
| 1323 | EXPECT_EQ(2U, received_packets_.size()); | 1324 | EXPECT_EQ(2U, received_packets_.size()); |
| 1324 | } | 1325 | } |
| 1325 | 1326 | ||
| 1327 | +TEST_F(P2PSocketUdpWithInterceptorTest, ReentrantDestructionSend) { | ||
| 1328 | + // Enable throttling. | ||
| 1329 | + SetNetworkState({.latency = base::Milliseconds(100)}); | ||
| 1330 | + | ||
| 1331 | + // Send a packet, which starts the retry timer. | ||
| 1332 | + webrtc::AsyncSocketPacketOptions options; | ||
| 1333 | + std::vector<uint8_t> packet; | ||
| 1334 | + CreateRandomPacket(&packet); | ||
| 1335 | + socket_impl_->Send(packet, P2PPacketInfo(dest1_, options, 0)); | ||
| 1336 | + | ||
| 1337 | + // Mark the socket for destruction. | ||
| 1338 | + socket_delegate_.ExpectDestruction(std::move(socket_impl_)); | ||
| 1339 | + socket_ = nullptr; | ||
| 1340 | + | ||
| 1341 | + // Advance clock past the retry delay, which will call OnSendNetworkTimer(), | ||
| 1342 | + // DoSend() and DestroySocket() synchronously. | ||
| 1343 | + AdvanceClock(base::Milliseconds(100)); | ||
| 1344 | +} | ||
| 1345 | + | ||
| 1326 | } // namespace network | 1346 | } // namespace network |
| @@ -483,7 +483,7 @@ std::vector<std::string> PrivilegedNoCorsHeaderNames() { | |||
| 483 | return {"range"}; | 483 | return {"range"}; |
| 484 | } | 484 | } |
| 485 | 485 | ||
| 486 | -bool IsForbiddenMethod(const std::string& method) { | 486 | +bool IsForbiddenMethod(std::string_view method) { |
| 487 | const std::string upper_method = base::ToUpperASCII(method); | 487 | const std::string upper_method = base::ToUpperASCII(method); |
| 488 | return upper_method == net::HttpRequestHeaders::kConnectMethod || | 488 | return upper_method == net::HttpRequestHeaders::kConnectMethod || |
| 489 | upper_method == net::HttpRequestHeaders::kTraceMethod || | 489 | upper_method == net::HttpRequestHeaders::kTraceMethod || |
| @@ -132,7 +132,7 @@ std::vector<std::string> PrivilegedNoCorsHeaderNames(); | |||
| 132 | 132 | ||
| 133 | // Checks forbidden method in the fetch spec. | 133 | // Checks forbidden method in the fetch spec. |
| 134 | // See https://fetch.spec.whatwg.org/#forbidden-method. | 134 | // See https://fetch.spec.whatwg.org/#forbidden-method. |
| 135 | -COMPONENT_EXPORT(NETWORK_CPP) bool IsForbiddenMethod(const std::string& name); | 135 | +COMPONENT_EXPORT(NETWORK_CPP) bool IsForbiddenMethod(std::string_view name); |
| 136 | 136 | ||
| 137 | // Returns true if |type| is a response type which makes a response | 137 | // Returns true if |type| is a response type which makes a response |
| 138 | // CORS-same-origin. See https://html.spec.whatwg.org/C/#cors-same-origin. | 138 | // CORS-same-origin. See https://html.spec.whatwg.org/C/#cors-same-origin. |
| @@ -14,6 +14,8 @@ | |||
| 14 | 14 | ||
| 15 | 15 | ||
| 16 | 16 | ||
| 17 | + | ||
| 18 | + | ||
| 17 | 19 | ||
| 18 | 20 | ||
| 19 | 21 | ||
| @@ -91,6 +93,17 @@ bool IsRequestHeaderSafe(std::string_view key, std::string_view value) { | |||
| 91 | if (base::StartsWith(key, "Proxy-", base::CompareCase::INSENSITIVE_ASCII)) | 93 | if (base::StartsWith(key, "Proxy-", base::CompareCase::INSENSITIVE_ASCII)) |
| 92 | return false; | 94 | return false; |
| 93 | 95 | ||
| 96 | + if (base::EqualsCaseInsensitiveASCII(key, "X-HTTP-Method") || | ||
| 97 | + base::EqualsCaseInsensitiveASCII(key, "X-HTTP-Method-Override") || | ||
| 98 | + base::EqualsCaseInsensitiveASCII(key, "X-Method-Override")) { | ||
| 99 | + net::HttpUtil::ValuesIterator method_iterator(value, ','); | ||
| 100 | + while (method_iterator.GetNext()) { | ||
| 101 | + if (cors::IsForbiddenMethod(method_iterator.value())) { | ||
| 102 | + return false; | ||
| 103 | + } | ||
| 104 | + } | ||
| 105 | + } | ||
| 106 | + | ||
| 94 | return true; | 107 | return true; |
| 95 | } | 108 | } |
| 96 | 109 | ||
| @@ -22,6 +22,11 @@ class URLResponseHead; | |||
| 22 | } // namespace mojom | 22 | } // namespace mojom |
| 23 | 23 | ||
| 24 | // Checks if a single request header is safe to send. | 24 | // Checks if a single request header is safe to send. |
| 25 | +// | ||
| 26 | +// Per https://fetch.spec.whatwg.org/#forbidden-request-header, the method- | ||
| 27 | +// override headers are forbidden when their value parses to a forbidden | ||
| 28 | +// method. The logic is almost compatible but exclude some headers that would | ||
| 29 | +// be set by renderer's internal code. | ||
| 25 | COMPONENT_EXPORT(NETWORK_CPP) | 30 | COMPONENT_EXPORT(NETWORK_CPP) |
| 26 | bool IsRequestHeaderSafe(std::string_view key, std::string_view value); | 31 | bool IsRequestHeaderSafe(std::string_view key, std::string_view value); |
| 27 | 32 | ||
| @@ -38,6 +38,13 @@ TEST(HeaderUtilTest, IsRequestHeaderSafe) { | |||
| 38 | {"Proxy-Foo", "bar", false}, | 38 | {"Proxy-Foo", "bar", false}, |
| 39 | {"PrOxY-FoO", "bar", false}, | 39 | {"PrOxY-FoO", "bar", false}, |
| 40 | 40 | ||
| 41 | + {"X-HTTP-Method-Override", "TRACE", false}, | ||
| 42 | + {"x-http-method-override", "trAcE", false}, | ||
| 43 | + {"X-HTTP-Method-Override", "GET", true}, | ||
| 44 | + {"X-HTTP-Method-Override", "GET, TRACE", false}, | ||
| 45 | + {"X-HTTP-Method", "TRACK", false}, | ||
| 46 | + {"X-Method-Override", "CONNECT", false}, | ||
| 47 | + | ||
| 41 | {"dnt", "1", true}, | 48 | {"dnt", "1", true}, |
| 42 | }; | 49 | }; |
| 43 | 50 | ||
| @@ -79,6 +86,13 @@ TEST(HeaderUtilTest, AreRequestHeadersSafe) { | |||
| 79 | {"Proxy-Foo", "bar", false}, | 86 | {"Proxy-Foo", "bar", false}, |
| 80 | {"PrOxY-FoO", "bar", false}, | 87 | {"PrOxY-FoO", "bar", false}, |
| 81 | 88 | ||
| 89 | + {"X-HTTP-Method-Override", "TRACE", false}, | ||
| 90 | + {"x-http-method-override", "trAcE", false}, | ||
| 91 | + {"X-HTTP-Method-Override", "GET", true}, | ||
| 92 | + {"X-HTTP-Method-Override", "GET, TRACE", false}, | ||
| 93 | + {"X-HTTP-Method", "TRACK", false}, | ||
| 94 | + {"X-Method-Override", "CONNECT", false}, | ||
| 95 | + | ||
| 82 | {"dnt", "1", true}, | 96 | {"dnt", "1", true}, |
| 83 | }; | 97 | }; |
| 84 | 98 | ||
| @@ -1034,28 +1034,28 @@ class SaveToFileBodyHandler : public BodyHandler { | |||
| 1034 | DCHECK(!file_.IsValid()); | 1034 | DCHECK(!file_.IsValid()); |
| 1035 | DCHECK(!body_reader_); | 1035 | DCHECK(!body_reader_); |
| 1036 | 1036 | ||
| 1037 | - bool have_path = !create_temp_file_; | 1037 | + if (create_temp_file_) { |
| 1038 | - if (!have_path) { | 1038 | + base::FilePath temp_dir; |
| 1039 | - DCHECK(create_temp_file_); | 1039 | + if (base::GetTempDir(&temp_dir)) { |
| 1040 | - have_path = base::CreateTemporaryFile(&path_); | 1040 | + file_ = base::CreateAndOpenTemporaryFileInDir(temp_dir, &path_); |
| 1041 | - // CreateTemporaryFile() creates an empty file. | 1041 | + } |
| 1042 | - if (have_path) | 1042 | + } else { |
| 1043 | - owns_file_ = true; | ||
| 1044 | - } | ||
| 1045 | - | ||
| 1046 | - if (have_path) { | ||
| 1047 | - // Try to initialize |file_|, creating the file if needed. | ||
| 1048 | file_.Initialize( | 1043 | file_.Initialize( |
| 1049 | path_, base::File::FLAG_WRITE | base::File::FLAG_CREATE_ALWAYS); | 1044 | path_, base::File::FLAG_WRITE | base::File::FLAG_CREATE_ALWAYS); |
| 1050 | } | 1045 | } |
| 1051 | 1046 | ||
| 1052 | // If CreateTemporaryFile() or File::Initialize() failed, report failure. | 1047 | // If CreateTemporaryFile() or File::Initialize() failed, report failure. |
| 1053 | if (!file_.IsValid()) { | 1048 | if (!file_.IsValid()) { |
| 1049 | + net::Error net_error = net::FileErrorToNetError(file_.error_details()); | ||
| 1050 | + if (net_error == net::OK) { | ||
| 1051 | + net_error = net::MapSystemError(logging::GetLastSystemErrorCode()); | ||
| 1052 | + if (net_error == net::OK) { | ||
| 1053 | + net_error = net::ERR_FILE_NOT_FOUND; | ||
| 1054 | + } | ||
| 1055 | + } | ||
| 1054 | body_handler_task_runner_->PostTask( | 1056 | body_handler_task_runner_->PostTask( |
| 1055 | - FROM_HERE, base::BindOnce(std::move(on_done_callback), | 1057 | + FROM_HERE, base::BindOnce(std::move(on_done_callback), net_error, 0, |
| 1056 | - net::MapSystemError( | 1058 | + base::FilePath())); |
| 1057 | - logging::GetLastSystemErrorCode()), | ||
| 1058 | - 0, base::FilePath())); | ||
| 1059 | return; | 1059 | return; |
| 1060 | } | 1060 | } |
| 1061 | 1061 | ||
| @@ -1472,6 +1472,7 @@ interface NetworkContext { | |||
| 1472 | // managed by the same NetworkService. The particular NetworkContext this is | 1472 | // managed by the same NetworkService. The particular NetworkContext this is |
| 1473 | // called on will determine which NetworkContext gets its information and | 1473 | // called on will determine which NetworkContext gets its information and |
| 1474 | // configuration summary written out at the end of the log. | 1474 | // configuration summary written out at the end of the log. |
| 1475 | + [AllowedContext=sandbox.mojom.Context.kBrowser] | ||
| 1475 | CreateNetLogExporter(pending_receiver<NetLogExporter> receiver); | 1476 | CreateNetLogExporter(pending_receiver<NetLogExporter> receiver); |
| 1476 | 1477 | ||
| 1477 | // Tries to preconnect to `url`. `num_streams` may be used to request more | 1478 | // Tries to preconnect to `url`. `num_streams` may be used to request more |
| @@ -152,12 +152,15 @@ void ThrottlingP2PNetworkInterceptor::OnSendNetworkTimer() { | |||
| 152 | continue; | 152 | continue; |
| 153 | } | 153 | } |
| 154 | P2PSocketUdp* socket = packet_iterator->second.socket; | 154 | P2PSocketUdp* socket = packet_iterator->second.socket; |
| 155 | + // Move `pending_packet` out of the iterator to avoid using an iterator that | ||
| 156 | + // could be invalidated by SendFromInterceptor() below. | ||
| 157 | + P2PPendingPacket pending_packet = std::move(packet_iterator->second.packet); | ||
| 158 | + send_packets_.erase(packet_iterator); | ||
| 155 | 159 | ||
| 156 | // Check for dropped packets | 160 | // Check for dropped packets |
| 157 | if (packet.receive_time_us != webrtc::PacketDeliveryInfo::kNotReceived) { | 161 | if (packet.receive_time_us != webrtc::PacketDeliveryInfo::kNotReceived) { |
| 158 | - socket->SendFromInterceptor(packet_iterator->second.packet); | 162 | + socket->SendFromInterceptor(pending_packet); |
| 159 | } | 163 | } |
| 160 | - send_packets_.erase(packet_iterator); | ||
| 161 | } | 164 | } |
| 162 | 165 | ||
| 163 | // Schedule the next delivery timer | 166 | // Schedule the next delivery timer |
| @@ -215,15 +218,19 @@ void ThrottlingP2PNetworkInterceptor::OnReceiveNetworkTimer() { | |||
| 215 | } | 218 | } |
| 216 | 219 | ||
| 217 | P2PSocketUdp* socket = packet_iterator->second.socket; | 220 | P2PSocketUdp* socket = packet_iterator->second.socket; |
| 221 | + mojom::P2PReceivedPacketPtr pending_packet = | ||
| 222 | + std::move(packet_iterator->second.packet); | ||
| 223 | + scoped_refptr<net::IOBuffer> pending_buffer = | ||
| 224 | + std::move(packet_iterator->second.buffer); | ||
| 225 | + receive_packets_.erase(packet_iterator); | ||
| 218 | 226 | ||
| 219 | // Check for dropped packets | 227 | // Check for dropped packets |
| 220 | if (packet.receive_time_us != webrtc::PacketDeliveryInfo::kNotReceived) { | 228 | if (packet.receive_time_us != webrtc::PacketDeliveryInfo::kNotReceived) { |
| 221 | - packet_iterator->second.packet->timestamp = | 229 | + pending_packet->timestamp = |
| 222 | base::TimeTicks() + base::Microseconds(packet.receive_time_us); | 230 | base::TimeTicks() + base::Microseconds(packet.receive_time_us); |
| 223 | - socket->ReceiveFromInterceptor(std::move(packet_iterator->second.packet), | 231 | + socket->ReceiveFromInterceptor(std::move(pending_packet), |
| 224 | - std::move(packet_iterator->second.buffer)); | 232 | + std::move(pending_buffer)); |
| 225 | } | 233 | } |
| 226 | - receive_packets_.erase(packet_iterator); | ||
| 227 | } | 234 | } |
| 228 | 235 | ||
| 229 | // Schedule the next delivery timer | 236 | // Schedule the next delivery timer |
| @@ -37,6 +37,7 @@ ozone_platform_interactive_ui_tests_sources = | |||
| 37 | if (ozone_platform_headless) { | 37 | if (ozone_platform_headless) { |
| 38 | ozone_platforms += [ "headless" ] | 38 | ozone_platforms += [ "headless" ] |
| 39 | ozone_platform_deps += [ "platform/headless" ] | 39 | ozone_platform_deps += [ "platform/headless" ] |
| 40 | + ozone_platform_test_deps += [ "platform/headless:headless_unittests" ] | ||
| 40 | } | 41 | } |
| 41 | 42 | ||
| 42 | if (ozone_platform_ohos) { | 43 | if (ozone_platform_ohos) { |
| @@ -348,6 +349,7 @@ static_library("test_support") { | |||
| 348 | visibility = [ | 349 | visibility = [ |
| 349 | ":*", | 350 | ":*", |
| 350 | "platform/flatland:flatland_unittests", | 351 | "platform/flatland:flatland_unittests", |
| 352 | + "platform/headless:headless_unittests", | ||
| 351 | "platform/wayland:test_support", | 353 | "platform/wayland:test_support", |
| 352 | "platform/wayland:wayland_unittests", | 354 | "platform/wayland:wayland_unittests", |
| 353 | "platform/x11:x11_unittests", | 355 | "platform/x11:x11_unittests", |
| @@ -57,3 +57,18 @@ source_set("headless") { | |||
| 57 | deps += [ "//gpu/vulkan" ] | 57 | deps += [ "//gpu/vulkan" ] |
| 58 | } | 58 | } |
| 59 | } | 59 | } |
| 60 | + | ||
| 61 | +source_set("headless_unittests") { | ||
| 62 | + testonly = true | ||
| 63 | + sources = [ "headless_window_unittest.cc" ] | ||
| 64 | + deps = [ | ||
| 65 | + ":headless", | ||
| 66 | + "//base", | ||
| 67 | + "//base/test:test_support", | ||
| 68 | + "//testing/gtest", | ||
| 69 | + "//ui/display:test_support", | ||
| 70 | + "//ui/events:test_support", | ||
| 71 | + "//ui/ozone:platform", | ||
| 72 | + "//ui/ozone:test_support", | ||
| 73 | + ] | ||
| 74 | +} | ||
| @@ -78,18 +78,31 @@ void HeadlessWindow::SetFullscreen(bool fullscreen, int64_t target_display_id) { | |||
| 78 | return; | 78 | return; |
| 79 | } | 79 | } |
| 80 | 80 | ||
| 81 | + // PlatformWindowDelegate callbacks can destroy the window, invalidating | ||
| 82 | + // `this`. Keep a weak pointer and check it after each callback, here and | ||
| 83 | + // below. | ||
| 84 | + auto weak_ptr = GetWeakPtr(); | ||
| 85 | + | ||
| 81 | if (fullscreen) { | 86 | if (fullscreen) { |
| 82 | if (window_state_ != PlatformWindowState::kMaximized && | 87 | if (window_state_ != PlatformWindowState::kMaximized && |
| 83 | window_state_ != PlatformWindowState::kFullScreen) { | 88 | window_state_ != PlatformWindowState::kFullScreen) { |
| 84 | restored_bounds_ = bounds_; | 89 | restored_bounds_ = bounds_; |
| 85 | } | 90 | } |
| 86 | ZoomWindowBounds(); | 91 | ZoomWindowBounds(); |
| 92 | + if (!weak_ptr) { | ||
| 93 | + return; | ||
| 94 | + } | ||
| 95 | + | ||
| 87 | UpdateWindowState(PlatformWindowState::kFullScreen); | 96 | UpdateWindowState(PlatformWindowState::kFullScreen); |
| 88 | } else { | 97 | } else { |
| 89 | if (window_state_ != PlatformWindowState::kFullScreen) { | 98 | if (window_state_ != PlatformWindowState::kFullScreen) { |
| 90 | return; | 99 | return; |
| 91 | } | 100 | } |
| 92 | RestoreWindowBounds(); | 101 | RestoreWindowBounds(); |
| 102 | + if (!weak_ptr) { | ||
| 103 | + return; | ||
| 104 | + } | ||
| 105 | + | ||
| 93 | UpdateWindowState(PlatformWindowState::kNormal); | 106 | UpdateWindowState(PlatformWindowState::kNormal); |
| 94 | } | 107 | } |
| 95 | } | 108 | } |
| @@ -101,21 +114,37 @@ void HeadlessWindow::Maximize() { | |||
| 101 | 114 | ||
| 102 | if (window_state_ != PlatformWindowState::kMaximized && | 115 | if (window_state_ != PlatformWindowState::kMaximized && |
| 103 | window_state_ != PlatformWindowState::kFullScreen) { | 116 | window_state_ != PlatformWindowState::kFullScreen) { |
| 117 | + auto weak_ptr = GetWeakPtr(); | ||
| 118 | + | ||
| 104 | restored_bounds_ = bounds_; | 119 | restored_bounds_ = bounds_; |
| 105 | ZoomWindowBounds(); | 120 | ZoomWindowBounds(); |
| 121 | + if (!weak_ptr) { | ||
| 122 | + return; | ||
| 123 | + } | ||
| 124 | + | ||
| 106 | UpdateWindowState(PlatformWindowState::kMaximized); | 125 | UpdateWindowState(PlatformWindowState::kMaximized); |
| 107 | } | 126 | } |
| 108 | } | 127 | } |
| 109 | 128 | ||
| 110 | void HeadlessWindow::Minimize() { | 129 | void HeadlessWindow::Minimize() { |
| 111 | if (window_state_ != PlatformWindowState::kMinimized) { | 130 | if (window_state_ != PlatformWindowState::kMinimized) { |
| 131 | + auto weak_ptr = GetWeakPtr(); | ||
| 132 | + | ||
| 112 | // Minimized window retains its size and position, however, it's made | 133 | // Minimized window retains its size and position, however, it's made |
| 113 | // hidden by Aura. | 134 | // hidden by Aura. |
| 114 | if (window_state_ == PlatformWindowState::kMaximized || | 135 | if (window_state_ == PlatformWindowState::kMaximized || |
| 115 | window_state_ == PlatformWindowState::kFullScreen) { | 136 | window_state_ == PlatformWindowState::kFullScreen) { |
| 116 | RestoreWindowBounds(); | 137 | RestoreWindowBounds(); |
| 138 | + if (!weak_ptr) { | ||
| 139 | + return; | ||
| 140 | + } | ||
| 117 | } | 141 | } |
| 142 | + | ||
| 118 | UpdateWindowState(PlatformWindowState::kMinimized); | 143 | UpdateWindowState(PlatformWindowState::kMinimized); |
| 144 | + if (!weak_ptr) { | ||
| 145 | + return; | ||
| 146 | + } | ||
| 147 | + | ||
| 119 | // Minimized windows are inactive. Aura activates minimized windows | 148 | // Minimized windows are inactive. Aura activates minimized windows |
| 120 | // when restoring. If we don't deactivate the window here, the subsequent | 149 | // when restoring. If we don't deactivate the window here, the subsequent |
| 121 | // activation will be optimized away, causing https://crbug.com/358998544. | 150 | // activation will be optimized away, causing https://crbug.com/358998544. |
| @@ -125,7 +154,13 @@ void HeadlessWindow::Minimize() { | |||
| 125 | 154 | ||
| 126 | void HeadlessWindow::Restore() { | 155 | void HeadlessWindow::Restore() { |
| 127 | if (window_state_ != PlatformWindowState::kNormal) { | 156 | if (window_state_ != PlatformWindowState::kNormal) { |
| 157 | + auto weak_ptr = GetWeakPtr(); | ||
| 158 | + | ||
| 128 | RestoreWindowBounds(); | 159 | RestoreWindowBounds(); |
| 160 | + if (!weak_ptr) { | ||
| 161 | + return; | ||
| 162 | + } | ||
| 163 | + | ||
| 129 | UpdateWindowState(PlatformWindowState::kNormal); | 164 | UpdateWindowState(PlatformWindowState::kNormal); |
| 130 | } | 165 | } |
| 131 | } | 166 | } |
| @@ -9,6 +9,7 @@ | |||
| 9 | 9 | ||
| 10 | 10 | ||
| 11 | 11 | ||
| 12 | + | ||
| 12 | 13 | ||
| 13 | 14 | ||
| 14 | 15 | ||
| @@ -73,6 +74,10 @@ class HeadlessWindow : public PlatformWindow { | |||
| 73 | protected: | 74 | protected: |
| 74 | PlatformWindowDelegate* delegate() { return delegate_; } | 75 | PlatformWindowDelegate* delegate() { return delegate_; } |
| 75 | 76 | ||
| 77 | + base::WeakPtr<HeadlessWindow> GetWeakPtr() { | ||
| 78 | + return weak_ptr_factory_.GetWeakPtr(); | ||
| 79 | + } | ||
| 80 | + | ||
| 76 | private: | 81 | private: |
| 77 | enum class ActivationState { | 82 | enum class ActivationState { |
| 78 | kUnknown, | 83 | kUnknown, |
| @@ -90,6 +95,8 @@ class HeadlessWindow : public PlatformWindow { | |||
| 90 | std::optional<gfx::Rect> restored_bounds_; | 95 | std::optional<gfx::Rect> restored_bounds_; |
| 91 | PlatformWindowState window_state_ = PlatformWindowState::kUnknown; | 96 | PlatformWindowState window_state_ = PlatformWindowState::kUnknown; |
| 92 | ActivationState activation_state_ = ActivationState::kUnknown; | 97 | ActivationState activation_state_ = ActivationState::kUnknown; |
| 98 | + | ||
| 99 | + base::WeakPtrFactory<HeadlessWindow> weak_ptr_factory_{this}; | ||
| 93 | }; | 100 | }; |
| 94 | 101 | ||
| 95 | } // namespace ui | 102 | } // namespace ui |
| @@ -0,0 +1,143 @@ | |||
| 1 | +// Copyright 2026 The Chromium Authors | ||
| 2 | +// Use of this source code is governed by a BSD-style license that can be | ||
| 3 | +// found in the LICENSE file. | ||
| 4 | + | ||
| 5 | + | ||
| 6 | + | ||
| 7 | + | ||
| 8 | + | ||
| 9 | + | ||
| 10 | + | ||
| 11 | + | ||
| 12 | + | ||
| 13 | + | ||
| 14 | + | ||
| 15 | +namespace ui { | ||
| 16 | +namespace { | ||
| 17 | + | ||
| 18 | +// A PlatformWindowDelegate that simulates the production behaviour of | ||
| 19 | +// DesktopWindowTreeHostPlatform: when the platform window calls back into the | ||
| 20 | +// delegate (OnBoundsChanged / OnWindowStateChanged), an observer may | ||
| 21 | +// synchronously call Widget::CloseNow(), which ends up calling | ||
| 22 | +// HeadlessWindow::Close() -> delegate_->OnClosed() -> | ||
| 23 | +// DWTHP::OnClosed() -> SetPlatformWindow(nullptr) -> ~HeadlessWindow(). | ||
| 24 | +// | ||
| 25 | +// This delegate models that by resetting its owned unique_ptr<PlatformWindow> | ||
| 26 | +// from inside the chosen callback. | ||
| 27 | +class DestroyingDelegate : public PlatformWindowDelegate { | ||
| 28 | + public: | ||
| 29 | + enum class DestroyOn { | ||
| 30 | + kNone, | ||
| 31 | + kBoundsChanged, | ||
| 32 | + kWindowStateChanged, | ||
| 33 | + }; | ||
| 34 | + | ||
| 35 | + explicit DestroyingDelegate(DestroyOn destroy_on) : destroy_on_(destroy_on) {} | ||
| 36 | + ~DestroyingDelegate() override = default; | ||
| 37 | + | ||
| 38 | + void set_destroy_on(DestroyOn destroy_on) { destroy_on_ = destroy_on; } | ||
| 39 | + | ||
| 40 | + void SetWindow(std::unique_ptr<PlatformWindow> window) { | ||
| 41 | + window_ = std::move(window); | ||
| 42 | + } | ||
| 43 | + | ||
| 44 | + // PlatformWindowDelegate: | ||
| 45 | + void OnBoundsChanged(const BoundsChange& change) override { | ||
| 46 | + if (destroy_on_ == DestroyOn::kBoundsChanged) { | ||
| 47 | + window_.reset(); | ||
| 48 | + } | ||
| 49 | + } | ||
| 50 | + void OnDamageRect(const gfx::Rect& damaged_region) override {} | ||
| 51 | + void DispatchEvent(Event* event) override {} | ||
| 52 | + void OnCloseRequest() override {} | ||
| 53 | + void OnClosed() override { window_.reset(); } | ||
| 54 | + void OnWindowStateChanged(PlatformWindowState old_state, | ||
| 55 | + PlatformWindowState new_state) override { | ||
| 56 | + if (destroy_on_ == DestroyOn::kWindowStateChanged) { | ||
| 57 | + window_.reset(); | ||
| 58 | + } | ||
| 59 | + } | ||
| 60 | + void OnLostCapture() override {} | ||
| 61 | + void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget) override {} | ||
| 62 | + void OnWillDestroyAcceleratedWidget() override {} | ||
| 63 | + void OnAcceleratedWidgetDestroyed() override {} | ||
| 64 | + void OnActivationChanged(bool active) override {} | ||
| 65 | + void OnCursorUpdate() override {} | ||
| 66 | + bool CanMaximize() const override { return true; } | ||
| 67 | + bool CanFullscreen() const override { return true; } | ||
| 68 | + gfx::Rect ConvertRectToPixels(const gfx::Rect& rect_in_dip) const override { | ||
| 69 | + return rect_in_dip; | ||
| 70 | + } | ||
| 71 | + gfx::Rect ConvertRectToDIP(const gfx::Rect& rect_in_pixels) const override { | ||
| 72 | + return rect_in_pixels; | ||
| 73 | + } | ||
| 74 | + | ||
| 75 | + private: | ||
| 76 | + DestroyOn destroy_on_ = DestroyOn::kNone; | ||
| 77 | + std::unique_ptr<PlatformWindow> window_; | ||
| 78 | +}; | ||
| 79 | + | ||
| 80 | +class HeadlessWindowCrashTest : public ::testing::Test { | ||
| 81 | + public: | ||
| 82 | + HeadlessWindowCrashTest() = default; | ||
| 83 | + ~HeadlessWindowCrashTest() override = default; | ||
| 84 | + | ||
| 85 | + protected: | ||
| 86 | + display::test::TestScreen test_screen_{/*create_display=*/true, | ||
| 87 | + /*register_screen=*/true}; | ||
| 88 | + HeadlessWindowManager manager_; | ||
| 89 | +}; | ||
| 90 | + | ||
| 91 | +TEST_F(HeadlessWindowCrashTest, DestroyViaObserverInSetFullscreen) { | ||
| 92 | + DestroyingDelegate delegate(DestroyingDelegate::DestroyOn::kBoundsChanged); | ||
| 93 | + | ||
| 94 | + auto window = std::make_unique<HeadlessWindow>(&delegate, &manager_, | ||
| 95 | + gfx::Rect(0, 0, 100, 100)); | ||
| 96 | + HeadlessWindow* headless_window = window.get(); | ||
| 97 | + delegate.SetWindow(std::move(window)); | ||
| 98 | + | ||
| 99 | + headless_window->SetFullscreen(/*fullscreen=*/true, | ||
| 100 | + /*target_display_id=*/-1); | ||
| 101 | +} | ||
| 102 | + | ||
| 103 | +TEST_F(HeadlessWindowCrashTest, DestroyViaObserverInMaximize) { | ||
| 104 | + DestroyingDelegate delegate( | ||
| 105 | + DestroyingDelegate::DestroyOn::kWindowStateChanged); | ||
| 106 | + | ||
| 107 | + auto window = std::make_unique<HeadlessWindow>(&delegate, &manager_, | ||
| 108 | + gfx::Rect(0, 0, 100, 100)); | ||
| 109 | + HeadlessWindow* headless_window = window.get(); | ||
| 110 | + delegate.SetWindow(std::move(window)); | ||
| 111 | + | ||
| 112 | + headless_window->Maximize(); | ||
| 113 | +} | ||
| 114 | + | ||
| 115 | +TEST_F(HeadlessWindowCrashTest, DestroyViaObserverInMinimize) { | ||
| 116 | + DestroyingDelegate delegate( | ||
| 117 | + DestroyingDelegate::DestroyOn::kWindowStateChanged); | ||
| 118 | + | ||
| 119 | + auto window = std::make_unique<HeadlessWindow>(&delegate, &manager_, | ||
| 120 | + gfx::Rect(0, 0, 100, 100)); | ||
| 121 | + HeadlessWindow* headless_window = window.get(); | ||
| 122 | + delegate.SetWindow(std::move(window)); | ||
| 123 | + | ||
| 124 | + headless_window->Minimize(); | ||
| 125 | +} | ||
| 126 | + | ||
| 127 | +TEST_F(HeadlessWindowCrashTest, DestroyViaObserverInRestore) { | ||
| 128 | + DestroyingDelegate delegate(DestroyingDelegate::DestroyOn::kNone); | ||
| 129 | + | ||
| 130 | + auto window = std::make_unique<HeadlessWindow>(&delegate, &manager_, | ||
| 131 | + gfx::Rect(0, 0, 100, 100)); | ||
| 132 | + HeadlessWindow* headless_window = window.get(); | ||
| 133 | + delegate.SetWindow(std::move(window)); | ||
| 134 | + | ||
| 135 | + headless_window->Maximize(); | ||
| 136 | + | ||
| 137 | + delegate.set_destroy_on(DestroyingDelegate::DestroyOn::kBoundsChanged); | ||
| 138 | + | ||
| 139 | + headless_window->Restore(); | ||
| 140 | +} | ||
| 141 | + | ||
| 142 | +} // namespace | ||
| 143 | +} // namespace ui | ||