From 90aea56de6a4e1da5a678339db711a300faa6065 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 11 Oct 2023 15:18:35 -0400 Subject: [PATCH] ceftests: Change CreateTestRequestContext to async usage With Chrome runtime, custom request contexts may be initialized asynchronously. Wait for that initialization before using the request context. This also removes TEST_RC_MODE_CUSTOM (uses TEST_RC_MODE_CUSTOM_WITH_HANDLER instead) since we always need a handler to get the initialization callback. --- tests/ceftests/certificate_error_unittest.cc | 9 ++- tests/ceftests/download_unittest.cc | 30 +++++---- tests/ceftests/hsts_redirect_unittest.cc | 10 ++- tests/ceftests/navigation_unittest.cc | 17 +++-- tests/ceftests/request_context_unittest.cc | 15 +++-- tests/ceftests/test_util.cc | 66 ++++++++++++-------- tests/ceftests/test_util.h | 35 +++++------ 7 files changed, 111 insertions(+), 71 deletions(-) diff --git a/tests/ceftests/certificate_error_unittest.cc b/tests/ceftests/certificate_error_unittest.cc index b6c82933d..927c6c101 100644 --- a/tests/ceftests/certificate_error_unittest.cc +++ b/tests/ceftests/certificate_error_unittest.cc @@ -249,8 +249,13 @@ class CertificateErrorTest : public TestHandler, public CefTestServerHandler { EXPECT_UI_THREAD(); // Create a new in-memory context so certificate decisions aren't cached. - auto request_context = CreateTestRequestContext( - TEST_RC_MODE_CUSTOM, /*cache_path=*/std::string()); + CreateTestRequestContext( + TEST_RC_MODE_CUSTOM_WITH_HANDLER, /*cache_path=*/std::string(), + base::BindOnce(&CertificateErrorTest::DoCreateBrowserContinue, this)); + } + + void DoCreateBrowserContinue(CefRefPtr request_context) { + EXPECT_UI_THREAD(); CreateBrowser(GetStartURL(), request_context); } diff --git a/tests/ceftests/download_unittest.cc b/tests/ceftests/download_unittest.cc index 50028ac4f..768906e46 100644 --- a/tests/ceftests/download_unittest.cc +++ b/tests/ceftests/download_unittest.cc @@ -199,6 +199,24 @@ class DownloadTestHandler : public TestHandler { } void RunTest() override { + if (!is_clicked() || is_clicked_and_downloaded()) { + // Create a new temporary directory. + EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); + test_path_ = + client::file_util::JoinPath(temp_dir_.GetPath(), kTestFileName); + } + + CreateTestRequestContext( + rc_mode_, rc_cache_path_, + base::BindOnce(&DownloadTestHandler::RunTestContinue, this)); + + // Time out the test after a reasonable period of time. + SetTestTimeout(); + } + + void RunTestContinue(CefRefPtr request_context) { + EXPECT_UI_THREAD(); + DelayCallbackVendor delay_callback_vendor; if (test_mode_ == NAVIGATED || test_mode_ == PENDING) { delay_callback_vendor = base::BindRepeating( @@ -212,8 +230,6 @@ class DownloadTestHandler : public TestHandler { new DownloadSchemeHandlerFactory(delay_callback_vendor, &got_download_request_); - CefRefPtr request_context = - CreateTestRequestContext(rc_mode_, rc_cache_path_); if (request_context) { request_context->RegisterSchemeHandlerFactory("https", kTestDomain, scheme_factory); @@ -221,13 +237,6 @@ class DownloadTestHandler : public TestHandler { CefRegisterSchemeHandlerFactory("https", kTestDomain, scheme_factory); } - if (!is_clicked() || is_clicked_and_downloaded()) { - // Create a new temporary directory. - EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); - test_path_ = - client::file_util::JoinPath(temp_dir_.GetPath(), kTestFileName); - } - if (test_mode_ == NAVIGATED) { // Add the resource that we'll navigate to. AddResource(kTestNavUrl, "Navigated", @@ -254,9 +263,6 @@ class DownloadTestHandler : public TestHandler { // Create the browser CreateBrowser(kTestStartUrl, request_context); - - // Time out the test after a reasonable period of time. - SetTestTimeout(); } void OnLoadEnd(CefRefPtr browser, diff --git a/tests/ceftests/hsts_redirect_unittest.cc b/tests/ceftests/hsts_redirect_unittest.cc index 218ba16d8..1666eaabe 100644 --- a/tests/ceftests/hsts_redirect_unittest.cc +++ b/tests/ceftests/hsts_redirect_unittest.cc @@ -263,8 +263,14 @@ class HSTSRedirectTest : public TestHandler { EXPECT_TRUE(https_url_.find("https://localhost:") == 0); // Create a new in-memory context so HSTS decisions aren't cached. - auto request_context = CreateTestRequestContext( - TEST_RC_MODE_CUSTOM, /*cache_path=*/std::string()); + CreateTestRequestContext( + TEST_RC_MODE_CUSTOM_WITH_HANDLER, /*cache_path=*/std::string(), + base::BindOnce(&HSTSRedirectTest::StartedHttpsServerContinue, this)); + } + + void StartedHttpsServerContinue( + CefRefPtr request_context) { + EXPECT_UI_THREAD(); CreateBrowser(http_url_, request_context); } diff --git a/tests/ceftests/navigation_unittest.cc b/tests/ceftests/navigation_unittest.cc index 42d81b9fc..0ea5849a2 100644 --- a/tests/ceftests/navigation_unittest.cc +++ b/tests/ceftests/navigation_unittest.cc @@ -2430,14 +2430,21 @@ class PopupJSWindowOpenTestHandler : public TestHandler { // Create a new disk-based request context so that we can grant default // popup permission (used for the popup without a valid URL) without // impacting the global context. - auto request_context = CreateTestRequestContext(mode_, cache_path_); + CreateTestRequestContext( + mode_, cache_path_, + base::BindOnce(&PopupJSWindowOpenTestHandler::RunTestContinue, this)); + + // Time out the test after a reasonable period of time. + SetTestTimeout(); + } + + void RunTestContinue(CefRefPtr request_context) { + EXPECT_UI_THREAD(); + GrantPopupPermission(request_context, CefString()); // Create the browser. CreateBrowser(kPopupJSOpenMainUrl, request_context); - - // Time out the test after a reasonable period of time. - SetTestTimeout(); } bool OnBeforePopup(CefRefPtr browser, @@ -2565,7 +2572,7 @@ class PopupJSWindowOpenTestHandler : public TestHandler { RC_TEST_SINGLE(NavigationTest, PopupJSWindowOpen, PopupJSWindowOpenTestHandler, - TEST_RC_MODE_CUSTOM, + TEST_RC_MODE_CUSTOM_WITH_HANDLER, /*with_cache_path*/ true) namespace { diff --git a/tests/ceftests/request_context_unittest.cc b/tests/ceftests/request_context_unittest.cc index 2159437bc..a8882b0ca 100644 --- a/tests/ceftests/request_context_unittest.cc +++ b/tests/ceftests/request_context_unittest.cc @@ -486,8 +486,16 @@ class PopupNavTestHandler : public TestHandler { AddResource(kPopupNavPopupUrl2, "Popup2", "text/html"); } - CefRefPtr request_context = - CreateTestRequestContext(rc_mode_, rc_cache_path_); + CreateTestRequestContext( + rc_mode_, rc_cache_path_, + base::BindOnce(&PopupNavTestHandler::RunTestContinue, this)); + + // Time out the test after a reasonable period of time. + SetTestTimeout(); + } + + void RunTestContinue(CefRefPtr request_context) { + EXPECT_UI_THREAD(); if (request_context) { GrantPopupPermission(request_context, kPopupNavPageUrl); @@ -498,9 +506,6 @@ class PopupNavTestHandler : public TestHandler { // Create the browser. CreateBrowser(kPopupNavPageUrl, request_context); - - // Time out the test after a reasonable period of time. - SetTestTimeout(); } bool OnBeforePopup(CefRefPtr browser, diff --git a/tests/ceftests/test_util.cc b/tests/ceftests/test_util.cc index 34c10ab15..aaa65bccd 100644 --- a/tests/ceftests/test_util.cc +++ b/tests/ceftests/test_util.cc @@ -13,6 +13,7 @@ #include "include/cef_command_line.h" #include "include/cef_request_context_handler.h" #include "include/wrapper/cef_closure_task.h" +#include "include/wrapper/cef_helpers.h" #include "tests/gtest/include/gtest/gtest.h" #include "tests/shared/common/string_util.h" @@ -386,43 +387,54 @@ void GrantPopupPermission(CefRefPtr request_context, } } -CefRefPtr CreateTestRequestContext( - TestRequestContextMode mode, - const std::string& cache_path) { - EXPECT_TRUE(cache_path.empty() || mode == TEST_RC_MODE_CUSTOM || - mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER); +void CreateTestRequestContext(TestRequestContextMode mode, + const std::string& cache_path, + RCInitCallback init_callback) { + DCHECK(!init_callback.is_null()); + EXPECT_TRUE(cache_path.empty() || mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER); - if (mode == TEST_RC_MODE_NONE) { - return nullptr; - } - if (mode == TEST_RC_MODE_GLOBAL) { - return CefRequestContext::GetGlobalContext(); + if (mode == TEST_RC_MODE_NONE || mode == TEST_RC_MODE_GLOBAL) { + // Global contexts are initialized synchronously during startup, so we can + // execute the callback immediately. + CefRefPtr request_context; + if (mode == TEST_RC_MODE_GLOBAL) { + request_context = CefRequestContext::GetGlobalContext(); + } + + CefPostTask(TID_UI, + base::BindOnce(std::move(init_callback), request_context)); + return; } - CefRefPtr rc_handler; - if (mode == TEST_RC_MODE_GLOBAL_WITH_HANDLER || - mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER) { - class Handler : public CefRequestContextHandler { - public: - Handler() {} + class Handler : public CefRequestContextHandler { + public: + explicit Handler(RCInitCallback init_callback) + : init_callback_(std::move(init_callback)) {} - private: - IMPLEMENT_REFCOUNTING(Handler); - }; - rc_handler = new Handler(); - } + void OnRequestContextInitialized( + CefRefPtr request_context) override { + CEF_REQUIRE_UI_THREAD(); + std::move(init_callback_).Run(request_context); + } - if (mode == TEST_RC_MODE_CUSTOM || mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER) { + private: + RCInitCallback init_callback_; + IMPLEMENT_REFCOUNTING(Handler); + }; + + CefRefPtr rc_handler = + new Handler(std::move(init_callback)); + + if (mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER) { CefRequestContextSettings settings; if (!cache_path.empty()) { CefString(&settings.cache_path) = cache_path; } - return CefRequestContext::CreateContext(settings, rc_handler); + CefRequestContext::CreateContext(settings, rc_handler); + } else { + CefRequestContext::CreateContext(CefRequestContext::GetGlobalContext(), + rc_handler); } - - EXPECT_EQ(mode, TEST_RC_MODE_GLOBAL_WITH_HANDLER); - return CefRequestContext::CreateContext(CefRequestContext::GetGlobalContext(), - rc_handler); } CefTime CefTimeFrom(CefBaseTime value) { diff --git a/tests/ceftests/test_util.h b/tests/ceftests/test_util.h index 359d7ca20..2c024f8e3 100644 --- a/tests/ceftests/test_util.h +++ b/tests/ceftests/test_util.h @@ -8,6 +8,7 @@ #include +#include "include/base/cef_callback.h" #include "include/cef_process_message.h" #include "include/cef_request.h" #include "include/cef_request_context.h" @@ -74,13 +75,11 @@ enum TestRequestContextMode { TEST_RC_MODE_NONE, TEST_RC_MODE_GLOBAL, TEST_RC_MODE_GLOBAL_WITH_HANDLER, - TEST_RC_MODE_CUSTOM, TEST_RC_MODE_CUSTOM_WITH_HANDLER, }; inline bool IsTestRequestContextModeCustom(TestRequestContextMode mode) { - return mode == TEST_RC_MODE_CUSTOM || - mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER; + return mode == TEST_RC_MODE_CUSTOM_WITH_HANDLER; } // Returns true if the old CefResourceHandler API should be tested. @@ -114,12 +113,14 @@ void SendMouseClickEvent(CefRefPtr browser, void GrantPopupPermission(CefRefPtr request_context, const std::string& parent_url); -// Return a RequestContext object matching the specified |mode|. -// |cache_path| may be specified for CUSTOM modes. -// Use the RC_TEST_GROUP_BASE macro to test all valid combinations. -CefRefPtr CreateTestRequestContext( - TestRequestContextMode mode, - const std::string& cache_path); +// Create a CefRequestContext object matching the specified |mode|. |cache_path| +// may be specified for CUSTOM modes. |init_callback| will be executed +// asynchronously on the UI thread. Use the RC_TEST_GROUP_ALL macro to test all +// valid combinations. +using RCInitCallback = base::OnceCallback)>; +void CreateTestRequestContext(TestRequestContextMode mode, + const std::string& cache_path, + RCInitCallback init_callback); // Run a single test without additional test modes. #define RC_TEST_SINGLE(test_case_name, test_name, test_class, rc_mode, \ @@ -170,17 +171,13 @@ CefRefPtr CreateTestRequestContext( TEST_RC_MODE_GLOBAL, false) \ RC_TEST_BASE(test_case_name, test_name##RCGlobalWithHandler, test_class, \ test_mode, TEST_RC_MODE_GLOBAL_WITH_HANDLER, false) \ - RC_TEST_BASE(test_case_name, test_name##RCCustomInMemory, test_class, \ - test_mode, TEST_RC_MODE_CUSTOM, false) \ RC_TEST_BASE(test_case_name, test_name##RCCustomInMemoryWithHandler, \ test_class, test_mode, TEST_RC_MODE_CUSTOM_WITH_HANDLER, false) // RequestContextModes that operate on disk. -#define RC_TEST_GROUP_ON_DISK(test_case_name, test_name, test_class, \ - test_mode) \ - RC_TEST_BASE(test_case_name, test_name##RCCustomOnDisk, test_class, \ - test_mode, TEST_RC_MODE_CUSTOM, true) \ - RC_TEST_BASE(test_case_name, test_name##RCCustomOnDiskWithHandler, \ +#define RC_TEST_GROUP_ON_DISK(test_case_name, test_name, test_class, \ + test_mode) \ + RC_TEST_BASE(test_case_name, test_name##RCCustomOnDiskWithHandler, \ test_class, test_mode, TEST_RC_MODE_CUSTOM_WITH_HANDLER, true) // Helper macro for testing all valid combinations of RequestContextMode values. @@ -204,9 +201,11 @@ CefRefPtr CreateTestRequestContext( // // void RunTest() override { // // Create a RequestContext with the specified attributes. -// CefRefPtr request_context = -// CreateTestRequestContext(rc_mode_, rc_cache_path_); +// CreateTestRequestContext(rc_mode_, rc_cache_path_, +// base::BindOnce(&MyTestHandler::RunTestContinue, this)); +// } // +// void RunTestContinue(CefRefPtr request_context) { // // Do something with |test_mode_| and |request_context|... // } //