diff --git a/src/build/config/ohos/config.gni b/src/build/config/ohos/config.gni index 1ad2a5244f..8f181d0938 100644 --- a/src/build/config/ohos/config.gni +++ b/src/build/config/ohos/config.gni @@ -18,6 +18,7 @@ if (is_ohos) { declare_args() { product_name = "" + gpu_switch = "on" } # Defines the name the ohos build gives to the current host CPU @@ -60,64 +61,75 @@ if (is_ohos) { # ohos include and libs dependencies ohos_src_includes = [ - "$ohos_build_root/base/location/services/location_locator/callback/include", - "$ohos_build_root/base/location/services/location_common/common/include", - "$ohos_build_root/base/location/interfaces/inner_api/include", - "$ohos_build_root/base/location/services/utils/include", - "$ohos_build_root/foundation/bundlemanager/bundle_framework/interfaces/innerkits/libeventhandler/include", - "$ohos_build_root/foundation/appexecfwk/standard/interfaces/innerkits/libeventhandler/include", "$ohos_build_root/utils/native/base/include", "$ohos_build_root/base/hiviewdfx/hilog/interfaces/native/innerkits/include", "$ohos_build_root/foundation/graphic/graphic/interfaces/inner_api/common", + "$ohos_build_root/foundation/graphic/graphic_2d/interfaces/inner_api/common", + "$ohos_build_root/foundation/graphic/graphic_2d/utils/buffer_handle/export", + "$ohos_build_root/foundation/graphic/standard/interfaces/innerkits/common", + "$ohos_build_root/foundation/graphic/standard/utils/buffer_handle/export", "$ohos_build_root/drivers/peripheral/display/interfaces/include", "$ohos_build_root/foundation/graphic/graphic/utils/buffer_handle/export", "$ohos_build_root/foundation/multimedia/media_standard/interfaces/innerkits/native/media/include", + "$ohos_build_root/foundation/bundlemanager/bundle_framework/interfaces/innerkits/libeventhandler/include", + "$ohos_build_root/foundation/bundlemanager/bundle_framework/interfaces/inner_api/appexecfwk_base/include", "$ohos_build_root/foundation/multimedia/media_standard/interfaces/inner_api/native", "$ohos_build_root/drivers/peripheral/base", - "$ohos_build_root/foundation/graphic/graphic/interfaces/inner_api/surface", + "$ohos_build_root/foundation/graphic/standard/interfaces/innerkits/surface", "$ohos_build_root/foundation/graphic/surface/interfaces/kits", + "$ohos_build_root/foundation/graphic/graphic/interfaces/inner_api/surface", + "$ohos_build_root/foundation/graphic/graphic_2d/interfaces/inner_api/surface", + "$ohos_build_root/foundation/communication/ipc", "$ohos_build_root/foundation/communication/ipc/interfaces/innerkits/ipc_core/include", "$ohos_build_root/base/miscservices/inputmethod/frameworks/inputmethod_controller/include", "$ohos_build_root/base/miscservices/inputmethod/services/include", "$ohos_build_root/base/miscservices/inputmethod/frameworks/inputmethod_ability/include", "$ohos_build_root/utils/native/base/include", "$ohos_build_root/foundation/multimodalinput/input/interfaces/native/innerkits/event/include", + "$ohos_build_root/base/location/location_locator/callback/include", "$ohos_build_root/base/location/utils/include", "$ohos_build_root/base/location/interfaces/innerkits/locator_standard/include", "$ohos_build_root/base/location/location_common/common/include", + "$ohos_build_root/base/location/services/location_common/common/include", + "$ohos_build_root/base/location/serviceslocation_locator/callback/include", + "$ohos_build_root/base/location/services/utils/include", + "$ohos_build_root/base/location/interfaces/innerkits/locator_standard/include", + "$ohos_build_root/base/location/interfaces/inner_api/include", "$ohos_build_root/foundation/distributedschedule/samgr/interfaces/innerkits/samgr_proxy/include", "$ohos_build_root/utils/system/safwk/native/include", "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/base/include", + "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/base/include/ohos/aafwk/base", + "$ohos_build_root/foundation/aafwk/standard/ability_base/interfaces/inner_api/want/include", + "$ohos_build_root/foundation/aafwk/standard/ability_base/interfaces/inner_api/uri/include", + "$ohos_build_root/foundation/aafwk/standard/ability_base/interfaces/inner_api/base/include", + "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/uri/include", "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/want/include/ohos/aafwk/content", "$ohos_build_root/foundation/appexecfwk/standard/common/log/include", "$ohos_build_root/foundation/appexecfwk/standard/common/perf/include", + "$ohos_build_root/foundation/appexecfwk/standard/interfaces/inner_api/appexecfwk_base/include", + "$ohos_build_root/foundation/appexecfwk/standard/interfaces/innerkits/libeventhandler/include", "$ohos_build_root/foundation/appexecfwk/standard/interfaces/innerkits/appexecfwk_base/include", "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/app_manager/include", "$ohos_build_root/foundation/distributedschedule/dmsfwk/services/dtbschedmgr/include", + "$ohos_build_root/developtools/bytrace_standard/interfaces/innerkits/native/include", "$ohos_build_root/third_party/jsoncpp/include", "$ohos_build_root/third_party/json/include", - "$ohos_build_root/base/security/access_token/interfaces/innerkits/accesstoken/include/", - "$ohos_build_root/foundation/aafwk/standard/interfaces/innerkits/uri/include", + "$ohos_build_root/base/security/access_token/interfaces/innerkits/accesstoken/include", "$ohos_build_root/base/web/webview/ohos_adapter/interfaces", - "$ohos_build_root/base/location/services/location_common/common/include", - "$ohos_build_root/base/location/interfaces/inner_api/include", - "$ohos_build_root/base/location/services/utils/include", - "$ohos_build_root/base/location/services/location_locator/callback/include" ] - if (use_musl) { - if (current_cpu == "arm") { - ohos_libs_dir = [ "$ohos_build_root/out/rk3568/packages/phone/system/lib" ] - } else if (current_cpu == "arm64") { - ohos_libs_dir = [ "$ohos_build_root/out/rk3568/packages/phone/system/lib64" ] - } + if (current_cpu == "arm") { + ohos_libs_dir = [ "$ohos_build_root/out/rk3568/packages/phone/system/lib"] + } else if (current_cpu == "arm64") { + ohos_libs_dir = [ "$ohos_build_root/out/rk3568/packages/phone/system/lib64"] + } } else { - if (current_cpu == "arm") { - ohos_libs_dir = [ "$ohos_build_root/out/ohos-arm-release/packages/phone/system/lib" ] - } else if (current_cpu == "arm64") { - ohos_libs_dir = [ "$ohos_build_root/out/ohos-arm64-release/packages/phone/system/lib64" ] - } + if (current_cpu == "arm") { + ohos_libs_dir = [ "$ohos_build_root/out/ohos-arm-release/packages/phone/system/lib"] + } else if (current_cpu == "arm64") { + ohos_libs_dir = [ "$ohos_build_root/out/ohos-arm64-release/packages/phone/system/lib64"] + } } } else { if (use_musl) { diff --git a/src/cc/paint/paint_op_reader.cc b/src/cc/paint/paint_op_reader.cc index 68429994cd..201e67587f 100644 --- a/src/cc/paint/paint_op_reader.cc +++ b/src/cc/paint/paint_op_reader.cc @@ -310,6 +310,10 @@ void PaintOpReader::Read(PaintImage* image) { SkImageInfo image_info = SkImageInfo::Make(width, height, color_type, kPremul_SkAlphaType); + if (pixel_size < image_info.computeMinByteSize()) { + SetInvalid(); + return; + } const volatile void* pixel_data = ExtractReadableMemory(pixel_size); if (!valid_) return; diff --git a/src/chrome/browser/extensions/BUILD.gn b/src/chrome/browser/extensions/BUILD.gn index 37117db7fc..21440d4f94 100644 --- a/src/chrome/browser/extensions/BUILD.gn +++ b/src/chrome/browser/extensions/BUILD.gn @@ -792,6 +792,7 @@ static_library("extensions") { "//chrome/browser/safe_browsing", "//chrome/browser/web_applications", "//chrome/browser/web_applications/components", + "//components/security_interstitials/content:security_interstitial_page", "//components/site_engagement/core/mojom:mojo_bindings", "//components/webapps/browser", diff --git a/src/chrome/browser/extensions/api/debugger/debugger_api.cc b/src/chrome/browser/extensions/api/debugger/debugger_api.cc index 429f16cc38..56e2a22db5 100644 --- a/src/chrome/browser/extensions/api/debugger/debugger_api.cc +++ b/src/chrome/browser/extensions/api/debugger/debugger_api.cc @@ -37,6 +37,7 @@ #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h" #include "chrome/common/chrome_switches.h" #include "content/public/browser/browser_task_traits.h" +#include "components/security_interstitials/content/security_interstitial_tab_helper.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/devtools_agent_host.h" #include "content/public/browser/notification_service.h" @@ -125,6 +126,14 @@ bool ExtensionMayAttachToWebContents(const Extension& extension, WebContents& web_contents, Profile* profile, std::string* error) { + security_interstitials::SecurityInterstitialTabHelper* + security_interstitial_tab_helper = security_interstitials:: + SecurityInterstitialTabHelper::FromWebContents(&web_contents); + if (security_interstitial_tab_helper && + security_interstitial_tab_helper->IsDisplayingInterstitial()) { + *error = debugger_api_constants::kRestrictedError; + return false; + } // This is *not* redundant to the checks below, as // web_contents.GetLastCommittedURL() may be different from // web_contents.GetMainFrame()->GetLastCommittedURL(), with the diff --git a/src/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/src/chrome/browser/extensions/api/debugger/debugger_apitest.cc index 44bcd6b136..51a2156d26 100644 --- a/src/chrome/browser/extensions/api/debugger/debugger_apitest.cc +++ b/src/chrome/browser/extensions/api/debugger/debugger_apitest.cc @@ -24,9 +24,15 @@ #include "chrome/test/base/ui_test_utils.h" #include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar_delegate.h" +#include "components/security_interstitials/content/security_interstitial_controller_client.h" +#include "components/security_interstitials/content/security_interstitial_page.h" +#include "components/security_interstitials/content/security_interstitial_tab_helper.h" +#include "components/security_interstitials/content/settings_page_helper.h" +#include "components/security_interstitials/core/metrics_helper.h" #include "components/sessions/content/session_tab_helper.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" +#include "content/public/test/mock_navigation_handle.h" #include "extensions/browser/extension_function.h" #include "extensions/common/extension.h" #include "extensions/common/extension_builder.h" @@ -50,6 +56,9 @@ class DebuggerApiTest : public ExtensionApiTest { // to succeed. testing::AssertionResult RunAttachFunction(const GURL& url, const std::string& expected_error); + testing::AssertionResult RunAttachFunction( + const content::WebContents* web_contents, + const std::string& expected_error); const Extension* extension() const { return extension_.get(); } base::CommandLine* command_line() const { return command_line_; } @@ -97,9 +106,13 @@ void DebuggerApiTest::SetUpOnMainThread() { testing::AssertionResult DebuggerApiTest::RunAttachFunction( const GURL& url, const std::string& expected_error) { ui_test_utils::NavigateToURL(browser(), url); - content::WebContents* web_contents = - browser()->tab_strip_model()->GetActiveWebContents(); + return RunAttachFunction(browser()->tab_strip_model()->GetActiveWebContents(), + expected_error); +} +testing::AssertionResult DebuggerApiTest::RunAttachFunction( + const content::WebContents* web_contents, + const std::string& expected_error) { // Attach by tabId. int tab_id = sessions::SessionTabHelper::IdForTab(web_contents).id(); std::string debugee_by_tab = base::StringPrintf("{\"tabId\": %d}", tab_id); @@ -209,6 +222,59 @@ IN_PROC_BROWSER_TEST_F(DebuggerApiTest, EXPECT_TRUE(RunExtensionTest("debugger_file_access")) << message_; } +class TestInterstitialPage + : public security_interstitials::SecurityInterstitialPage { + public: + TestInterstitialPage(content::WebContents* web_contents, + const GURL& request_url) + : SecurityInterstitialPage( + web_contents, + request_url, + std::make_unique< + security_interstitials::SecurityInterstitialControllerClient>( + web_contents, + CreateTestMetricsHelper(web_contents), + nullptr, + base::i18n::GetConfiguredLocale(), + GURL(), + /* settings_page_helper*/ nullptr)) {} + + ~TestInterstitialPage() override = default; + void OnInterstitialClosing() override {} + + protected: + void PopulateInterstitialStrings(base::Value* load_time_data) override {} + + std::unique_ptr + CreateTestMetricsHelper(content::WebContents* web_contents) { + security_interstitials::MetricsHelper::ReportDetails report_details; + report_details.metric_prefix = "test_blocking_page"; + return std::make_unique( + GURL(), report_details, nullptr); + } +}; + +IN_PROC_BROWSER_TEST_F(DebuggerApiTest, + DebuggerNotAllowedOnSecirutyInterstitials) { + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + std::unique_ptr navigation_handle = + std::make_unique( + GURL("https://google.com/"), web_contents->GetMainFrame()); + navigation_handle->set_has_committed(true); + navigation_handle->set_is_same_document(false); + EXPECT_TRUE(RunAttachFunction(web_contents, "")); + + security_interstitials::SecurityInterstitialTabHelper::AssociateBlockingPage( + navigation_handle.get(), + std::make_unique(web_contents, GURL())); + security_interstitials::SecurityInterstitialTabHelper::FromWebContents( + web_contents) + ->DidFinishNavigation(navigation_handle.get()); + + EXPECT_TRUE(RunAttachFunction(web_contents, "Cannot attach to this target.")); +} + IN_PROC_BROWSER_TEST_F(DebuggerApiTest, InfoBar) { int tab_id = sessions::SessionTabHelper::IdForTab( browser()->tab_strip_model()->GetActiveWebContents()) diff --git a/src/chrome/browser/ui/toolbar/media_router_action_controller.cc b/src/chrome/browser/ui/toolbar/media_router_action_controller.cc index 9c506973a9..b3a1b91dc9 100644 --- a/src/chrome/browser/ui/toolbar/media_router_action_controller.cc +++ b/src/chrome/browser/ui/toolbar/media_router_action_controller.cc @@ -21,9 +21,7 @@ MediaRouterActionController::MediaRouterActionController(Profile* profile) profile, media_router::MediaRouterFactory::GetApiForBrowserContext(profile)) {} -MediaRouterActionController::~MediaRouterActionController() { - DCHECK_EQ(dialog_count_, 0u); -} +MediaRouterActionController::~MediaRouterActionController() = default; // static bool MediaRouterActionController::IsActionShownByPolicy(Profile* profile) { diff --git a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc index b64eaf262f..cc8556fd6a 100644 --- a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc +++ b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.cc @@ -124,14 +124,15 @@ bool MediaRouterDialogControllerViews::IsShowingMediaRouterDialog() const { void MediaRouterDialogControllerViews::Reset() { // If |ui_| is null, Reset() has already been called. if (ui_) { - if (IsShowingMediaRouterDialog() && GetActionController()) + if (GetActionController()) GetActionController()->OnDialogHidden(); ui_.reset(); MediaRouterDialogController::Reset(); } } -void MediaRouterDialogControllerViews::OnWidgetClosing(views::Widget* widget) { +void MediaRouterDialogControllerViews::OnWidgetDestroying( + views::Widget* widget) { DCHECK(scoped_widget_observations_.IsObservingSource(widget)); if (ui_) ui_->LogMediaSinkStatus(); diff --git a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h index 40f19f2894..bf438f4833 100644 --- a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h +++ b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h @@ -40,7 +40,7 @@ class MediaRouterDialogControllerViews void Reset() override; // views::WidgetObserver: - void OnWidgetClosing(views::Widget* widget) override; + void OnWidgetDestroying(views::Widget* widget) override; // Sets a callback to be called whenever a dialog is created. void SetDialogCreationCallbackForTesting(base::RepeatingClosure callback); diff --git a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views_browsertest.cc b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views_browsertest.cc index a8fee9a096..b20537794e 100644 --- a/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views_browsertest.cc +++ b/src/chrome/browser/ui/views/media_router/media_router_dialog_controller_views_browsertest.cc @@ -7,6 +7,7 @@ #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/views/global_media_controls/media_dialog_view.h" #include "chrome/browser/ui/views/media_router/cast_dialog_view.h" #include "chrome/browser/ui/views/media_router/media_router_dialog_controller_views.h" #include "chrome/test/base/in_process_browser_test.h" @@ -14,6 +15,8 @@ #include "content/public/test/browser_test.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/base/page_transition_types.h" +#include "ui/views/test/widget_test.h" +#include "ui/views/widget/native_widget_private.h" #include "ui/views/widget/widget.h" using content::WebContents; @@ -26,6 +29,7 @@ class MediaRouterDialogControllerViewsTest : public InProcessBrowserTest { ~MediaRouterDialogControllerViewsTest() override = default; void OpenMediaRouterDialog(); + void CloseWebContents(); protected: WebContents* initiator_; @@ -51,6 +55,10 @@ void MediaRouterDialogControllerViewsTest::OpenMediaRouterDialog() { ASSERT_TRUE(dialog_controller_->IsShowingMediaRouterDialog()); } +void MediaRouterDialogControllerViewsTest::CloseWebContents() { + initiator_->Close(); +} + // Create/Get a media router dialog for initiator. IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerViewsTest, OpenCloseMediaRouterDialog) { @@ -63,4 +71,22 @@ IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerViewsTest, EXPECT_EQ(CastDialogView::GetCurrentDialogWidget(), nullptr); } +// Regression test for crbug.com/1308341. +IN_PROC_BROWSER_TEST_F(MediaRouterDialogControllerViewsTest, + MediaBubbleClosedByPlatform) { + OpenMediaRouterDialog(); + base::RunLoop().RunUntilIdle(); + views::Widget* widget = CastDialogView::GetCurrentDialogWidget(); + ASSERT_TRUE(widget); + EXPECT_TRUE(widget->HasObserver(dialog_controller_)); + // The media bubble usually will close itself on deactivation, but + // crbug.com/1308341 shows a state where the browser is not responsive + // to activation change. Simulate that. + CastDialogView::GetInstance()->set_close_on_deactivate(false); + views::test::WidgetDestroyedWaiter waiter(widget); + widget->native_widget_private()->Close(); + waiter.Wait(); + CloseWebContents(); +} + } // namespace media_router diff --git a/src/chrome/browser/ui/views/media_router/media_router_ui_browsertest.cc b/src/chrome/browser/ui/views/media_router/media_router_ui_browsertest.cc index 190cbc6ec3..ef254708fb 100644 --- a/src/chrome/browser/ui/views/media_router/media_router_ui_browsertest.cc +++ b/src/chrome/browser/ui/views/media_router/media_router_ui_browsertest.cc @@ -18,6 +18,7 @@ #include "chrome/browser/ui/toolbar/media_router_action_controller.h" #include "chrome/browser/ui/views/frame/browser_view.h" #include "chrome/browser/ui/views/media_router/app_menu_test_api.h" +#include "chrome/browser/ui/views/media_router/cast_dialog_view.h" #include "chrome/browser/ui/views/media_router/cast_toolbar_button.h" #include "chrome/browser/ui/views/toolbar/toolbar_view.h" #include "chrome/common/url_constants.h" @@ -33,6 +34,7 @@ #include "content/public/test/test_utils.h" #include "ui/base/ui_base_features.h" #include "ui/events/base_event_utils.h" +#include "ui/views/test/widget_test.h" #include "ui/views/widget/widget.h" namespace media_router { @@ -116,7 +118,10 @@ IN_PROC_BROWSER_TEST_F(MediaRouterUIBrowserTest, OpenDialogFromContextMenu) { menu.ExecuteCommand(IDC_ROUTE_MEDIA, 0); EXPECT_TRUE(dialog_controller->IsShowingMediaRouterDialog()); + views::test::WidgetDestroyedWaiter waiter( + CastDialogView::GetCurrentDialogWidget()); dialog_controller->HideMediaRouterDialog(); + waiter.Wait(); EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); } @@ -133,8 +138,11 @@ IN_PROC_BROWSER_TEST_F(MediaRouterUIBrowserTest, OpenDialogFromAppMenu) { app_menu_test_api->ExecuteCommand(IDC_ROUTE_MEDIA); EXPECT_TRUE(dialog_controller->IsShowingMediaRouterDialog()); + views::test::WidgetDestroyedWaiter waiter( + CastDialogView::GetCurrentDialogWidget()); dialog_controller->HideMediaRouterDialog(); EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); + waiter.Wait(); } // TODO(crbug.com/1004635) Disabled due to flake on Windows and Linux @@ -156,8 +164,12 @@ IN_PROC_BROWSER_TEST_F(MediaRouterUIBrowserTest, dialog_controller->ShowMediaRouterDialog(MediaRouterDialogOpenOrigin::PAGE); EXPECT_TRUE(ToolbarIconExists()); + + views::test::WidgetDestroyedWaiter waiter( + CastDialogView::GetCurrentDialogWidget()); // Clicking on the toolbar icon should hide both the dialog and the icon. PressToolbarIcon(); + waiter.Wait(); EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); EXPECT_FALSE(ToolbarIconExists()); @@ -254,21 +266,32 @@ IN_PROC_BROWSER_TEST_F(MediaRouterUIBrowserTest, EXPECT_TRUE(dialog_controller->IsShowingMediaRouterDialog()); // Reload the browser and wait. - content::TestNavigationObserver reload_observer( - browser()->tab_strip_model()->GetActiveWebContents()); - chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB); - reload_observer.Wait(); + { + views::test::WidgetDestroyedWaiter waiter( + CastDialogView::GetCurrentDialogWidget()); + content::TestNavigationObserver reload_observer( + browser()->tab_strip_model()->GetActiveWebContents()); + chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB); + reload_observer.Wait(); + + waiter.Wait(); + // The reload should have closed the dialog. + EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); + } - // The reload should have closed the dialog. - EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); PressToolbarIcon(); EXPECT_TRUE(dialog_controller->IsShowingMediaRouterDialog()); - // Navigate away. - ui_test_utils::NavigateToURL(browser(), GURL("about:blank")); + { + views::test::WidgetDestroyedWaiter waiter( + CastDialogView::GetCurrentDialogWidget()); + // Navigate away. + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL("about:blank"))); - // The navigation should have closed the dialog. - EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); + // The navigation should have closed the dialog. + waiter.Wait(); + EXPECT_FALSE(dialog_controller->IsShowingMediaRouterDialog()); + } SetAlwaysShowActionPref(false); } diff --git a/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_browsertest.cc b/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_browsertest.cc index 9cfdcd25f6..8a336cd03f 100644 --- a/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_browsertest.cc +++ b/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_browsertest.cc @@ -23,10 +23,13 @@ #include "chrome/browser/web_applications/components/web_application_info.h" #include "chrome/browser/web_applications/test/web_app_icon_test_utils.h" #include "chrome/test/base/in_process_browser_test.h" +#include "components/keep_alive_registry/keep_alive_types.h" +#include "components/keep_alive_registry/scoped_keep_alive.h" #include "content/public/test/browser_test.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/test_utils.h" #include "extensions/browser/extension_dialog_auto_confirm.h" +#include "ui/gfx/native_widget_types.h" using web_app::AppId; @@ -134,6 +137,47 @@ IN_PROC_BROWSER_TEST_F(WebAppUninstallDialogViewBrowserTest, EXPECT_TRUE(was_uninstalled); } +#if BUILDFLAG(IS_MAC) +// https://crbug.com/1224161 +#define MAYBE_UninstallOnCancelShutdownBrowser \ + DISABLED_UninstallOnCancelShutdownBrowser +#else +#define MAYBE_UninstallOnCancelShutdownBrowser UninstallOnCancelShutdownBrowser +#endif +IN_PROC_BROWSER_TEST_F(WebAppUninstallDialogViewBrowserTest, + MAYBE_UninstallOnCancelShutdownBrowser) { + extensions::ScopedTestDialogAutoConfirm auto_confirm( + extensions::ScopedTestDialogAutoConfirm::CANCEL); + AppId app_id = InstallTestWebApp(browser()->profile()); + + std::unique_ptr dialog( + web_app::WebAppUninstallDialog::Create(browser()->profile(), + gfx::kNullNativeWindow)); + + base::RunLoop().RunUntilIdle(); + + base::RunLoop run_loop; + bool was_uninstalled = true; + // ScopedKeepAlive is used by `UninstallWebAppWithDialogFromStartupSwitch`. + // ScopedKeepAlive's destruction triggers browser shutdown when there is no + // open window. This verifies the destruction doesn't race with the dialog + // shutdown itself. + std::unique_ptr scoped_keep_alive = + std::make_unique(KeepAliveOrigin::WEB_APP_UNINSTALL, + KeepAliveRestartOption::DISABLED); + + chrome::CloseWindow(browser()); + + dialog->ConfirmUninstall(app_id, webapps::WebappUninstallSource::kAppMenu, + base::BindLambdaForTesting([&](bool uninstalled) { + was_uninstalled = uninstalled; + scoped_keep_alive.reset(); + run_loop.Quit(); + })); + run_loop.Run(); + EXPECT_FALSE(was_uninstalled); +} + IN_PROC_BROWSER_TEST_F(WebAppUninstallDialogViewBrowserTest, TestDialogUserFlow_Cancel) { extensions::ScopedTestDialogAutoConfirm auto_confirm( diff --git a/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc b/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc index 67ef38f947..3cd064cdab 100644 --- a/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc +++ b/src/chrome/browser/ui/views/web_apps/web_app_uninstall_dialog_view.cc @@ -141,9 +141,7 @@ void WebAppUninstallDialogDelegateView::OnDialogAccepted() { void WebAppUninstallDialogDelegateView::OnDialogCanceled() { UMA_HISTOGRAM_ENUMERATION("Webapp.UninstallDialogAction", HistogramCloseAction::kCancelled); - - if (dialog_) - std::exchange(dialog_, nullptr)->UninstallCancelled(); + // `dialog_->UninstallCancelled()` is handled in the destructor. } gfx::ImageSkia WebAppUninstallDialogDelegateView::GetWindowIcon() { diff --git a/src/chromecast/graphics/accessibility/fullscreen_magnification_controller.cc b/src/chromecast/graphics/accessibility/fullscreen_magnification_controller.cc index 1412290bed..baed03da3b 100644 --- a/src/chromecast/graphics/accessibility/fullscreen_magnification_controller.cc +++ b/src/chromecast/graphics/accessibility/fullscreen_magnification_controller.cc @@ -101,7 +101,10 @@ FullscreenMagnificationController::FullscreenMagnificationController( this, gesture_provider_client_.get()); } -FullscreenMagnificationController::~FullscreenMagnificationController() {} +FullscreenMagnificationController::~FullscreenMagnificationController() { + // Destroy `gesture_provider_` before `gesture_provider_client_`. + gesture_provider_.reset(); +} void FullscreenMagnificationController::SetEnabled(bool enabled) { if (is_enabled_ == enabled) diff --git a/src/components/safe_browsing/core/resources/download_file_types.asciipb b/src/components/safe_browsing/core/resources/download_file_types.asciipb index 114499d5e4..1074b7d091 100644 --- a/src/components/safe_browsing/core/resources/download_file_types.asciipb +++ b/src/components/safe_browsing/core/resources/download_file_types.asciipb @@ -8,7 +8,7 @@ ## ## Top level settings ## -version_id: 43 +version_id: 50 sampled_ping_probability: 0.01 max_archived_binaries_to_report: 10 default_file_type { @@ -1743,7 +1743,16 @@ file_types { auto_open_hint: DISALLOW_AUTO_OPEN } } - +file_types { + extension: "inetloc" + uma_value: 397 + ping_setting: FULL_PING + platform_settings { + platform: PLATFORM_MAC + danger_level: DANGEROUS + auto_open_hint: DISALLOW_AUTO_OPEN + } +} # VBScript files. May open with Windows Script Host and execute with # user privileges. file_types { diff --git a/src/content/browser/child_process_launcher_helper.h b/src/content/browser/child_process_launcher_helper.h index 6e92b30ed6..bdeed89696 100755 --- a/src/content/browser/child_process_launcher_helper.h +++ b/src/content/browser/child_process_launcher_helper.h @@ -48,7 +48,7 @@ #endif #if defined(OS_OHOS) -#include "ohos_adapter_helper.h" +#include "appmgr/app_mgr_client.h" #endif namespace base { @@ -262,7 +262,7 @@ class ChildProcessLauncherHelper : #endif #if defined(OS_OHOS) - std::unique_ptr app_mgr_client_adapter_{nullptr}; + std::unique_ptr app_mgr_client_{nullptr}; #endif }; diff --git a/src/content/browser/child_process_launcher_helper_linux.cc b/src/content/browser/child_process_launcher_helper_linux.cc index 0bdff0732d..4d65ac4f44 100755 --- a/src/content/browser/child_process_launcher_helper_linux.cc +++ b/src/content/browser/child_process_launcher_helper_linux.cc @@ -120,10 +120,10 @@ ChildProcessLauncherHelper::LaunchProcessOnLauncherThread( int32_t shared_fd = options.fds_to_remap[SHARED_FD_INDEX].first; int32_t ipc_fd = options.fds_to_remap[IPC_FD_INDEX].first; pid_t render_pid = 0; - if (app_mgr_client_adapter_ == nullptr) { - app_mgr_client_adapter_ = OHOS::NWeb::OhosAdapterHelper::GetInstance().CreateAafwkAdapter(); + if (app_mgr_client_ == nullptr) { + app_mgr_client_ = std::make_unique(); } - int ret = app_mgr_client_adapter_->StartRenderProcess(argv_ss.str(), ipc_fd, shared_fd, render_pid); + int ret = app_mgr_client_->StartRenderProcess(argv_ss.str(), ipc_fd, shared_fd, render_pid); if (ret != 0) { LOG(ERROR) << "start render process error, ret=" << ret << ", render pid=" << render_pid; process.process = base::Process(); @@ -186,9 +186,9 @@ ChildProcessTerminationInfo ChildProcessLauncherHelper::GetTerminationInfo( info.status = process.zygote->GetTerminationStatus( process.process.Handle(), known_dead, &info.exit_code); #if defined(OS_OHOS) - } else if (app_mgr_client_adapter_) { + } else if (app_mgr_client_) { int exitStatus; - int ret = app_mgr_client_adapter_->GetRenderProcessTerminationStatus(process.process.Handle(), exitStatus); + int ret = app_mgr_client_->GetRenderProcessTerminationStatus(process.process.Handle(), exitStatus); if (ret != 0) { LOG(ERROR) << "get render process termination status failed, ret = " << ret; } else if (exitStatus < 0) { diff --git a/src/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/src/content/browser/devtools/protocol/devtools_protocol_browsertest.cc index dfeab9ee4c..613ae49aed 100644 --- a/src/content/browser/devtools/protocol/devtools_protocol_browsertest.cc +++ b/src/content/browser/devtools/protocol/devtools_protocol_browsertest.cc @@ -472,7 +472,8 @@ class CaptureScreenshotTest : public DevToolsProtocolTest { bool from_surface, const gfx::RectF& clip = gfx::RectF(), float clip_scale = 0, - bool capture_beyond_viewport = false) { + bool capture_beyond_viewport = false, + bool expect_error = false) { std::unique_ptr params(new base::DictionaryValue()); params->SetString("format", encoding == ENCODING_PNG ? "png" : "jpeg"); params->SetInteger("quality", 100); @@ -492,16 +493,24 @@ class CaptureScreenshotTest : public DevToolsProtocolTest { } SendCommand("Page.captureScreenshot", std::move(params)); - std::string base64; - EXPECT_TRUE(result_->GetString("data", &base64)); std::unique_ptr result_bitmap; - if (encoding == ENCODING_PNG) { - result_bitmap.reset(new SkBitmap()); - EXPECT_TRUE(DecodePNG(base64, result_bitmap.get())); + if (expect_error) { + EXPECT_THAT(error_, base::test::DictionaryHasValue( + "code", base::Value(static_cast( + crdtp::DispatchCode::SERVER_ERROR)))); } else { - result_bitmap = DecodeJPEG(base64); + std::string base64; + EXPECT_TRUE(result_->GetString("data", &base64)); + if (encoding == ScreenshotEncoding::ENCODING_PNG) { + result_bitmap = std::make_unique(); + EXPECT_TRUE(DecodePNG(base64, result_bitmap.get())); + } else if (encoding == ScreenshotEncoding::ENCODING_JPEG) { + result_bitmap = DecodeJPEG(base64); + } else { + // Decode not implemented. + } + EXPECT_TRUE(result_bitmap); } - EXPECT_TRUE(result_bitmap); return result_bitmap; } @@ -600,6 +609,10 @@ class CaptureScreenshotTest : public DevToolsProtocolTest { SendCommand("Emulation.clearDeviceMetricsOverride", nullptr); } + bool MayAttachToBrowser() override { return may_attach_to_browser_; } + + bool may_attach_to_browser_ = true; + private: #if !defined(OS_ANDROID) void SetUp() override { @@ -948,6 +961,17 @@ IN_PROC_BROWSER_TEST_F(CaptureScreenshotTest, TransparentScreenshots) { #endif // !defined(OS_ANDROID) } +IN_PROC_BROWSER_TEST_F(CaptureScreenshotTest, + OnlyScreenshotsFromSurfaceWhenUnsafeNotAllowed) { + may_attach_to_browser_ = false; + shell()->LoadURL(GURL("about:blank")); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + Attach(); + + CaptureScreenshot(ScreenshotEncoding::ENCODING_PNG, false, gfx::RectF(), 0, true, + true); +} + #if defined(OS_ANDROID) // Disabled, see http://crbug.com/469947. IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, DISABLED_SynthesizePinchGesture) { diff --git a/src/content/browser/devtools/protocol/page_handler.cc b/src/content/browser/devtools/protocol/page_handler.cc index 554b129494..7b7719648d 100644 --- a/src/content/browser/devtools/protocol/page_handler.cc +++ b/src/content/browser/devtools/protocol/page_handler.cc @@ -184,7 +184,8 @@ bool CanExecuteGlobalCommands( PageHandler::PageHandler(EmulationHandler* emulation_handler, BrowserHandler* browser_handler, - bool allow_file_access) + bool allow_file_access, + bool may_capture_screenshots_not_from_surface) : DevToolsDomainHandler(Page::Metainfo::domainName), enabled_(false), screencast_enabled_(false), @@ -200,7 +201,9 @@ PageHandler::PageHandler(EmulationHandler* emulation_handler, last_surface_size_(gfx::Size()), host_(nullptr), emulation_handler_(emulation_handler), - browser_handler_(browser_handler) { + browser_handler_(browser_handler), + may_capture_screenshots_not_from_surface_( + may_capture_screenshots_not_from_surface) { bool create_video_consumer = true; #ifdef OS_ANDROID // Video capture doesn't work on Android WebView. Use CopyFromSurface instead. @@ -715,6 +718,11 @@ void PageHandler::CaptureScreenshot( // We don't support clip/emulation when capturing from window, bail out. if (!from_surface.fromMaybe(true)) { + if (!may_capture_screenshots_not_from_surface_) { + callback->sendFailure( + Response::ServerError("Only screenshots from surface are allowed.")); + return; + } widget_host->GetSnapshotFromBrowser( base::BindOnce(&PageHandler::ScreenshotCaptured, weak_factory_.GetWeakPtr(), std::move(callback), diff --git a/src/content/browser/devtools/protocol/page_handler.h b/src/content/browser/devtools/protocol/page_handler.h index be667cb7da..e5507b9718 100644 --- a/src/content/browser/devtools/protocol/page_handler.h +++ b/src/content/browser/devtools/protocol/page_handler.h @@ -65,7 +65,8 @@ class PageHandler : public DevToolsDomainHandler, public: PageHandler(EmulationHandler* emulation_handler, BrowserHandler* browser_handler, - bool allow_file_access); + bool allow_file_access, + bool may_capture_screenshots_not_from_surface); ~PageHandler() override; static std::vector EnabledForWebContents( @@ -238,6 +239,7 @@ class PageHandler : public DevToolsDomainHandler, RenderFrameHostImpl* host_; EmulationHandler* emulation_handler_; BrowserHandler* browser_handler_; + const bool may_capture_screenshots_not_from_surface_; std::unique_ptr frontend_; diff --git a/src/content/browser/devtools/render_frame_devtools_agent_host.cc b/src/content/browser/devtools/render_frame_devtools_agent_host.cc index 2bbe13752c..7dc7937b90 100644 --- a/src/content/browser/devtools/render_frame_devtools_agent_host.cc +++ b/src/content/browser/devtools/render_frame_devtools_agent_host.cc @@ -331,7 +331,8 @@ bool RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session, GetId(), GetRendererChannel(), session->GetRootSession())); session->AddHandler(std::make_unique( emulation_handler_ptr, browser_handler_ptr, - session->GetClient()->MayReadLocalFiles())); + session->GetClient()->MayReadLocalFiles(), + session->GetClient()->MayAttachToBrowser())); session->AddHandler(std::make_unique()); if (!frame_tree_node_ || !frame_tree_node_->parent()) { session->AddHandler(std::make_unique( diff --git a/src/content/browser/file_system_access/file_system_access_directory_handle_impl.cc b/src/content/browser/file_system_access/file_system_access_directory_handle_impl.cc index b386fe1dca..19abb01eb5 100644 --- a/src/content/browser/file_system_access/file_system_access_directory_handle_impl.cc +++ b/src/content/browser/file_system_access/file_system_access_directory_handle_impl.cc @@ -409,10 +409,13 @@ namespace { bool IsShellIntegratedExtension(const base::FilePath::StringType& extension) { base::FilePath::StringType extension_lower = base::ToLowerASCII(extension); - // .lnk files may be used to execute arbitrary code (see - // https://nvd.nist.gov/vuln/detail/CVE-2010-2568). - if (extension_lower == FILE_PATH_LITERAL("lnk")) + // .lnk and .scf files may be used to execute arbitrary code (see + // https://nvd.nist.gov/vuln/detail/CVE-2010-2568 and + // https://crbug.com/1227995, respectively). + if (extension_lower == FILE_PATH_LITERAL("lnk") || + extension_lower == FILE_PATH_LITERAL("scf")) { return true; + } // Setting a file's extension to a CLSID may conceal its actual file type on // some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420). diff --git a/src/content/browser/file_system_access/file_system_access_manager_impl.cc b/src/content/browser/file_system_access/file_system_access_manager_impl.cc index a47eceba37..a64d8f47cb 100755 --- a/src/content/browser/file_system_access/file_system_access_manager_impl.cc +++ b/src/content/browser/file_system_access/file_system_access_manager_impl.cc @@ -480,6 +480,16 @@ void FileSystemAccessManagerImpl::SetDefaultPathAndShowPicker( suggested_name, std::string(), std::string()) : base::FilePath(); + auto suggested_extension = suggested_name_path.Extension(); + // Our version of `IsShellIntegratedExtension()` is more stringent than + // the version used in `net::GenerateFileName()`. See + // `FileSystemChooser::IsShellIntegratedExtension()` for details. + if (FileSystemChooser::IsShellIntegratedExtension(suggested_extension)) { + suggested_extension = FILE_PATH_LITERAL("download"); + suggested_name_path = + suggested_name_path.ReplaceExtension(suggested_extension); + } + FileSystemChooser::Options file_system_chooser_options( GetSelectFileDialogType(options), GetAndMoveAcceptsTypesInfo(options), std::move(default_directory), std::move(suggested_name_path)); diff --git a/src/content/browser/file_system_access/file_system_chooser.cc b/src/content/browser/file_system_access/file_system_chooser.cc index fa4c151286..dc02983400 100644 --- a/src/content/browser/file_system_access/file_system_chooser.cc +++ b/src/content/browser/file_system_access/file_system_chooser.cc @@ -71,30 +71,6 @@ base::FilePath::StringType GetLastExtension( : extension; } -// Returns whether the specified extension receives special handling by the -// Windows shell. -bool IsShellIntegratedExtension(const base::FilePath::StringType& extension) { - // TODO(https://crbug.com/1154757): Figure out some way to unify this with - // net::IsSafePortablePathComponent, with the result probably ending up in - // base/i18n/file_util_icu.h. - base::FilePath::StringType extension_lower = base::ToLowerASCII(extension); - - // .lnk files may be used to execute arbitrary code (see - // https://nvd.nist.gov/vuln/detail/CVE-2010-2568). .local files are used by - // Windows to determine which DLLs to load for an application. - if ((extension_lower == FILE_PATH_LITERAL("local")) || - (extension_lower == FILE_PATH_LITERAL("lnk"))) - return true; - - // Setting a file's extension to a CLSID may conceal its actual file type on - // some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420). - if (!extension_lower.empty() && - (extension_lower.front() == FILE_PATH_LITERAL('{')) && - (extension_lower.back() == FILE_PATH_LITERAL('}'))) - return true; - return false; -} - // Extension validation primarily takes place in the renderer. This checks for a // subset of invalid extensions in the event the renderer is compromised. bool IsInvalidExtension(base::FilePath::StringType& extension) { @@ -102,7 +78,7 @@ bool IsInvalidExtension(base::FilePath::StringType& extension) { auto extension16 = base::UTF8ToUTF16(component8.c_str()); return !base::i18n::IsFilenameLegal(extension16) || - IsShellIntegratedExtension(GetLastExtension(extension)); + FileSystemChooser::IsShellIntegratedExtension(extension); } // Converts the accepted mime types and extensions from |option| into a list @@ -289,6 +265,40 @@ void FileSystemChooser::CreateAndShow( /*params=*/nullptr); } +// static +bool FileSystemChooser::IsShellIntegratedExtension( + const base::FilePath::StringType& extension) { + // TODO(https://crbug.com/1154757): Figure out some way to unify this with + // net::IsSafePortablePathComponent, with the result probably ending up in + // base/i18n/file_util_icu.h. + // - For the sake of consistency across platforms, we sanitize '.lnk' and + // '.local' files on all platforms (not just Windows) + // - There are some extensions (i.e. '.scf') we would like to sanitize which + // `net::GenerateFileName()` does not + base::FilePath::StringType extension_lower = + base::ToLowerASCII(GetLastExtension(extension)); + + // .lnk and .scf files may be used to execute arbitrary code (see + // https://nvd.nist.gov/vuln/detail/CVE-2010-2568 and + // https://crbug.com/1227995, respectively). .local files are used by Windows + // to determine which DLLs to load for an application. + if ((extension_lower == FILE_PATH_LITERAL("lnk")) || + (extension_lower == FILE_PATH_LITERAL("local")) || + (extension_lower == FILE_PATH_LITERAL("scf"))) { + return true; + } + + // Setting a file's extension to a CLSID may conceal its actual file type on + // some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420). + if (!extension_lower.empty() && + (extension_lower.front() == FILE_PATH_LITERAL('{')) && + (extension_lower.back() == FILE_PATH_LITERAL('}'))) { + return true; + } + + return false; +} + FileSystemChooser::FileSystemChooser(ui::SelectFileDialog::Type type, ResultCallback callback, base::ScopedClosureRunner fullscreen_block) diff --git a/src/content/browser/file_system_access/file_system_chooser.h b/src/content/browser/file_system_access/file_system_chooser.h index 8b672a2782..fdb6012ca5 100644 --- a/src/content/browser/file_system_access/file_system_chooser.h +++ b/src/content/browser/file_system_access/file_system_chooser.h @@ -68,6 +68,12 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener { ResultCallback callback, base::ScopedClosureRunner fullscreen_block); + // Returns whether the specified extension receives special handling by the + // Windows shell. These extensions should be sanitized before being shown in + // the "save as" file picker. + static bool IsShellIntegratedExtension( + const base::FilePath::StringType& extension); + FileSystemChooser(ui::SelectFileDialog::Type type, ResultCallback callback, base::ScopedClosureRunner fullscreen_block); diff --git a/src/content/browser/file_system_access/file_system_chooser_browsertest.cc b/src/content/browser/file_system_access/file_system_chooser_browsertest.cc index 72ff09ab09..f65ef61e4c 100644 --- a/src/content/browser/file_system_access/file_system_chooser_browsertest.cc +++ b/src/content/browser/file_system_access/file_system_chooser_browsertest.cc @@ -1321,22 +1321,28 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, SuggestedName) { name_infos.push_back({"not_matching.jpg", ListValueOf(".txt"), false, "not_matching.jpg", false}); -#if defined(OS_WIN) - // ".local" and ".lnk" extensions should be sanitized on Windows. + // ".lnk", ".local", and ".scf" extensions should be sanitized. name_infos.push_back({"dangerous_extension.local", ListValueOf(".local"), true, "dangerous_extension.download", false}); name_infos.push_back({"dangerous_extension.lnk", ListValueOf(".lnk"), true, "dangerous_extension.download", false}); -#else - // ".local" and ".lnk" extensions should be allowed on other OSes. - // TODO(https://crbug.com/1154757): `expected_exclude_accept_all_option` is - // false here because ".local" and ".lnk" extensions are not allowed in - // `accepts`, but are only sanitized by net::GenerateSafeFileName on Windows. - name_infos.push_back({"dangerous_extension.local", ListValueOf(".local"), - true, "dangerous_extension.local", false}); - name_infos.push_back({"dangerous_extension.lnk", ListValueOf(".lnk"), true, - "dangerous_extension.lnk", false}); -#endif + name_infos.push_back({"dangerous_extension.scf", ListValueOf(".scf"), true, + "dangerous_extension.download", false}); + // Compound extensions ending in a dangerous extension should be sanitized. + name_infos.push_back({"dangerous_extension.png.local", ListValueOf(".local"), + true, "dangerous_extension.png.download", false}); + name_infos.push_back({"dangerous_extension.png.lnk", ListValueOf(".lnk"), + true, "dangerous_extension.png.download", false}); + name_infos.push_back({"dangerous_extension.png.scf", ListValueOf(".scf"), + true, "dangerous_extension.png.download", false}); + // Compound extensions not ending in a dangerous extension should not be + // sanitized. + name_infos.push_back({"dangerous_extension.local.png", ListValueOf(".png"), + true, "dangerous_extension.local.png", true}); + name_infos.push_back({"dangerous_extension.lnk.png", ListValueOf(".png"), + true, "dangerous_extension.lnk.png", true}); + name_infos.push_back({"dangerous_extension.scf.png", ListValueOf(".png"), + true, "dangerous_extension.scf.png", true}); // Invalid characters should be sanitized. name_infos.push_back({R"(inv*l:d\\charבאמת!a({}), std::vector( - {"lnk", "foo.lnk", "foo.bar.local", "text", "local"}))); + {"lnk", "foo.lnk", "foo.bar.local", "text", "local", "scf"}))); SyncShowDialog(std::move(accepts), /*include_accepts_all=*/false); ASSERT_TRUE(dialog_params.file_types); diff --git a/src/content/renderer/BUILD.gn b/src/content/renderer/BUILD.gn index 810393cab2..9325b96383 100755 --- a/src/content/renderer/BUILD.gn +++ b/src/content/renderer/BUILD.gn @@ -23,7 +23,16 @@ if (is_ohos) { import("//build/config/ohos/config.gni") config("ohos_system_libs") { libs = [ - "nweb_ohos_adapter.z" + "hilog", + "utils.z", + "eventhandler.z", + "app_manager.z", + "base.z", + "want.z", + "appexecfwk_common.z", + "appexecfwk_base.z", + "samgr_proxy.z", + "ipc_core.z", ] include_dirs = ohos_src_includes lib_dirs = ohos_libs_dir diff --git a/src/content/renderer/render_remote_proxy.cc b/src/content/renderer/render_remote_proxy.cc index 1d0a8b3697..858d5e4c01 100755 --- a/src/content/renderer/render_remote_proxy.cc +++ b/src/content/renderer/render_remote_proxy.cc @@ -5,15 +5,16 @@ #include "render_remote_proxy.h" #include +#include "appmgr/app_mgr_client.h" #include "base/command_line.h" #include "base/posix/global_descriptors.h" #include "content/public/common/content_descriptors.h" #include "content/public/common/content_switches.h" -#include "ohos_adapter_helper.h" +#include "refbase.h" namespace content { -std::unique_ptr g_app_mgr_client_adapter{nullptr}; -std::shared_ptr g_render_remote_proxy{nullptr}; +std::unique_ptr g_app_mgr_client{nullptr}; +OHOS::sptr g_render_remote_proxy{nullptr}; std::mutex RenderRemoteProxy::browser_fd_mtx_; std::condition_variable RenderRemoteProxy::browser_fd_cv_; @@ -52,9 +53,9 @@ void RenderRemoteProxy::NotifyBrowserFd(int32_t ipcFd, int32_t sharedFd) { void RenderRemoteProxy::CreateAndRegist(const base::CommandLine& command_line) { is_for_test_ = command_line.HasSwitch(switches::kForTest); if (!is_for_test_) { - g_app_mgr_client_adapter = OHOS::NWeb::OhosAdapterHelper::GetInstance().CreateAafwkAdapter(); - g_render_remote_proxy = std::make_shared(); - g_app_mgr_client_adapter->AttachRenderProcess(g_render_remote_proxy); + g_app_mgr_client = std::make_unique(); + g_render_remote_proxy = new RenderRemoteProxy(); + g_app_mgr_client->AttachRenderProcess(g_render_remote_proxy); } } @@ -71,7 +72,7 @@ bool RenderRemoteProxy::WaitForBrowserFd() { if (!browser_fd_cv_.wait_for(lk, std::chrono::milliseconds(kTimeOutDur), []() { return is_browser_fd_received_; })) { LOG(INFO) << "retry AttachRenderProcess for " << wait_count << "time"; - g_app_mgr_client_adapter->AttachRenderProcess(g_render_remote_proxy); + g_app_mgr_client->AttachRenderProcess(g_render_remote_proxy); } else { LOG(INFO) << "success, wait for browser fd end"; return true; diff --git a/src/content/renderer/render_remote_proxy.h b/src/content/renderer/render_remote_proxy.h index c6e3b98b25..1127280fbf 100755 --- a/src/content/renderer/render_remote_proxy.h +++ b/src/content/renderer/render_remote_proxy.h @@ -7,14 +7,14 @@ #include #include -#include "aafwk_render_scheduler_host_adapter.h" +#include "appmgr/render_scheduler_host.h" namespace base { class CommandLine; } namespace content { -class RenderRemoteProxy : public OHOS::NWeb::AafwkRenderSchedulerHostAdapter { +class RenderRemoteProxy : public OHOS::AppExecFwk::RenderSchedulerHost { public: RenderRemoteProxy() = default; ~RenderRemoteProxy() = default; diff --git a/src/ohos_nweb/BUILD.gn b/src/ohos_nweb/BUILD.gn index 1fd2aea892..c8b528bdb1 100644 --- a/src/ohos_nweb/BUILD.gn +++ b/src/ohos_nweb/BUILD.gn @@ -64,7 +64,6 @@ component("cef_nweb") { sources = [ "src/cef_delegate/nweb_application.h", "src/cef_delegate/nweb_application.cc", - "src/cef_delegate/nweb_input_delegate.cc", "src/cef_delegate/nweb_handler_delegate.h", "src/cef_delegate/nweb_handler_delegate.cc", "src/cef_delegate/nweb_delegate.cc", diff --git a/src/ohos_nweb/include/nweb.h b/src/ohos_nweb/include/nweb.h index 8c93edb7ba..ebc8a83f55 100755 --- a/src/ohos_nweb/include/nweb.h +++ b/src/ohos_nweb/include/nweb.h @@ -73,7 +73,6 @@ class OHOS_NWEB_EXPORT NWeb : public std::enable_shared_from_this { virtual void OnTouchMove(int32_t id, double x, double y) = 0; virtual void OnTouchCancel() = 0; virtual void OnNavigateBack() = 0; - virtual bool SendKeyEvent(int32_t keyCode, int32_t keyAction) = 0; /** * Loads the given URL. diff --git a/src/ohos_nweb/src/cef_delegate/nweb_delegate.cc b/src/ohos_nweb/src/cef_delegate/nweb_delegate.cc index 4a47e77222..266d00d842 100755 --- a/src/ohos_nweb/src/cef_delegate/nweb_delegate.cc +++ b/src/ohos_nweb/src/cef_delegate/nweb_delegate.cc @@ -147,14 +147,6 @@ void NWebDelegate::OnTouchCancel() { } } -bool NWebDelegate::SendKeyEvent(int32_t keyCode, int32_t keyAction) { - bool retVal = false; - if (event_handler_ != nullptr) { - retVal = event_handler_->SendKeyEvent(keyCode, keyAction); - } - return retVal; -} - std::shared_ptr NWebDelegate::GetPreference() const { return preference_delegate_; } diff --git a/src/ohos_nweb/src/cef_delegate/nweb_delegate.h b/src/ohos_nweb/src/cef_delegate/nweb_delegate.h index 63ad48c690..d1792e1e01 100755 --- a/src/ohos_nweb/src/cef_delegate/nweb_delegate.h +++ b/src/ohos_nweb/src/cef_delegate/nweb_delegate.h @@ -51,7 +51,6 @@ class NWebDelegate : public NWebDelegateInterface, void OnTouchRelease(int32_t id, double x, double y) override; void OnTouchMove(int32_t id, double x, double y) override; void OnTouchCancel() override; - bool SendKeyEvent(int32_t keyCode, int32_t keyAction) override; void Load(const std::string& url) override; bool IsNavigatebackwardAllowed() const override; diff --git a/src/ohos_nweb/src/cef_delegate/nweb_event_handler.cc b/src/ohos_nweb/src/cef_delegate/nweb_event_handler.cc index 99d1b5f0e7..ced49179e0 100644 --- a/src/ohos_nweb/src/cef_delegate/nweb_event_handler.cc +++ b/src/ohos_nweb/src/cef_delegate/nweb_event_handler.cc @@ -82,16 +82,4 @@ void NWebEventHandler::OnTouchCancel() { browser_->GetHost()->SendTouchEvent(touch_cancelled); } } - -bool NWebEventHandler::SendKeyEvent(int32_t keyCode, int32_t keyAction) { - CefKeyEvent keyEvent; - input_delegate_.SetModifiers(keyCode, keyAction); - keyEvent.windows_key_code = NWebInputDelegate::CefConverter("keycode", keyCode); - keyEvent.type = static_cast(NWebInputDelegate::CefConverter("keyaction", keyAction)); - keyEvent.modifiers = input_delegate_.GetModifiers(); - if (browser_ && browser_->GetHost()) { - browser_->GetHost()->SendKeyEvent(keyEvent); - } - return false; -} } // namespace OHOS::NWeb diff --git a/src/ohos_nweb/src/cef_delegate/nweb_event_handler.h b/src/ohos_nweb/src/cef_delegate/nweb_event_handler.h index 0ef2e92f3e..1ca96dc201 100644 --- a/src/ohos_nweb/src/cef_delegate/nweb_event_handler.h +++ b/src/ohos_nweb/src/cef_delegate/nweb_event_handler.h @@ -17,7 +17,6 @@ #define NWEB_EVENT_HANDLER_H #include "cef/include/cef_client.h" -#include "nweb_input_delegate.h" namespace OHOS::NWeb { class NWebEventHandler { @@ -35,11 +34,9 @@ class NWebEventHandler { void OnTouchRelease(int32_t id, double x, double y); void OnTouchCancel(); void OnKeyBack(); - bool SendKeyEvent(int32_t keyCode, int32_t keyAction); private: CefRefPtr browser_ = nullptr; - NWebInputDelegate input_delegate_; }; } // namespace OHOS::NWeb diff --git a/src/ohos_nweb/src/cef_delegate/nweb_handler_delegate.cc b/src/ohos_nweb/src/cef_delegate/nweb_handler_delegate.cc index 04dc120990..65a577d0c8 100755 --- a/src/ohos_nweb/src/cef_delegate/nweb_handler_delegate.cc +++ b/src/ohos_nweb/src/cef_delegate/nweb_handler_delegate.cc @@ -541,8 +541,7 @@ void NWebHandlerDelegate::OnBeforeDownload( download_item->GetTotalBytes()); } } - -/* CefKeyboardHandler methods end */ +/* CefDownloadHandler methods end */ /* CefResourceRequestHandler method begin */ CefResourceRequestHandler::ReturnValue diff --git a/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.cc b/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.cc deleted file mode 100644 index 50b80661fd..0000000000 --- a/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.cc +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright (c) 2022 Huawei Device Co., Ltd. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "nweb_input_delegate.h" -#include "cef/include/internal/cef_types_wrappers.h" - -using namespace OHOS::NWeb; -const int32_t KEY_DOWN = 0; -const int32_t KEY_UP = 1; - -const std::unordered_mapkeycodeConverter = { - {OHOS::MMI::KeyEvent::KEYCODE_0, ui::VKEY_0}, - {OHOS::MMI::KeyEvent::KEYCODE_1, ui::VKEY_1}, - {OHOS::MMI::KeyEvent::KEYCODE_2, ui::VKEY_2}, - {OHOS::MMI::KeyEvent::KEYCODE_3, ui::VKEY_3}, - {OHOS::MMI::KeyEvent::KEYCODE_4, ui::VKEY_4}, - {OHOS::MMI::KeyEvent::KEYCODE_5, ui::VKEY_5}, - {OHOS::MMI::KeyEvent::KEYCODE_6, ui::VKEY_6}, - {OHOS::MMI::KeyEvent::KEYCODE_7, ui::VKEY_7}, - {OHOS::MMI::KeyEvent::KEYCODE_8, ui::VKEY_8}, - {OHOS::MMI::KeyEvent::KEYCODE_9, ui::VKEY_9}, - {OHOS::MMI::KeyEvent::KEYCODE_A, ui::VKEY_A}, - {OHOS::MMI::KeyEvent::KEYCODE_B, ui::VKEY_B}, - {OHOS::MMI::KeyEvent::KEYCODE_C, ui::VKEY_C}, - {OHOS::MMI::KeyEvent::KEYCODE_D, ui::VKEY_D}, - {OHOS::MMI::KeyEvent::KEYCODE_E, ui::VKEY_E}, - {OHOS::MMI::KeyEvent::KEYCODE_F, ui::VKEY_F}, - {OHOS::MMI::KeyEvent::KEYCODE_G, ui::VKEY_G}, - {OHOS::MMI::KeyEvent::KEYCODE_H, ui::VKEY_H}, - {OHOS::MMI::KeyEvent::KEYCODE_I, ui::VKEY_I}, - {OHOS::MMI::KeyEvent::KEYCODE_J, ui::VKEY_J}, - {OHOS::MMI::KeyEvent::KEYCODE_K, ui::VKEY_K}, - {OHOS::MMI::KeyEvent::KEYCODE_L, ui::VKEY_L}, - {OHOS::MMI::KeyEvent::KEYCODE_M, ui::VKEY_M}, - {OHOS::MMI::KeyEvent::KEYCODE_N, ui::VKEY_N}, - {OHOS::MMI::KeyEvent::KEYCODE_O, ui::VKEY_O}, - {OHOS::MMI::KeyEvent::KEYCODE_P, ui::VKEY_P}, - {OHOS::MMI::KeyEvent::KEYCODE_Q, ui::VKEY_Q}, - {OHOS::MMI::KeyEvent::KEYCODE_R, ui::VKEY_R}, - {OHOS::MMI::KeyEvent::KEYCODE_S, ui::VKEY_S}, - {OHOS::MMI::KeyEvent::KEYCODE_T, ui::VKEY_T}, - {OHOS::MMI::KeyEvent::KEYCODE_U, ui::VKEY_U}, - {OHOS::MMI::KeyEvent::KEYCODE_V, ui::VKEY_V}, - {OHOS::MMI::KeyEvent::KEYCODE_W, ui::VKEY_W}, - {OHOS::MMI::KeyEvent::KEYCODE_X, ui::VKEY_X}, - {OHOS::MMI::KeyEvent::KEYCODE_Y, ui::VKEY_Y}, - {OHOS::MMI::KeyEvent::KEYCODE_Z, ui::VKEY_Z}, - {OHOS::MMI::KeyEvent::KEYCODE_SHIFT_LEFT, ui::VKEY_SHIFT}, - {OHOS::MMI::KeyEvent::KEYCODE_SHIFT_RIGHT, ui::VKEY_SHIFT}, - {OHOS::MMI::KeyEvent::KEYCODE_TAB, ui::VKEY_TAB}, - {OHOS::MMI::KeyEvent::KEYCODE_SPACE, ui::VKEY_SPACE}, - {OHOS::MMI::KeyEvent::KEYCODE_ENTER, ui::VKEY_RETURN}, - {OHOS::MMI::KeyEvent::KEYCODE_DEL, ui::VKEY_DELETE}, - {OHOS::MMI::KeyEvent::KEYCODE_CTRL_LEFT, ui::VKEY_CONTROL}, - {OHOS::MMI::KeyEvent::KEYCODE_CTRL_RIGHT, ui::VKEY_CONTROL}, - {OHOS::MMI::KeyEvent::KEYCODE_F1, ui::VKEY_F1}, - {OHOS::MMI::KeyEvent::KEYCODE_F2, ui::VKEY_F2}, - {OHOS::MMI::KeyEvent::KEYCODE_F3, ui::VKEY_F3}, - {OHOS::MMI::KeyEvent::KEYCODE_F4, ui::VKEY_F4}, - {OHOS::MMI::KeyEvent::KEYCODE_F5, ui::VKEY_F5}, - {OHOS::MMI::KeyEvent::KEYCODE_F6, ui::VKEY_F6}, - {OHOS::MMI::KeyEvent::KEYCODE_F7, ui::VKEY_F7}, - {OHOS::MMI::KeyEvent::KEYCODE_F8, ui::VKEY_F8}, - {OHOS::MMI::KeyEvent::KEYCODE_F9, ui::VKEY_F9}, - {OHOS::MMI::KeyEvent::KEYCODE_F10, ui::VKEY_F10}, - {OHOS::MMI::KeyEvent::KEYCODE_F11, ui::VKEY_F11}, - {OHOS::MMI::KeyEvent::KEYCODE_F12, ui::VKEY_F12}, -}; - -const std::unordered_map keyactionConverter = { - {KEY_UP, KEYEVENT_KEYUP}, - {KEY_DOWN, KEYEVENT_KEYDOWN}, -}; - -const std::unordered_map> keyValueConverter = { - {"keycode", keycodeConverter}, - {"keyaction", keyactionConverter}, -}; - -bool NWebInputDelegate::KeyValueConvert(const std::string keyValue, std::unordered_map& map) { - auto itKeyValue = keyValueConverter.find(keyValue); - if (itKeyValue == keyValueConverter.end()) { - return false; - } - map = itKeyValue->second; - return true; -} - -int NWebInputDelegate::CefConverter(const std::string keyValue, int input) { - std::unordered_map itConverter; - if (KeyValueConvert(keyValue, itConverter) == false) { - return -1; - } - auto item = itConverter.find(input); - if (item == itConverter.end()) { - return -1; - } - return item->second; -} - -int NWebInputDelegate::OhosConverter(const std::string keyValue, int input) { - std::unordered_map itConverter; - if (KeyValueConvert(keyValue, itConverter) == false) { - return -1; - } - auto item = itConverter.find(input); - if (item == itConverter.end()) { - return -1; - } - return item->first; -} - -void NWebInputDelegate::SetModifiers(int keyCode, int keyAction) -{ - if ((keyCode == OHOS::MMI::KeyEvent::KEYCODE_CTRL_LEFT || - keyCode == OHOS::MMI::KeyEvent::KEYCODE_CTRL_RIGHT) && - keyCTL_ == true && keyAction == KEY_UP) { - keyCTL_ = false; - } - - if ((keyCode == OHOS::MMI::KeyEvent::KEYCODE_CTRL_LEFT || - keyCode == OHOS::MMI::KeyEvent::KEYCODE_CTRL_RIGHT) && - keyCTL_ == false && keyAction == KEY_DOWN) { - keyCTL_ = true; - } -} - -unsigned int NWebInputDelegate::GetModifiers() -{ - if (keyCTL_ == true) { - return (0 | EVENTFLAG_CONTROL_DOWN); - } - return 0; -} \ No newline at end of file diff --git a/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.h b/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.h deleted file mode 100644 index da5b1e1c26..0000000000 --- a/src/ohos_nweb/src/cef_delegate/nweb_input_delegate.h +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (c) 2022 Huawei Device Co., Ltd. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef NWEB_KEYCODE_INTERFACE_H -#define NWEB_KEYCODE_INTERFACE_H - -#include -#include "key_event.h" -#include "ui/events/keycodes/keyboard_codes_posix.h" - -namespace OHOS::NWeb { -class NWebInputDelegate { - public: - virtual ~NWebInputDelegate() = default; - static int CefConverter(const std::string keyValue, int input); - static int OhosConverter(const std::string keyValue, int input); - void SetModifiers(int keyCode, int keyAction); - unsigned int GetModifiers(); - - private: - static bool KeyValueConvert(const std::string keyValue, std::unordered_map& map); - bool keyCTL_ = false; -}; -} // namespace OHOS::NWeb - -#endif \ No newline at end of file diff --git a/src/ohos_nweb/src/nweb_delegate_interface.h b/src/ohos_nweb/src/nweb_delegate_interface.h index f3f659e2a7..487de47028 100755 --- a/src/ohos_nweb/src/nweb_delegate_interface.h +++ b/src/ohos_nweb/src/nweb_delegate_interface.h @@ -43,7 +43,6 @@ class NWebDelegateInterface { virtual void OnTouchRelease(int32_t id, double x, double y) = 0; virtual void OnTouchMove(int32_t id, double x, double y) = 0; virtual void OnTouchCancel() = 0; - virtual bool SendKeyEvent(int32_t keyCode, int32_t keyAction) = 0; virtual void Load(const std::string& url) = 0; virtual bool IsNavigatebackwardAllowed() const = 0; diff --git a/src/ohos_nweb/src/nweb_impl.cc b/src/ohos_nweb/src/nweb_impl.cc index 60804d8523..3ca196269d 100755 --- a/src/ohos_nweb/src/nweb_impl.cc +++ b/src/ohos_nweb/src/nweb_impl.cc @@ -159,9 +159,7 @@ void NWebImpl::InitWebEngineArgs(const NWebInitArgs& init_args) { web_engine_args_.emplace_back("/system/bin/web_render"); web_engine_args_.emplace_back("--in-process-gpu"); web_engine_args_.emplace_back("--disable-dev-shm-usage"); -#ifdef GPU_RK3568 web_engine_args_.emplace_back("--disable-gpu"); -#endif web_engine_args_.emplace_back("--no-unsandboxed-zygote"); web_engine_args_.emplace_back("--no-zygote"); web_engine_args_.emplace_back("--off-screen-frame-rate=60"); @@ -252,13 +250,6 @@ void NWebImpl::OnNavigateBack() { input_handler_->OnNavigateBack(); } -bool NWebImpl::SendKeyEvent(int32_t keyCode, int32_t keyAction) { - if (input_handler_ == nullptr) { - return false; - } - return input_handler_->SendKeyEvent(keyCode, keyAction); -} - void NWebImpl::Load(const std::string& url) const { if (nweb_delegate_ == nullptr || output_handler_ == nullptr) { return; diff --git a/src/ohos_nweb/src/nweb_impl.h b/src/ohos_nweb/src/nweb_impl.h index 65daaffe87..d752003c9a 100755 --- a/src/ohos_nweb/src/nweb_impl.h +++ b/src/ohos_nweb/src/nweb_impl.h @@ -43,7 +43,6 @@ class NWebImpl : public NWeb { void OnTouchMove(int32_t id, double x, double y) override; void OnTouchCancel() override; void OnNavigateBack() override; - bool SendKeyEvent(int32_t keyCode, int32_t keyAction) override; // public api void Load(const std::string& url) const override; diff --git a/src/ohos_nweb/src/nweb_input_handler.cc b/src/ohos_nweb/src/nweb_input_handler.cc index 0708fb360e..247d2fab7d 100644 --- a/src/ohos_nweb/src/nweb_input_handler.cc +++ b/src/ohos_nweb/src/nweb_input_handler.cc @@ -94,13 +94,6 @@ void NWebInputHandler::OnNavigateBack() { } } -bool NWebInputHandler::SendKeyEvent(int32_t keyCode, int32_t keyAction) { - if (nweb_delegate_ == nullptr) { - return false; - } - return nweb_delegate_->SendKeyEvent(keyCode, keyAction); -} - void NWebInputHandler::CheckSlideNavigation(int16_t start_x, int16_t end_x) { if (nweb_delegate_ == nullptr) { return; diff --git a/src/ohos_nweb/src/nweb_input_handler.h b/src/ohos_nweb/src/nweb_input_handler.h index 3351edbf94..802229b4b0 100644 --- a/src/ohos_nweb/src/nweb_input_handler.h +++ b/src/ohos_nweb/src/nweb_input_handler.h @@ -39,7 +39,6 @@ class NWebInputHandler { void OnTouchMove(int32_t id, double x, double y); void OnTouchCancel(); void OnNavigateBack(); - bool SendKeyEvent(int32_t keyCode, int32_t keyAction); private: void CheckSlideNavigation(int16_t start_x, int16_t end_x); diff --git a/src/third_party/blink/renderer/core/inspector/inspect_tools.cc b/src/third_party/blink/renderer/core/inspector/inspect_tools.cc index 937955cb02..474cd28fd6 100644 --- a/src/third_party/blink/renderer/core/inspector/inspect_tools.cc +++ b/src/third_party/blink/renderer/core/inspector/inspect_tools.cc @@ -436,16 +436,11 @@ bool PersistentTool::IsEmpty() { return !grid_node_highlights_.size() && !flex_container_configs_.size(); } -void PersistentTool::SetGridConfigs( - Vector, - std::unique_ptr>> configs) { +void PersistentTool::SetGridConfigs(GirdConfigs configs) { grid_node_highlights_ = std::move(configs); } -void PersistentTool::SetFlexContainerConfigs( - Vector, - std::unique_ptr>> - configs) { +void PersistentTool::SetFlexContainerConfigs(FlexContainerConfigs configs) { flex_container_configs_ = std::move(configs); } @@ -464,14 +459,14 @@ bool PersistentTool::HideOnMouseMove() { void PersistentTool::Draw(float scale) { for (auto& entry : grid_node_highlights_) { std::unique_ptr highlight = - InspectorGridHighlight(entry.first.Get(), *(entry.second)); + InspectorGridHighlight(entry.key, *(entry.value)); if (!highlight) continue; overlay_->EvaluateInOverlay("drawGridHighlight", std::move(highlight)); } for (auto& entry : flex_container_configs_) { std::unique_ptr highlight = - InspectorFlexContainerHighlight(entry.first.Get(), *(entry.second)); + InspectorFlexContainerHighlight(entry.key, *(entry.value)); if (!highlight) continue; overlay_->EvaluateInOverlay("drawFlexContainerHighlight", @@ -485,7 +480,7 @@ PersistentTool::GetGridInspectorHighlightsAsJson() const { protocol::ListValue::create(); for (auto& entry : grid_node_highlights_) { std::unique_ptr highlight = - InspectorGridHighlight(entry.first.Get(), *(entry.second)); + InspectorGridHighlight(entry.key, *(entry.value)); if (!highlight) continue; highlights->pushValue(std::move(highlight)); @@ -498,6 +493,12 @@ PersistentTool::GetGridInspectorHighlightsAsJson() const { return result; } +void PersistentTool::Trace(Visitor* visitor) const { + InspectTool::Trace(visitor); + visitor->Trace(grid_node_highlights_); + visitor->Trace(flex_container_configs_); +} + // SourceOrderTool ----------------------------------------------------------- SourceOrderTool::SourceOrderTool( diff --git a/src/third_party/blink/renderer/core/inspector/inspect_tools.h b/src/third_party/blink/renderer/core/inspector/inspect_tools.h index bd16149f25..095843a9d9 100644 --- a/src/third_party/blink/renderer/core/inspector/inspect_tools.h +++ b/src/third_party/blink/renderer/core/inspector/inspect_tools.h @@ -142,11 +142,11 @@ class SourceOrderTool : public InspectTool { // ----------------------------------------------------------------------------- -using GirdConfigs = Vector< - std::pair, std::unique_ptr>>; +using GirdConfigs = HeapHashMap, + std::unique_ptr>; using FlexContainerConfigs = - Vector, - std::unique_ptr>>; + HeapHashMap, + std::unique_ptr>; class PersistentTool : public InspectTool { using InspectTool::InspectTool; @@ -159,6 +159,8 @@ class PersistentTool : public InspectTool { std::unique_ptr GetGridInspectorHighlightsAsJson() const; + void Trace(Visitor* visitor) const override; + private: bool ForwardEventsToOverlay() override; bool HideOnMouseMove() override; diff --git a/src/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc b/src/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc index a0ea77ccff..d911642214 100644 --- a/src/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc +++ b/src/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc @@ -700,7 +700,7 @@ Response InspectorOverlayAgent::setShowGridOverlays( MakeGarbageCollected(this, GetFrontend()); } - Vector, std::unique_ptr>> + HeapHashMap, std::unique_ptr> configs; for (std::unique_ptr& config : *grid_node_highlight_configs) { @@ -708,9 +708,8 @@ Response InspectorOverlayAgent::setShowGridOverlays( Response response = dom_agent_->AssertNode(config->getNodeId(), node); if (!response.IsSuccess()) return response; - configs.push_back( - std::make_pair(node, InspectorOverlayAgent::ToGridHighlightConfig( - config->getGridHighlightConfig()))); + configs.insert(node, InspectorOverlayAgent::ToGridHighlightConfig( + config->getGridHighlightConfig())); } persistent_tool_->SetGridConfigs(std::move(configs)); @@ -728,8 +727,8 @@ Response InspectorOverlayAgent::setShowFlexOverlays( MakeGarbageCollected(this, GetFrontend()); } - Vector, - std::unique_ptr>> + HeapHashMap, + std::unique_ptr> configs; for (std::unique_ptr& config : @@ -738,9 +737,8 @@ Response InspectorOverlayAgent::setShowFlexOverlays( Response response = dom_agent_->AssertNode(config->getNodeId(), node); if (!response.IsSuccess()) return response; - configs.push_back(std::make_pair( - node, InspectorOverlayAgent::ToFlexContainerHighlightConfig( - config->getFlexContainerHighlightConfig()))); + configs.insert(node, InspectorOverlayAgent::ToFlexContainerHighlightConfig( + config->getFlexContainerHighlightConfig())); } persistent_tool_->SetFlexContainerConfigs(std::move(configs)); @@ -844,16 +842,16 @@ Response InspectorOverlayAgent::getGridHighlightObjectsForTest( std::unique_ptr> node_ids, std::unique_ptr* highlights) { PersistentTool persistent_tool(this, GetFrontend()); - Vector, std::unique_ptr>> + + HeapHashMap, std::unique_ptr> configs; for (const int node_id : *node_ids) { Node* node = nullptr; Response response = dom_agent_->AssertNode(node_id, node); if (!response.IsSuccess()) return response; - configs.push_back( - std::make_pair(node, std::make_unique( - InspectorHighlight::DefaultGridConfig()))); + configs.insert(node, std::make_unique( + InspectorHighlight::DefaultGridConfig())); } persistent_tool.SetGridConfigs(std::move(configs)); *highlights = persistent_tool.GetGridInspectorHighlightsAsJson(); diff --git a/src/tools/metrics/histograms/enums.xml b/src/tools/metrics/histograms/enums.xml index 808a241114..4af899b058 100644 --- a/src/tools/metrics/histograms/enums.xml +++ b/src/tools/metrics/histograms/enums.xml @@ -68571,6 +68571,7 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf + diff --git a/src/ui/aura/gestures/gesture_recognizer_unittest.cc b/src/ui/aura/gestures/gesture_recognizer_unittest.cc index 40ad046607..fafd5cb609 100644 --- a/src/ui/aura/gestures/gesture_recognizer_unittest.cc +++ b/src/ui/aura/gestures/gesture_recognizer_unittest.cc @@ -29,6 +29,7 @@ #include "ui/events/event_utils.h" #include "ui/events/gesture_detection/gesture_configuration.h" #include "ui/events/gesture_detection/gesture_provider.h" +#include "ui/events/gestures/gesture_recognizer_impl.h" #include "ui/events/gestures/gesture_types.h" #include "ui/events/test/event_generator.h" #include "ui/events/test/events_test_utils.h" @@ -1737,6 +1738,75 @@ TEST_F(GestureRecognizerTest, GestureTapFollowedByScroll) { EXPECT_TRUE(delegate->fling()); } +// Verifies that destroying a gesture provider aura instance before a touch +// event is ACKed works as expected (see https://crbug.com/1292264). +TEST_F(GestureRecognizerTest, DestroyGestureProviderAuraBeforeAck) { + TimedEvents tes; + const int kTouchId = 4; + std::unique_ptr delegate( + new GestureEventConsumeDelegate()); + std::unique_ptr window1(CreateTestWindowWithDelegate( + delegate.get(), /*id=*/-2345, /*bounds=*/gfx::Rect(0, 0, 50, 50), + /*parent=*/root_window())); + + // Touch press then release on `window1`. + constexpr gfx::Point touch_location(/*x=*/10, /*y=*/20); + ui::TouchEvent press( + ui::ET_TOUCH_PRESSED, touch_location, /*time_stamp=*/tes.Now(), + ui::PointerDetails(ui::EventPointerType::kTouch, kTouchId)); + delegate->Reset(); + DispatchEventUsingWindowDispatcher(&press); + EXPECT_TRUE(delegate->tap_down()); + delegate->Reset(); + ui::TouchEvent release( + ui::ET_TOUCH_RELEASED, touch_location, + /*time_stamp=*/press.time_stamp() + base::Milliseconds(50), + ui::PointerDetails(ui::EventPointerType::kTouch, kTouchId)); + DispatchEventUsingWindowDispatcher(&release); + EXPECT_FALSE(delegate->tap_down()); + + // Verify that the gesture provider for `window1` is created. + auto* gesture_recognizer = static_cast( + aura::Env::GetInstance()->gesture_recognizer()); + const auto& consumer_provider_mappings = + gesture_recognizer->consumer_gesture_provider_; + EXPECT_NE(consumer_provider_mappings.cend(), + consumer_provider_mappings.find(window1.get())); + + // Create a second window for handling touch events. + std::unique_ptr delegate2( + new QueueTouchEventDelegate(host()->dispatcher())); + const int kTouchId2 = 4; + std::unique_ptr window2(CreateTestWindowWithDelegate( + delegate2.get(), /*id=*/-1234, /*bounds=*/gfx::Rect(100, 100, 500, 500), + root_window())); + delegate2->set_window(window2.get()); + + // Send a press event on `window2`. Verify that the gesture provider for + // `window2` is created. + ui::TouchEvent press2( + ui::ET_TOUCH_PRESSED, /*location=*/gfx::Point(200, 200), + /*time_stamp=*/tes.Now(), + ui::PointerDetails(ui::EventPointerType::kTouch, kTouchId2)); + DispatchEventUsingWindowDispatcher(&press2); + EXPECT_NE(consumer_provider_mappings.cend(), + consumer_provider_mappings.find(window2.get())); + + // Verify that `press2` is associated with a gesture provider raw pointer. + const auto& event_provider_mappings = + gesture_recognizer->event_to_gesture_provider_; + EXPECT_NE(event_provider_mappings.cend(), + event_provider_mappings.find(press2.unique_event_id())); + + // Before ACKing `press2`, replacing the gesture provider of `window2` with a + // new value through event transferal. + aura::Env::GetInstance()->gesture_recognizer()->TransferEventsTo( + window1.get(), window2.get(), ui::TransferTouchesBehavior::kCancel); + + // ACK the press event. + delegate2->ReceivedAck(); +} + TEST_F(GestureRecognizerTest, AsynchronousGestureRecognition) { std::unique_ptr queued_delegate( new QueueTouchEventDelegate(host()->dispatcher())); diff --git a/src/ui/events/gestures/gesture_provider_aura.cc b/src/ui/events/gestures/gesture_provider_aura.cc index eb43a56ced..3d100093bb 100644 --- a/src/ui/events/gestures/gesture_provider_aura.cc +++ b/src/ui/events/gestures/gesture_provider_aura.cc @@ -39,7 +39,9 @@ GestureProviderAura::GestureProviderAura(GestureConsumer* consumer, kDoubleTapPlatformSupport); } -GestureProviderAura::~GestureProviderAura() {} +GestureProviderAura::~GestureProviderAura() { + client_->OnGestureProviderAuraWillBeDestroyed(this); +} bool GestureProviderAura::OnTouchEvent(TouchEvent* event) { if (!pointer_state_.OnTouch(*event)) @@ -110,4 +112,4 @@ void GestureProviderAura::OnTouchEnter(int pointer_id, float x, float y) { false /* is_source_touch_event_set_blocking */); } -} // namespace content +} // namespace ui diff --git a/src/ui/events/gestures/gesture_provider_aura.h b/src/ui/events/gestures/gesture_provider_aura.h index 1cf2b3a47d..1a7ff93aa0 100644 --- a/src/ui/events/gestures/gesture_provider_aura.h +++ b/src/ui/events/gestures/gesture_provider_aura.h @@ -27,6 +27,10 @@ class EVENTS_EXPORT GestureProviderAuraClient { virtual ~GestureProviderAuraClient() {} virtual void OnGestureEvent(GestureConsumer* consumer, GestureEvent* event) = 0; + + // Called when `gesture_provider` will be destroyed. + virtual void OnGestureProviderAuraWillBeDestroyed( + GestureProviderAura* gesture_provider) {} }; // Provides gesture detection and dispatch given a sequence of touch events diff --git a/src/ui/events/gestures/gesture_recognizer_impl.cc b/src/ui/events/gestures/gesture_recognizer_impl.cc index 85912f5ad4..344412f1c2 100644 --- a/src/ui/events/gestures/gesture_recognizer_impl.cc +++ b/src/ui/events/gestures/gesture_recognizer_impl.cc @@ -393,6 +393,18 @@ void GestureRecognizerImpl::OnGestureEvent(GestureConsumer* raw_input_consumer, DispatchGestureEvent(raw_input_consumer, event); } +void GestureRecognizerImpl::OnGestureProviderAuraWillBeDestroyed( + GestureProviderAura* gesture_provider) { + // Clean `event_to_gesture_provider_` by removing invalid raw pointers. + for (auto iter = event_to_gesture_provider_.begin(); + iter != event_to_gesture_provider_.end();) { + if (iter->second == gesture_provider) + iter = event_to_gesture_provider_.erase(iter); + else + ++iter; + } +} + GestureEventHelper* GestureRecognizerImpl::FindDispatchHelperForConsumer( GestureConsumer* consumer) { std::vector::iterator it; diff --git a/src/ui/events/gestures/gesture_recognizer_impl.h b/src/ui/events/gestures/gesture_recognizer_impl.h index 54d9c1079a..b2a24c0fbd 100644 --- a/src/ui/events/gestures/gesture_recognizer_impl.h +++ b/src/ui/events/gestures/gesture_recognizer_impl.h @@ -19,6 +19,11 @@ #include "ui/events/types/event_type.h" #include "ui/gfx/geometry/point.h" +namespace aura::test { +FORWARD_DECLARE_TEST(GestureRecognizerTest, + DestroyGestureProviderAuraBeforeAck); +} // namespace aura::test + namespace ui { class GestureConsumer; class GestureEvent; @@ -73,6 +78,9 @@ class EVENTS_EXPORT GestureRecognizerImpl : public GestureRecognizer, GestureConsumer* consumer) override; private: + FRIEND_TEST_ALL_PREFIXES(aura::test::GestureRecognizerTest, + DestroyGestureProviderAuraBeforeAck); + // Sets up the target consumer for gestures based on the touch-event. void SetupTargets(const TouchEvent& event, GestureConsumer* consumer); @@ -94,6 +102,8 @@ class EVENTS_EXPORT GestureRecognizerImpl : public GestureRecognizer, // Overridden from GestureProviderAuraClient void OnGestureEvent(GestureConsumer* raw_input_consumer, GestureEvent* event) override; + void OnGestureProviderAuraWillBeDestroyed( + GestureProviderAura* gesture_provider) override; // Convenience method to find the GestureEventHelper that can dispatch events // to a specific |consumer|. diff --git a/src/v8/src/builtins/finalization-registry.tq b/src/v8/src/builtins/finalization-registry.tq index 84499e19e1..389b9a5ce0 100644 --- a/src/v8/src/builtins/finalization-registry.tq +++ b/src/v8/src/builtins/finalization-registry.tq @@ -143,21 +143,22 @@ FinalizationRegistryRegister( ThrowTypeError( MessageTemplate::kWeakRefsRegisterTargetAndHoldingsMustNotBeSame); } - const unregisterToken = arguments[2]; // 5. If Type(unregisterToken) is not Object, // a. If unregisterToken is not undefined, throw a TypeError exception. // b. Set unregisterToken to empty. - let hasUnregisterToken: bool = false; - typeswitch (unregisterToken) { + const unregisterTokenRaw = arguments[2]; + let unregisterToken: JSReceiver|Undefined; + typeswitch (unregisterTokenRaw) { case (Undefined): { + unregisterToken = Undefined; } - case (JSReceiver): { - hasUnregisterToken = true; + case (unregisterTokenObj: JSReceiver): { + unregisterToken = unregisterTokenObj; } case (JSAny): deferred { ThrowTypeError( MessageTemplate::kWeakRefsUnregisterTokenMustBeObject, - unregisterToken); + unregisterTokenRaw); } } // 6. Let cell be the Record { [[WeakRefTarget]] : target, [[HeldValue]]: @@ -178,7 +179,7 @@ FinalizationRegistryRegister( }; // 7. Append cell to finalizationRegistry.[[Cells]]. PushCell(finalizationRegistry, cell); - if (hasUnregisterToken) { + if (unregisterToken != Undefined) { // If an unregister token is provided, a runtime call is needed to // do some OrderedHashTable operations and register the mapping. // See v8:10705. diff --git a/src/v8/src/compiler/backend/instruction-selector.cc b/src/v8/src/compiler/backend/instruction-selector.cc index 4c40835634..0abf01e4d6 100644 --- a/src/v8/src/compiler/backend/instruction-selector.cc +++ b/src/v8/src/compiler/backend/instruction-selector.cc @@ -274,7 +274,7 @@ Instruction* InstructionSelector::Emit(Instruction* instr) { bool InstructionSelector::CanCover(Node* user, Node* node) const { // 1. Both {user} and {node} must be in the same basic block. - if (schedule()->block(node) != schedule()->block(user)) { + if (schedule()->block(node) != current_block_) { return false; } // 2. Pure {node}s must be owned by the {user}. @@ -282,7 +282,7 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const { return node->OwnedBy(user); } // 3. Impure {node}s must match the effect level of {user}. - if (GetEffectLevel(node) != GetEffectLevel(user)) { + if (GetEffectLevel(node) != current_effect_level_) { return false; } // 4. Only {node} must have value edges pointing to {user}. @@ -294,21 +294,6 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const { return true; } -bool InstructionSelector::CanCoverTransitively(Node* user, Node* node, - Node* node_input) const { - if (CanCover(user, node) && CanCover(node, node_input)) { - // If {node} is pure, transitivity might not hold. - if (node->op()->HasProperty(Operator::kPure)) { - // If {node_input} is pure, the effect levels do not matter. - if (node_input->op()->HasProperty(Operator::kPure)) return true; - // Otherwise, {user} and {node_input} must have the same effect level. - return GetEffectLevel(user) == GetEffectLevel(node_input); - } - return true; - } - return false; -} - bool InstructionSelector::IsOnlyUserOfNodeInSameBlock(Node* user, Node* node) const { BasicBlock* bb_user = schedule()->block(user); @@ -1212,6 +1197,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) { int effect_level = 0; for (Node* const node : *block) { SetEffectLevel(node, effect_level); + current_effect_level_ = effect_level; if (node->opcode() == IrOpcode::kStore || node->opcode() == IrOpcode::kUnalignedStore || node->opcode() == IrOpcode::kCall || @@ -1231,6 +1217,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) { // control input should be on the same effect level as the last node. if (block->control_input() != nullptr) { SetEffectLevel(block->control_input(), effect_level); + current_effect_level_ = effect_level; } auto FinishEmittedInstructions = [&](Node* node, int instruction_start) { diff --git a/src/v8/src/compiler/backend/instruction-selector.h b/src/v8/src/compiler/backend/instruction-selector.h index c7bc99005d..51aafc36b5 100644 --- a/src/v8/src/compiler/backend/instruction-selector.h +++ b/src/v8/src/compiler/backend/instruction-selector.h @@ -417,12 +417,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final { // Used in pattern matching during code generation. // Check if {node} can be covered while generating code for the current // instruction. A node can be covered if the {user} of the node has the only - // edge and the two are in the same basic block. + // edge, the two are in the same basic block, and there are no side-effects + // in-between. The last check is crucial for soundness. + // For pure nodes, CanCover(a,b) is checked to avoid duplicated execution: + // If this is not the case, code for b must still be generated for other + // users, and fusing is unlikely to improve performance. bool CanCover(Node* user, Node* node) const; - // CanCover is not transitive. The counter example are Nodes A,B,C such that - // CanCover(A, B) and CanCover(B,C) and B is pure: The the effect level of A - // and B might differ. CanCoverTransitively does the additional checks. - bool CanCoverTransitively(Node* user, Node* node, Node* node_input) const; // Used in pattern matching during code generation. // This function checks that {node} and {user} are in the same basic block, @@ -741,6 +741,7 @@ class V8_EXPORT_PRIVATE InstructionSelector final { BoolVector defined_; BoolVector used_; IntVector effect_level_; + int current_effect_level_; IntVector virtual_registers_; IntVector virtual_register_rename_; InstructionScheduler* scheduler_; diff --git a/src/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc b/src/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc index 1fa6ce8f3d..d43a8a56e0 100644 --- a/src/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc +++ b/src/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc @@ -1523,7 +1523,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { if (CanCover(node, value)) { switch (value->opcode()) { case IrOpcode::kWord64Sar: { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryEmitExtendingLoad(this, value, node)) { return; } else { diff --git a/src/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc b/src/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc index 1d6b506685..7d2e5e7853 100644 --- a/src/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc +++ b/src/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc @@ -1277,7 +1277,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { if (CanCover(node, value)) { switch (value->opcode()) { case IrOpcode::kWord64Sar: { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryEmitExtendingLoad(this, value, node)) { return; } else { diff --git a/src/v8/src/compiler/backend/x64/instruction-selector-x64.cc b/src/v8/src/compiler/backend/x64/instruction-selector-x64.cc index 3f005475b8..ca33549e3a 100644 --- a/src/v8/src/compiler/backend/x64/instruction-selector-x64.cc +++ b/src/v8/src/compiler/backend/x64/instruction-selector-x64.cc @@ -1685,7 +1685,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { case IrOpcode::kWord64Shr: { Int64BinopMatcher m(value); if (m.right().Is(32)) { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryMatchLoadWord64AndShiftRight(this, value, kX64Movl)) { return EmitIdentity(node); } diff --git a/src/v8/src/heap/mark-compact.cc b/src/v8/src/heap/mark-compact.cc index 37cd937710..9e4f36d35c 100755 --- a/src/v8/src/heap/mark-compact.cc +++ b/src/v8/src/heap/mark-compact.cc @@ -2557,28 +2557,19 @@ void MarkCompactCollector::ClearJSWeakRefs() { RecordSlot(weak_cell, slot, HeapObject::cast(*slot)); } - HeapObject unregister_token = - HeapObject::cast(weak_cell.unregister_token()); + HeapObject unregister_token = weak_cell.unregister_token(); if (!non_atomic_marking_state()->IsBlackOrGrey(unregister_token)) { // The unregister token is dead. Remove any corresponding entries in the // key map. Multiple WeakCell with the same token will have all their // unregister_token field set to undefined when processing the first // WeakCell. Like above, we're modifying pointers during GC, so record the // slots. - HeapObject undefined = ReadOnlyRoots(isolate()).undefined_value(); JSFinalizationRegistry finalization_registry = JSFinalizationRegistry::cast(weak_cell.finalization_registry()); finalization_registry.RemoveUnregisterToken( JSReceiver::cast(unregister_token), isolate(), - [undefined](WeakCell matched_cell) { - matched_cell.set_unregister_token(undefined); - }, + JSFinalizationRegistry::kKeepMatchedCellsInRegistry, gc_notify_updated_slot); - // The following is necessary because in the case that weak_cell has - // already been popped and removed from the FinalizationRegistry, the call - // to JSFinalizationRegistry::RemoveUnregisterToken above will not find - // weak_cell itself to clear its unregister token. - weak_cell.set_unregister_token(undefined); } else { // The unregister_token is alive. ObjectSlot slot = weak_cell.RawField(WeakCell::kUnregisterTokenOffset); diff --git a/src/v8/src/heap/marking-visitor-inl.h b/src/v8/src/heap/marking-visitor-inl.h index 55c37e535b..557d6248fc 100644 --- a/src/v8/src/heap/marking-visitor-inl.h +++ b/src/v8/src/heap/marking-visitor-inl.h @@ -326,7 +326,7 @@ int MarkingVisitorBase::VisitWeakCell( this->VisitMapPointer(weak_cell); WeakCell::BodyDescriptor::IterateBody(map, weak_cell, size, this); HeapObject target = weak_cell.relaxed_target(); - HeapObject unregister_token = HeapObject::cast(weak_cell.unregister_token()); + HeapObject unregister_token = weak_cell.relaxed_unregister_token(); concrete_visitor()->SynchronizePageAccess(target); concrete_visitor()->SynchronizePageAccess(unregister_token); if (concrete_visitor()->marking_state()->IsBlackOrGrey(target) && diff --git a/src/v8/src/objects/js-weak-refs-inl.h b/src/v8/src/objects/js-weak-refs-inl.h index aa51ee18c8..0e39b00d13 100644 --- a/src/v8/src/objects/js-weak-refs-inl.h +++ b/src/v8/src/objects/js-weak-refs-inl.h @@ -71,16 +71,14 @@ bool JSFinalizationRegistry::Unregister( // key. Each WeakCell will be in the "active_cells" or "cleared_cells" list of // its FinalizationRegistry; remove it from there. return finalization_registry->RemoveUnregisterToken( - *unregister_token, isolate, - [isolate](WeakCell matched_cell) { - matched_cell.RemoveFromFinalizationRegistryCells(isolate); - }, + *unregister_token, isolate, kRemoveMatchedCellsFromRegistry, [](HeapObject, ObjectSlot, Object) {}); } -template +template bool JSFinalizationRegistry::RemoveUnregisterToken( - JSReceiver unregister_token, Isolate* isolate, MatchCallback match_callback, + JSReceiver unregister_token, Isolate* isolate, + RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot) { // This method is called from both FinalizationRegistry#unregister and for // removing weakly-held dead unregister tokens. The latter is during GC so @@ -118,7 +116,16 @@ bool JSFinalizationRegistry::RemoveUnregisterToken( value = weak_cell.key_list_next(); if (weak_cell.unregister_token() == unregister_token) { // weak_cell has the same unregister token; remove it from the key list. - match_callback(weak_cell); + switch (removal_mode) { + case kRemoveMatchedCellsFromRegistry: + weak_cell.RemoveFromFinalizationRegistryCells(isolate); + break; + case kKeepMatchedCellsInRegistry: + // Do nothing. + break; + } + // Clear unregister token-related fields. + weak_cell.set_unregister_token(undefined); weak_cell.set_key_list_prev(undefined); weak_cell.set_key_list_next(undefined); was_present = true; @@ -163,6 +170,10 @@ HeapObject WeakCell::relaxed_target() const { return TaggedField::Relaxed_Load(*this, kTargetOffset); } +HeapObject WeakCell::relaxed_unregister_token() const { + return TaggedField::Relaxed_Load(*this, kUnregisterTokenOffset); +} + template void WeakCell::Nullify(Isolate* isolate, GCNotifyUpdatedSlotCallback gc_notify_updated_slot) { diff --git a/src/v8/src/objects/js-weak-refs.h b/src/v8/src/objects/js-weak-refs.h index 300673381a..88361ad1c0 100644 --- a/src/v8/src/objects/js-weak-refs.h +++ b/src/v8/src/objects/js-weak-refs.h @@ -53,10 +53,14 @@ class JSFinalizationRegistry : public JSObject { // it modifies slots in key_map and WeakCells and the normal write barrier is // disabled during GC, we need to tell the GC about the modified slots via the // gc_notify_updated_slot function. - template + enum RemoveUnregisterTokenMode { + kRemoveMatchedCellsFromRegistry, + kKeepMatchedCellsInRegistry + }; + template inline bool RemoveUnregisterToken( JSReceiver unregister_token, Isolate* isolate, - MatchCallback match_callback, + RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot); // Returns true if the cleared_cells list is non-empty. @@ -93,6 +97,9 @@ class WeakCell : public TorqueGeneratedWeakCell { // Provide relaxed load access to target field. inline HeapObject relaxed_target() const; + // Provide relaxed load access to the unregister token field. + inline HeapObject relaxed_unregister_token() const; + // Nullify is called during GC and it modifies the pointers in WeakCell and // JSFinalizationRegistry. Thus we need to tell the GC about the modified // slots via the gc_notify_updated_slot function. The normal write barrier is diff --git a/src/v8/src/objects/js-weak-refs.tq b/src/v8/src/objects/js-weak-refs.tq index 9008f64290..3447e31b71 100644 --- a/src/v8/src/objects/js-weak-refs.tq +++ b/src/v8/src/objects/js-weak-refs.tq @@ -22,7 +22,7 @@ extern class JSFinalizationRegistry extends JSObject { extern class WeakCell extends HeapObject { finalization_registry: Undefined|JSFinalizationRegistry; target: Undefined|JSReceiver; - unregister_token: JSAny; + unregister_token: Undefined|JSReceiver; holdings: JSAny; // For storing doubly linked lists of WeakCells in JSFinalizationRegistry's diff --git a/src/v8/src/objects/objects.cc b/src/v8/src/objects/objects.cc index fa3f9fc82d..0c1cc482de 100755 --- a/src/v8/src/objects/objects.cc +++ b/src/v8/src/objects/objects.cc @@ -6740,6 +6740,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( JSFinalizationRegistry::cast(Object(raw_finalization_registry)); WeakCell weak_cell = WeakCell::cast(Object(raw_weak_cell)); DCHECK(!weak_cell.unregister_token().IsUndefined(isolate)); + HeapObject undefined = ReadOnlyRoots(isolate).undefined_value(); // Remove weak_cell from the linked list of other WeakCells with the same // unregister token and remove its unregister token from key_map if necessary @@ -6748,7 +6749,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( if (weak_cell.key_list_prev().IsUndefined(isolate)) { SimpleNumberDictionary key_map = SimpleNumberDictionary::cast(finalization_registry.key_map()); - Object unregister_token = weak_cell.unregister_token(); + HeapObject unregister_token = weak_cell.unregister_token(); uint32_t key = Smi::ToInt(unregister_token.GetHash()); InternalIndex entry = key_map.FindEntry(isolate, key); DCHECK(entry.is_found()); @@ -6763,8 +6764,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( // of the key in the hash table. WeakCell next = WeakCell::cast(weak_cell.key_list_next()); DCHECK_EQ(next.key_list_prev(), weak_cell); - next.set_key_list_prev(ReadOnlyRoots(isolate).undefined_value()); - weak_cell.set_key_list_next(ReadOnlyRoots(isolate).undefined_value()); + next.set_key_list_prev(undefined); key_map.ValueAtPut(entry, next); } } else { @@ -6776,6 +6776,12 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( next.set_key_list_prev(weak_cell.key_list_prev()); } } + + // weak_cell is now removed from the unregister token map, so clear its + // unregister token-related fields. + weak_cell.set_unregister_token(undefined); + weak_cell.set_key_list_prev(undefined); + weak_cell.set_key_list_next(undefined); } } // namespace internal diff --git a/src/v8/test/cctest/test-js-weak-refs.cc b/src/v8/test/cctest/test-js-weak-refs.cc index 1291283515..52cf8a529e 100644 --- a/src/v8/test/cctest/test-js-weak-refs.cc +++ b/src/v8/test/cctest/test-js-weak-refs.cc @@ -831,7 +831,7 @@ TEST(TestRemoveUnregisterToken) { Handle token1 = CreateKey("token1", isolate); Handle token2 = CreateKey("token2", isolate); - Handle undefined = + Handle undefined = handle(ReadOnlyRoots(isolate).undefined_value(), isolate); Handle weak_cell1a = FinalizationRegistryRegister( @@ -861,9 +861,7 @@ TEST(TestRemoveUnregisterToken) { finalization_registry->RemoveUnregisterToken( JSReceiver::cast(*token2), isolate, - [undefined](WeakCell matched_cell) { - matched_cell.set_unregister_token(*undefined); - }, + JSFinalizationRegistry::kKeepMatchedCellsInRegistry, [](HeapObject, ObjectSlot, Object) {}); // Both weak_cell2a and weak_cell2b remain on the weak cell chains. @@ -989,15 +987,17 @@ TEST(UnregisterTokenHeapVerifier) { v8::HandleScope outer_scope(isolate); { - // Make a new FinalizationRegistry and register an object with an unregister - // token that's unreachable after the IIFE returns. + // Make a new FinalizationRegistry and register two objects with the same + // unregister token that's unreachable after the IIFE returns. v8::HandleScope scope(isolate); CompileRun( "var token = {}; " "var registry = new FinalizationRegistry(function () {}); " "(function () { " - " let o = {}; " - " registry.register(o, {}, token); " + " let o1 = {}; " + " let o2 = {}; " + " registry.register(o1, {}, token); " + " registry.register(o2, {}, token); " "})();"); } @@ -1022,5 +1022,52 @@ TEST(UnregisterTokenHeapVerifier) { EmptyMessageQueues(isolate); } +TEST(UnregisteredAndUnclearedCellHeapVerifier) { + if (!FLAG_incremental_marking) return; + ManualGCScope manual_gc_scope; +#ifdef VERIFY_HEAP + FLAG_verify_heap = true; +#endif + + CcTest::InitializeVM(); + v8::Isolate* isolate = CcTest::isolate(); + Heap* heap = CcTest::heap(); + v8::HandleScope outer_scope(isolate); + + { + // Make a new FinalizationRegistry and register an object with a token. + v8::HandleScope scope(isolate); + CompileRun( + "var token = {}; " + "var registry = new FinalizationRegistry(function () {}); " + "registry.register({}, undefined, token);"); + } + + // Start incremental marking to activate the marking barrier. + heap::SimulateIncrementalMarking(heap, false); + + { + // Make a WeakCell list with length >1, then unregister with the token to + // the WeakCell from the registry. The linked list manipulation keeps the + // unregistered WeakCell alive (i.e. not put into cleared_cells) due to the + // marking barrier from incremental marking. Then make the original token + // collectible. + v8::HandleScope scope(isolate); + CompileRun( + "registry.register({}); " + "registry.unregister(token); " + "token = 0;"); + } + + // Trigger GC. + CcTest::CollectAllGarbage(); + CcTest::CollectAllGarbage(); + + // Pump message loop to run the finalizer task, then the incremental marking + // task. The verifier will verify that live WeakCells don't point to dead + // unregister tokens. + EmptyMessageQueues(isolate); +} + } // namespace internal } // namespace v8