diff --git a/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java b/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java index 88246f87efbea43c88a3cbdd5c40202a74d02b8b..87decb6e3b32e39ac270d7b323ed1ceb527e615e 100644 --- a/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java +++ b/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java @@ -1598,12 +1598,13 @@ public class ContextualSearchManager @Override public void gatherSurroundingText() { if (mContext != null) mContext.destroy(); - mContext = new ContextualSearchContext() { - @Override - void onSelectionChanged() { - notifyObserversOfContextSelectionChanged(); - } - }; + mContext = + new ContextualSearchContext() { + @Override + void onSelectionChanged() { + notifyObserversOfContextSelectionChanged(); + } + }; boolean isResolvingGesture = mPolicy.isResolvingGesture(); if (isResolvingGesture && mPolicy.shouldPreviousGestureResolve()) { @@ -1613,9 +1614,12 @@ public class ContextualSearchManager if (webContents != null) { mInternalStateController.notifyStartingWorkOn( InternalState.GATHERING_SURROUNDINGS); - ContextualSearchManagerJni.get().gatherSurroundingText( - mNativeContextualSearchManagerPtr, ContextualSearchManager.this, - mContext, webContents); + ContextualSearchManagerJni.get() + .gatherSurroundingText( + mNativeContextualSearchManagerPtr, + ContextualSearchManager.this, + mContext, + webContents); } else { mInternalStateController.reset(StateChangeReason.UNKNOWN); } @@ -1650,9 +1654,13 @@ public class ContextualSearchManager mInternalStateController.notifyStartingWorkOn( InternalState.START_SHOWING_TAP_UI); mSelectAroundCaretCounter++; - baseWebContents.selectAroundCaret(SelectionGranularity.WORD, - /*shouldShowHandle=*/false, - /*shouldShowContextMenu=*/false); + baseWebContents.selectAroundCaret( + SelectionGranularity.WORD, + /* shouldShowHandle= */ false, + /* shouldShowContextMenu= */ false, + mContext.getSelectionStartOffset(), + mContext.getSelectionEndOffset(), + mContext.getSurroundingText().length()); // Let the policy know that a valid tap gesture has been received. mPolicy.registerTap(); } else { @@ -1663,43 +1671,51 @@ public class ContextualSearchManager /** * Waits for possible Tap gesture that's near enough to the previous tap to be * considered a "re-tap". We've done some work on the previous Tap and we just saw the - * selection get cleared (probably due to a Tap that may or may not be valid). - * If it's invalid we'll want to hide the UI. If it's valid we'll want to just update - * the UI rather than having the Bar hide and re-show. + * selection get cleared (probably due to a Tap that may or may not be valid). If it's + * invalid we'll want to hide the UI. If it's valid we'll want to just update the UI + * rather than having the Bar hide and re-show. */ @Override public void waitForPossibleTapNearPrevious() { mInternalStateController.notifyStartingWorkOn( InternalState.WAITING_FOR_POSSIBLE_TAP_NEAR_PREVIOUS); - new Handler().postDelayed(new Runnable() { - @Override - public void run() { - // We may have been destroyed. - if (mSearchPanel != null) mSearchPanel.hideCaption(); - mInternalStateController.notifyFinishedWorkOn( - InternalState.WAITING_FOR_POSSIBLE_TAP_NEAR_PREVIOUS); - } - }, TAP_NEAR_PREVIOUS_DETECTION_DELAY_MS); + new Handler() + .postDelayed( + new Runnable() { + @Override + public void run() { + // We may have been destroyed. + if (mSearchPanel != null) mSearchPanel.hideCaption(); + mInternalStateController.notifyFinishedWorkOn( + InternalState + .WAITING_FOR_POSSIBLE_TAP_NEAR_PREVIOUS); + } + }, + TAP_NEAR_PREVIOUS_DETECTION_DELAY_MS); } /** - * Waits for possible Tap gesture that's on a previously established tap-selection. - * If the current Tap was on the previous tap-selection then this selection will become - * a Long-press selection and we'll recognize that gesture and start processing it. - * If that doesn't happen within our time window (which is the common case) then we'll + * Waits for possible Tap gesture that's on a previously established tap-selection. If + * the current Tap was on the previous tap-selection then this selection will become a + * Long-press selection and we'll recognize that gesture and start processing it. If + * that doesn't happen within our time window (which is the common case) then we'll * advance to the next state in normal Tap processing. */ @Override public void waitForPossibleTapOnTapSelection() { mInternalStateController.notifyStartingWorkOn( InternalState.WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION); - new Handler().postDelayed(new Runnable() { - @Override - public void run() { - mInternalStateController.notifyFinishedWorkOn( - InternalState.WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION); - } - }, TAP_ON_TAP_SELECTION_DELAY_MS); + new Handler() + .postDelayed( + new Runnable() { + @Override + public void run() { + mInternalStateController.notifyFinishedWorkOn( + InternalState + .WAITING_FOR_POSSIBLE_TAP_ON_TAP_SELECTION); + } + }, + TAP_ON_TAP_SELECTION_DELAY_MS); } /** Starts a Resolve request to our server for the best Search Term. */ @@ -1736,8 +1752,10 @@ public class ContextualSearchManager } else { mInternalStateController.notifyStartingWorkOn(InternalState.SHOW_RESOLVING_UI); boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP; - showContextualSearch(isTap ? StateChangeReason.TEXT_SELECT_TAP - : StateChangeReason.TEXT_SELECT_LONG_PRESS); + showContextualSearch( + isTap + ? StateChangeReason.TEXT_SELECT_TAP + : StateChangeReason.TEXT_SELECT_LONG_PRESS); mInternalStateController.notifyFinishedWorkOn(InternalState.SHOW_RESOLVING_UI); } } diff --git a/android/junit/src/org/chromium/chrome/browser/customtabs/TrustedCdnTest.java b/android/junit/src/org/chromium/chrome/browser/customtabs/TrustedCdnTest.java index 75888f08ae26818b8895b9eae72ccf6789e0e446..16ffdb7a29356b37027b6a3a5b674b2555c0fb95 100644 --- a/android/junit/src/org/chromium/chrome/browser/customtabs/TrustedCdnTest.java +++ b/android/junit/src/org/chromium/chrome/browser/customtabs/TrustedCdnTest.java @@ -7,6 +7,7 @@ package org.chromium.chrome.browser.customtabs; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import org.junit.Before; @@ -14,6 +15,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestRule; import org.junit.runner.RunWith; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; @@ -89,7 +91,10 @@ public class TrustedCdnTest { mCustomTabTrustedCdnPublisherUrlVisibility = new CustomTabTrustedCdnPublisherUrlVisibility( mWindowAndroid, mLifecycleDispatcher, () -> mShouldPackageShowPublisherUrl); - TrustedCdn.setPublisherUrlForTesting(mTab, PUBLISHER_URL); + doReturn(PUBLISHER_URL) + .when(mTrustedCdnNatives) + .getPublisherUrl(ArgumentMatchers.anyLong()); + TrustedCdn.initForTesting(mTab); } @Test diff --git a/browser/BUILD.gn b/browser/BUILD.gn index 99b67927bdbb62ce3003e8637ff6e428813f6e95..d0e76696209a64db60b52307423f1a80e843455e 100644 --- a/browser/BUILD.gn +++ b/browser/BUILD.gn @@ -939,6 +939,8 @@ static_library("browser") { "optimization_guide/chrome_browser_main_extra_parts_optimization_guide.h", "optimization_guide/chrome_hints_manager.cc", "optimization_guide/chrome_hints_manager.h", + "optimization_guide/chrome_prediction_model_store.cc", + "optimization_guide/chrome_prediction_model_store.h", "optimization_guide/model_validator_keyed_service.cc", "optimization_guide/model_validator_keyed_service.h", "optimization_guide/model_validator_keyed_service_factory.cc", diff --git a/browser/android/trusted_cdn.cc b/browser/android/trusted_cdn.cc index feba0e0cc04990f50de33850ebd4ee8b87f3bc4d..e28aa6d8237f7e09adedf9ea90b970c29c1208ae 100644 --- a/browser/android/trusted_cdn.cc +++ b/browser/android/trusted_cdn.cc @@ -12,8 +12,8 @@ #include "chrome/browser/offline_pages/offline_page_utils.h" #include "chrome/browser/tab/jni_headers/TrustedCdn_jni.h" #include "components/embedder_support/android/util/cdn_utils.h" -#include "content/public/browser/page.h" #include "content/public/browser/web_contents.h" +#include "url/android/gurl_android.h" #include "url/gurl.h" using base::android::ConvertUTF8ToJavaString; @@ -29,36 +29,32 @@ TrustedCdn::~TrustedCdn() = default; void TrustedCdn::SetWebContents(JNIEnv* env, const JavaParamRef& obj, const JavaParamRef& jweb_contents) { - WebContentsObserver::Observe(WebContents::FromJavaWebContents(jweb_contents)); + web_contents_ = WebContents::FromJavaWebContents(jweb_contents); } void TrustedCdn::ResetWebContents(JNIEnv* env, const JavaParamRef& obj) { - WebContentsObserver::Observe(nullptr); + web_contents_ = nullptr; } void TrustedCdn::OnDestroyed(JNIEnv* env, const JavaParamRef& obj) { delete this; } -// TrustedCdn should only track primary pages and should skip subframe, -// same-document, or non-committed navigations (downloads or 204/205 responses). -void TrustedCdn::PrimaryPageChanged(content::Page& page) { - GURL publisher_url; - - // Offline pages don't have headers when they are loaded. - // TODO(bauerb): Consider storing the publisher URL on the offline page item. - if (!offline_pages::OfflinePageUtils::GetOfflinePageFromWebContents( - WebContents::FromRenderFrameHost(&page.GetMainDocument()))) { - publisher_url = embedder_support::GetPublisherURL(page); +base::android::ScopedJavaLocalRef TrustedCdn::GetPublisherUrl( + JNIEnv* env) { + if (!web_contents_ || web_contents_->IsBeingDestroyed()) { + return url::GURLAndroid::EmptyGURL(env); } - JNIEnv* env = base::android::AttachCurrentThread(); - base::android::ScopedJavaLocalRef j_publisher_url; - if (publisher_url.is_valid()) - j_publisher_url = ConvertUTF8ToJavaString(env, publisher_url.spec()); + if (offline_pages::OfflinePageUtils::GetOfflinePageFromWebContents( + web_contents_)) { + return url::GURLAndroid::EmptyGURL(env); + } - Java_TrustedCdn_setPublisherUrl(env, jobj_, j_publisher_url); + return url::GURLAndroid::FromNativeGURL( + env, + embedder_support::GetPublisherURL(web_contents_->GetPrimaryMainFrame())); } static jlong JNI_TrustedCdn_Init(JNIEnv* env, diff --git a/browser/android/trusted_cdn.h b/browser/android/trusted_cdn.h index 265988671a2580aa6897d4ccbf64c4002976f59b..d9f3bff7c4df8616009a30feb19e9fca480a3be0 100644 --- a/browser/android/trusted_cdn.h +++ b/browser/android/trusted_cdn.h @@ -6,17 +6,13 @@ #define CHROME_BROWSER_ANDROID_TRUSTED_CDN_H_ #include "base/android/scoped_java_ref.h" -#include "content/public/browser/web_contents_observer.h" - -namespace content { -class Page; -} +#include "content/public/browser/web_contents.h" // Native part of Trusted CDN publisher URL provider. Managed by Java layer. -class TrustedCdn : public content::WebContentsObserver { +class TrustedCdn { public: TrustedCdn(JNIEnv* env, const base::android::JavaParamRef& obj); - ~TrustedCdn() override; + ~TrustedCdn(); void SetWebContents( JNIEnv* env, @@ -26,12 +22,11 @@ class TrustedCdn : public content::WebContentsObserver { const base::android::JavaParamRef& obj); void OnDestroyed(JNIEnv* env, const base::android::JavaParamRef& obj); - - // content::WebContentsObserver - void PrimaryPageChanged(content::Page& page) override; + base::android::ScopedJavaLocalRef GetPublisherUrl(JNIEnv* env); private: base::android::ScopedJavaGlobalRef jobj_; + raw_ptr web_contents_; }; #endif // CHROME_BROWSER_ANDROID_TRUSTED_CDN_H_ diff --git a/browser/apps/guest_view/web_view_browsertest.cc b/browser/apps/guest_view/web_view_browsertest.cc index 4a229f579d1d93be1e76cb5d68784c441984de49..7da5bdd66224bdfbb482cbb025dc774916e67b68 100644 --- a/browser/apps/guest_view/web_view_browsertest.cc +++ b/browser/apps/guest_view/web_view_browsertest.cc @@ -44,6 +44,8 @@ #include "chrome/browser/task_manager/task_manager_browsertest_util.h" #include "chrome/browser/ui/browser_commands.h" #include "chrome/browser/ui/browser_dialogs.h" +#include "chrome/browser/ui/login/login_handler.h" +#include "chrome/browser/ui/login/login_handler_test_utils.h" #include "chrome/common/chrome_features.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" @@ -2817,7 +2819,7 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, ContextMenuLanguageSettings) { GURL page_url("http://www.google.com"); std::unique_ptr menu( - TestRenderViewContextMenu::Create(GetGuestRenderFrameHost(), page_url)); + TestRenderViewContextMenu::Create(GetGuestRenderFrameHost(), page_url,)); menu->ExecuteCommand(IDC_CONTENT_CONTEXT_LANGUAGE_SETTINGS, 0); content::WebContents* new_contents = @@ -2847,7 +2849,7 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, ContextMenusAPI_Basic) { GURL page_url("http://www.google.com"); // Create and build our test context menu. std::unique_ptr menu( - TestRenderViewContextMenu::Create(GetGuestRenderFrameHost(), page_url)); + TestRenderViewContextMenu::Create(GetGuestRenderFrameHost(), page_url,)); // Look for the extension item in the menu, and execute it. int command_id = ContextMenuMatcher::ConvertToExtensionsCustomCommandId(0); ASSERT_TRUE(menu->IsCommandIdEnabled(command_id)); @@ -3971,6 +3973,91 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestZoomBeforeNavigation) { TestHelper("testZoomBeforeNavigation", "web_view/shim", NO_TEST_SERVER); } +IN_PROC_BROWSER_TEST_F(WebViewTest, HttpAuth) { + ASSERT_TRUE(StartEmbeddedTestServer()); + LoadAppWithGuest("web_view/simple"); + + const GURL auth_url = embedded_test_server()->GetURL("/auth-basic"); + content::NavigationController* guest_controller = + &GetGuestView()->GetController(); + LoginPromptBrowserTestObserver login_observer; + login_observer.Register( + content::Source(guest_controller)); + WindowedAuthNeededObserver auth_needed(guest_controller); + WindowedAuthSuppliedObserver auth_supplied(guest_controller); + // There are two navigations occurring here. The first fails due to the need + // for auth. After it's supplied, a second navigation will succeed. + content::TestNavigationObserver nav_observer(GetGuestWebContents(), 2); + nav_observer.set_wait_event( + content::TestNavigationObserver::WaitEvent::kNavigationFinished); + + EXPECT_TRUE( + content::ExecJs(GetGuestRenderFrameHost(), + content::JsReplace("location.href = $1;", auth_url))); + auth_needed.Wait(); + + LoginHandler* login_handler = login_observer.handlers().front(); + login_handler->SetAuth(u"basicuser", u"secret"); + auth_supplied.Wait(); + nav_observer.WaitForNavigationFinished(); +} + +IN_PROC_BROWSER_TEST_F(WebViewTest, HttpAuthIdentical) { + ASSERT_TRUE(StartEmbeddedTestServer()); + LoadAppWithGuest("web_view/simple"); + + const GURL auth_url = embedded_test_server()->GetURL("/auth-basic"); + content::NavigationController* guest_controller = + &GetGuestView()->GetController(); + content::NavigationController* tab_controller = + &browser()->tab_strip_model()->GetActiveWebContents()->GetController(); + LoginPromptBrowserTestObserver login_observer; + login_observer.Register( + content::Source(guest_controller)); + login_observer.Register( + content::Source(tab_controller)); + WindowedAuthNeededObserver guest_auth_needed(guest_controller); + WindowedAuthNeededObserver tab_auth_needed(tab_controller); + WindowedAuthSuppliedObserver guest_auth_supplied(guest_controller); + // There are two navigations occurring here. The first fails due to the need + // for auth. After it's supplied, a second navigation will succeed. + content::TestNavigationObserver guest_nav_observer(GetGuestWebContents(), 2); + guest_nav_observer.set_wait_event( + content::TestNavigationObserver::WaitEvent::kNavigationFinished); + + EXPECT_TRUE( + content::ExecJs(GetGuestRenderFrameHost(), + content::JsReplace("location.href = $1;", auth_url))); + guest_auth_needed.Wait(); + + // While the login UI is showing for the app, navigate a tab to the same URL + // requiring auth. + tab_controller->LoadURL(auth_url, content::Referrer(), + ui::PAGE_TRANSITION_TYPED, std::string()); + tab_auth_needed.Wait(); + + // Both the guest and the tab should be prompting for credentials and the auth + // challenge should be the same. Normally, the login code de-duplicates + // identical challenges if multiple prompts are shown for them. However, + // credentials can't be shared across StoragePartitions. So providing + // credentials within the guest should not affect the tab. + ASSERT_EQ(2u, login_observer.handlers().size()); + LoginHandler* guest_login_handler = login_observer.handlers().front(); + LoginHandler* tab_login_handler = login_observer.handlers().back(); + EXPECT_EQ(tab_controller, + &tab_login_handler->web_contents()->GetController()); + EXPECT_TRUE(guest_login_handler->auth_info().MatchesExceptPath( + tab_login_handler->auth_info())); + + guest_login_handler->SetAuth(u"basicuser", u"secret"); + guest_auth_supplied.Wait(); + guest_nav_observer.WaitForNavigationFinished(); + + // The tab should still be prompting for credentials. + ASSERT_EQ(1u, login_observer.handlers().size()); + EXPECT_EQ(tab_login_handler, login_observer.handlers().front()); +} + namespace { class NullWebContentsDelegate : public content::WebContentsDelegate { diff --git a/browser/ash/arc/accessibility/ax_tree_source_arc.h b/browser/ash/arc/accessibility/ax_tree_source_arc.h index b7e0e6dea25fff3deae082f30a765df43756d3df..a4679012ae38730fa404523b5749a8072784f959 100644 --- a/browser/ash/arc/accessibility/ax_tree_source_arc.h +++ b/browser/ash/arc/accessibility/ax_tree_source_arc.h @@ -31,7 +31,8 @@ class Window; namespace arc { class AXTreeSourceArcTest; -using AXTreeArcSerializer = ui::AXTreeSerializer; +using AXTreeArcSerializer = ui::AXTreeSerializer>; // This class represents the accessibility tree from the focused ARC window. class AXTreeSourceArc : public ui::AXTreeSource, diff --git a/browser/ash/extensions/file_manager/private_api_dialog.cc b/browser/ash/extensions/file_manager/private_api_dialog.cc index b43401c85f993d013055e65fa949723dacb81856..5ab110c6e254d6e05a6f3c9d1c9a51c5d9e86867 100644 --- a/browser/ash/extensions/file_manager/private_api_dialog.cc +++ b/browser/ash/extensions/file_manager/private_api_dialog.cc @@ -58,14 +58,12 @@ ExtensionFunction::ResponseAction FileManagerPrivateSelectFileFunction::Run() { const absl::optional params = Params::Create(args()); EXTENSION_FUNCTION_VALIDATE(params); - std::vector file_paths; - file_paths.emplace_back(params->selected_path); Profile* profile = Profile::FromBrowserContext(browser_context()); - scoped_refptr file_system_context = - file_manager::util::GetFileSystemContextForRenderFrameHost( - profile, render_frame_host()); - const storage::FileSystemURL file_system_url = - file_system_context->CrackURLInFirstPartyContext(file_paths.back()); + base::FilePath local_path = file_manager::util::GetLocalPathFromURL( + render_frame_host(), profile, GURL(params->selected_path)); + if (local_path.empty()) { + return RespondNow(Error("Path not supported")); + } file_manager::util::GetSelectedFileInfoLocalPathOption option = file_manager::util::NO_LOCAL_PATH_RESOLUTION; @@ -75,27 +73,29 @@ ExtensionFunction::ResponseAction FileManagerPrivateSelectFileFunction::Run() { : file_manager::util::NEED_LOCAL_PATH_FOR_SAVING; } - if (file_manager::util::IsDriveLocalPath(profile, file_system_url.path()) && - file_manager::file_tasks::IsOfficeFile(file_system_url.path()) && + if (file_manager::util::IsDriveLocalPath(profile, local_path) && + file_manager::file_tasks::IsOfficeFile(local_path) && params->for_opening) { UMA_HISTOGRAM_ENUMERATION( file_manager::file_tasks::kUseOutsideDriveMetricName, file_manager::file_tasks::OfficeFilesUseOutsideDriveHook:: FILE_PICKER_SELECTION); - auto* drive_service = drive::util::GetIntegrationServiceByProfile(profile); - drive_service->ForceReSyncFile( - file_system_url.path(), - base::BindOnce( - &file_manager::util::GetSelectedFileInfo, render_frame_host(), - profile, file_paths, option, - base::BindOnce(&FileManagerPrivateSelectFileFunction:: - GetSelectedFileInfoResponse, - this, params->for_opening, params->index))); - return RespondLater(); + if (auto* drive_service = + drive::util::GetIntegrationServiceByProfile(profile)) { + drive_service->ForceReSyncFile( + local_path, + base::BindOnce( + &file_manager::util::GetSelectedFileInfo, profile, + std::vector({local_path}), option, + base::BindOnce(&FileManagerPrivateSelectFileFunction:: + GetSelectedFileInfoResponse, + this, params->for_opening, params->index))); + return RespondLater(); + } } file_manager::util::GetSelectedFileInfo( - render_frame_host(), profile, file_paths, option, + profile, {local_path}, option, base::BindOnce( &FileManagerPrivateSelectFileFunction::GetSelectedFileInfoResponse, this, params->for_opening, params->index)); @@ -134,42 +134,44 @@ ExtensionFunction::ResponseAction FileManagerPrivateSelectFilesFunction::Run() { should_return_local_path_ = params->should_return_local_path; Profile* const profile = Profile::FromBrowserContext(browser_context()); - scoped_refptr file_system_context = - file_manager::util::GetFileSystemContextForRenderFrameHost( - profile, render_frame_host()); - std::vector resync_paths; for (const auto& selected_path : params->selected_paths) { - file_urls_.emplace_back(selected_path); - const storage::FileSystemURL file_system_url = - file_system_context->CrackURLInFirstPartyContext(file_urls_.back()); + base::FilePath local_path = file_manager::util::GetLocalPathFromURL( + render_frame_host(), profile, GURL(selected_path)); + if (local_path.empty()) { + continue; + } - if (file_manager::util::IsDriveLocalPath(profile, file_system_url.path()) && - file_manager::file_tasks::IsOfficeFile(file_system_url.path())) { + if (file_manager::util::IsDriveLocalPath(profile, local_path) && + file_manager::file_tasks::IsOfficeFile(local_path)) { UMA_HISTOGRAM_ENUMERATION( file_manager::file_tasks::kUseOutsideDriveMetricName, file_manager::file_tasks::OfficeFilesUseOutsideDriveHook:: FILE_PICKER_SELECTION); - resync_paths.push_back(file_system_url.path()); + ++resync_files_remaining_; } + local_paths_.push_back(std::move(local_path)); } - resync_files_remaining_ = resync_paths.size(); - if (!resync_paths.empty()) { - auto* drive_service = drive::util::GetIntegrationServiceByProfile(profile); - if (drive_service) { - for (const auto& path : resync_paths) { - drive_service->ForceReSyncFile( - path, - base::BindOnce(&FileManagerPrivateSelectFilesFunction::OnReSyncFile, - this)); + if (resync_files_remaining_ > 0) { + if (auto* drive_service = + drive::util::GetIntegrationServiceByProfile(profile)) { + // This loop must be done after calculating `resync_files_remaining_` as + // ForceReSyncFile() may return synchronously. + for (const base::FilePath& local_path : local_paths_) { + if (file_manager::util::IsDriveLocalPath(profile, local_path) && + file_manager::file_tasks::IsOfficeFile(local_path)) { + drive_service->ForceReSyncFile( + local_path, + base::BindOnce( + &FileManagerPrivateSelectFilesFunction::OnReSyncFile, this)); + } } return RespondLater(); } } file_manager::util::GetSelectedFileInfo( - render_frame_host(), Profile::FromBrowserContext(browser_context()), - file_urls_, + Profile::FromBrowserContext(browser_context()), local_paths_, params->should_return_local_path ? file_manager::util::NEED_LOCAL_PATH_FOR_OPENING : file_manager::util::NO_LOCAL_PATH_RESOLUTION, @@ -185,8 +187,7 @@ void FileManagerPrivateSelectFilesFunction::OnReSyncFile() { return; } file_manager::util::GetSelectedFileInfo( - render_frame_host(), Profile::FromBrowserContext(browser_context()), - file_urls_, + Profile::FromBrowserContext(browser_context()), local_paths_, should_return_local_path_ ? file_manager::util::NEED_LOCAL_PATH_FOR_OPENING : file_manager::util::NO_LOCAL_PATH_RESOLUTION, diff --git a/browser/ash/extensions/file_manager/private_api_dialog.h b/browser/ash/extensions/file_manager/private_api_dialog.h index c44445c5211bf37414074f164233db089072ea17..75fec108fe1bebec2c9959a4411427c396384c1e 100644 --- a/browser/ash/extensions/file_manager/private_api_dialog.h +++ b/browser/ash/extensions/file_manager/private_api_dialog.h @@ -75,7 +75,7 @@ class FileManagerPrivateSelectFilesFunction : public LoggedExtensionFunction { const std::vector& files); bool should_return_local_path_; - std::vector file_urls_; + std::vector local_paths_; int resync_files_remaining_ = 0; }; diff --git a/browser/ash/extensions/file_manager/private_api_util.cc b/browser/ash/extensions/file_manager/private_api_util.cc index 77b3892f4914e2ed60f84c6d12a7050e0d5d8070..73bdd9205cc0503be85b5dc001dfff5f4773def7 100644 --- a/browser/ash/extensions/file_manager/private_api_util.cc +++ b/browser/ash/extensions/file_manager/private_api_util.cc @@ -667,26 +667,15 @@ base::FilePath GetLocalPathFromURL(content::RenderFrameHost* render_frame_host, return filesystem_url.path(); } -void GetSelectedFileInfo(content::RenderFrameHost* render_frame_host, - Profile* profile, - const std::vector& file_urls, +void GetSelectedFileInfo(Profile* profile, + std::vector local_paths, GetSelectedFileInfoLocalPathOption local_path_option, GetSelectedFileInfoCallback callback) { - DCHECK(render_frame_host); - DCHECK(profile); - std::unique_ptr params( new GetSelectedFileInfoParams); params->local_path_option = local_path_option; params->callback = std::move(callback); - - for (const GURL& url : file_urls) { - base::FilePath path = GetLocalPathFromURL(render_frame_host, profile, url); - if (!path.empty()) { - DVLOG(1) << "Selected: file path: " << path; - params->file_paths.push_back(std::move(path)); - } - } + params->file_paths = std::move(local_paths); base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask( FROM_HERE, diff --git a/browser/ash/extensions/file_manager/private_api_util.h b/browser/ash/extensions/file_manager/private_api_util.h index 9dcc441266edd7bf320c2b88e922c21a8abb31e4..ab0d8cbc44c6b9a91ae38956964b950a095d737e 100644 --- a/browser/ash/extensions/file_manager/private_api_util.h +++ b/browser/ash/extensions/file_manager/private_api_util.h @@ -163,10 +163,9 @@ enum GetSelectedFileInfoLocalPathOption { NEED_LOCAL_PATH_FOR_SAVING, }; -// Gets the information for |file_urls|. -void GetSelectedFileInfo(content::RenderFrameHost* render_frame_host, - Profile* profile, - const std::vector& file_urls, +// Gets the information for |local_paths|. +void GetSelectedFileInfo(Profile* profile, + std::vector local_paths, GetSelectedFileInfoLocalPathOption local_path_option, GetSelectedFileInfoCallback callback); diff --git a/browser/ash/login/error_screen_browsertest.cc b/browser/ash/login/error_screen_browsertest.cc index cf36c8c0d920f2a8dd98daafad49d1942a357d49..2a52d1cbde9bfbd1603bd028ef9fd6d29dcad2f4 100644 --- a/browser/ash/login/error_screen_browsertest.cc +++ b/browser/ash/login/error_screen_browsertest.cc @@ -45,6 +45,9 @@ namespace { constexpr char kWifiServiceName[] = "stub_wifi"; constexpr char kWifiNetworkName[] = "wifi-test-network"; +const test::UIPath kNetworkBackButton = {"error-message", "backButton"}; +const test::UIPath kNetworkConfigureScreenContinueButton = {"error-message", + "continueButton"}; const test::UIPath kErrorMessageGuestSigninLink = {"error-message", "error-guest-signin-link"}; @@ -288,6 +291,16 @@ class KioskErrorScreenTest : public MixinBasedInProcessBrowserTest { MixinBasedInProcessBrowserTest::SetUpInProcessBrowserTestFixture(); } + void SetOnline(bool is_online) { + network_helper_->SetServiceProperty(kWifiServiceName, shill::kStateProperty, + is_online + ? base::Value(shill::kStateOnline) + : base::Value(shill::kStateIdle)); + // Network modification notifications are posted asynchronously. Wait until + // idle to ensure observers are notified. + base::RunLoop().RunUntilIdle(); + } + void SetUpOnMainThread() override { network_helper_ = std::make_unique( /*use_default_devices_and_services=*/false); @@ -304,6 +317,18 @@ class KioskErrorScreenTest : public MixinBasedInProcessBrowserTest { MixinBasedInProcessBrowserTest::TearDownOnMainThread(); } + void SetBlockAppLaunch(bool block) { + if (block) { + block_app_launch_override_ = + KioskLaunchController::BlockAppLaunchForTesting(); + } else { + block_app_launch_override_.reset(); + } + } + + protected: + LoginManagerMixin login_mixin_{&mixin_host_}; + private: void AddKioskAppToDevicePolicy() { std::unique_ptr device_policy_update = @@ -319,6 +344,7 @@ class KioskErrorScreenTest : public MixinBasedInProcessBrowserTest { std::unique_ptr> skip_splash_wait_override_; std::unique_ptr> network_wait_override_; + std::unique_ptr> block_app_launch_override_; DeviceStateMixin device_state_{ &mixin_host_, DeviceStateMixin::State::OOBE_COMPLETED_CLOUD_ENROLLED}; @@ -326,8 +352,6 @@ class KioskErrorScreenTest : public MixinBasedInProcessBrowserTest { EmbeddedTestServerSetupMixin embedded_test_server_setup_{ &mixin_host_, embedded_test_server()}; KioskAppsMixin kiosk_apps_{&mixin_host_, embedded_test_server()}; - - LoginManagerMixin login_manager_{&mixin_host_, {}}; }; // Verify that certificate manager dialog opens. @@ -349,4 +373,50 @@ IN_PROC_BROWSER_TEST_F(KioskErrorScreenTest, OpenCertificateConfig) { waiter.Wait(); } +// The existence of user pods in the signin screen can influence +// the presence of the back button in the network configuration +// screen. Add a regular user to cover this case. +IN_PROC_BROWSER_TEST_F(KioskErrorScreenTest, + PRE_NoBackButtonInNetworkConfigureScreenAfterTimeout) { + login_mixin_.LoginAsNewRegularUser(); +} + +IN_PROC_BROWSER_TEST_F(KioskErrorScreenTest, + NoBackButtonInNetworkConfigureScreenAfterTimeout) { + KioskAppsMixin::WaitForAppsButton(); + EXPECT_TRUE(LoginScreenTestApi::IsAppsButtonShown()); + ASSERT_TRUE(LoginScreenTestApi::LaunchApp(KioskAppsMixin::kKioskAppId)); + EXPECT_TRUE(LoginScreenTestApi::IsOobeDialogVisible()); + OobeScreenWaiter(ErrorScreenView::kScreenId).Wait(); + test::OobeJS().ExpectPathDisplayed(false, kNetworkBackButton); + test::OobeJS().ExpectPathDisplayed(false, + kNetworkConfigureScreenContinueButton); +} + +// The existence of user pods in the signin screen can influence +// the presence of the back button in the network configuration +// screen. Add a regular user to cover this case. +IN_PROC_BROWSER_TEST_F(KioskErrorScreenTest, + PRE_NoBackButtonInNetworkConfigureScreenAfterShortcut) { + login_mixin_.LoginAsNewRegularUser(); +} + +IN_PROC_BROWSER_TEST_F(KioskErrorScreenTest, + NoBackButtonInNetworkConfigureScreenAfterShortcut) { + SetOnline(true); + KioskAppsMixin::WaitForAppsButton(); + EXPECT_TRUE(LoginScreenTestApi::IsAppsButtonShown()); + ASSERT_TRUE(LoginScreenTestApi::LaunchApp(KioskAppsMixin::kKioskAppId)); + SetBlockAppLaunch(true); + OobeScreenWaiter(AppLaunchSplashScreenView::kScreenId).Wait(); + + ASSERT_TRUE(LoginScreenTestApi::PressAccelerator( + ui::Accelerator(ui::VKEY_N, ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN))); + OobeScreenWaiter(ErrorScreenView::kScreenId).Wait(); + EXPECT_TRUE(LoginScreenTestApi::IsOobeDialogVisible()); + test::OobeJS().ExpectPathDisplayed(false, kNetworkBackButton); + test::OobeJS().ExpectPathDisplayed(true, + kNetworkConfigureScreenContinueButton); +} + } // namespace ash diff --git a/browser/ash/login/screens/error_screen.cc b/browser/ash/login/screens/error_screen.cc index f6bce7aee0413c1e9723e6d2cc38f102cf56521e..c4f16aedd0ae3da1677aa1b37ae5d73e4207adbc 100644 --- a/browser/ash/login/screens/error_screen.cc +++ b/browser/ash/login/screens/error_screen.cc @@ -191,9 +191,7 @@ void ErrorScreen::ShowConnectingIndicator(bool show) { } void ErrorScreen::SetIsPersistentError(bool is_persistent) { - if (view_) { - view_->SetIsPersistentError(is_persistent); - } + is_persistent_ = is_persistent; } base::CallbackListSubscription ErrorScreen::RegisterConnectRequestCallback( @@ -262,7 +260,9 @@ void ErrorScreen::ShowImpl() { return; } - view_->Show(); + const bool is_closeable = + LoginDisplayHost::default_host()->HasUserPods() && !is_persistent_; + view_->ShowScreenWithParam(is_closeable); LOG(WARNING) << "Network error screen message is shown"; NetworkHandler::Get()->network_state_handler()->RequestPortalDetection(); } diff --git a/browser/ash/login/screens/error_screen.h b/browser/ash/login/screens/error_screen.h index 41ebc1f3bc8150d4bcca51fd3536fbb83701c997..fcb62d9ccc243893e03c0f2f9f06c7fc862c9e48 100644 --- a/browser/ash/login/screens/error_screen.h +++ b/browser/ash/login/screens/error_screen.h @@ -82,7 +82,7 @@ class ErrorScreen : public BaseScreen, // Toggles the connection pending indicator. void ShowConnectingIndicator(bool show); - // Makes error persistent (e.g. non-closable). + // Makes error persistent (e.g. non-closeable). void SetIsPersistentError(bool is_persistent); // Register a callback to be invoked when the user indicates that an attempt @@ -155,6 +155,8 @@ class ErrorScreen : public BaseScreen, void StartGuestSessionAfterOwnershipCheck( DeviceSettingsService::OwnershipStatus ownership_status); + bool is_persistent_ = false; + base::WeakPtr view_; // We have the guest login logic in this screen because it might be required diff --git a/browser/ash/login/screens/mock_error_screen.h b/browser/ash/login/screens/mock_error_screen.h index d72bfd2953512e9c4b8a9c682d56e2baec053c30..d96216656a571cf9d8a1e3e0523adea351dff48e 100644 --- a/browser/ash/login/screens/mock_error_screen.h +++ b/browser/ash/login/screens/mock_error_screen.h @@ -34,7 +34,7 @@ class MockErrorScreenView : public ErrorScreenView { MockErrorScreenView(); ~MockErrorScreenView() override; - MOCK_METHOD0(Show, void()); + MOCK_METHOD1(ShowScreenWithParam, void(bool is_closeable)); MOCK_METHOD0(Hide, void()); MOCK_METHOD1(ShowOobeScreen, void(OobeScreenId screen)); MOCK_METHOD1(SetErrorStateCode, void(NetworkError::ErrorState error_state)); @@ -43,7 +43,6 @@ class MockErrorScreenView : public ErrorScreenView { MOCK_METHOD1(SetOfflineSigninAllowed, void(bool value)); MOCK_METHOD1(SetShowConnectingIndicator, void(bool value)); MOCK_METHOD1(SetUIState, void(NetworkError::UIState ui_state)); - MOCK_METHOD1(SetIsPersistentError, void(bool is_persistent)); MOCK_METHOD0(OnCancelButtonClicked, void()); }; diff --git a/browser/ash/power/renderer_freezer_unittest.cc b/browser/ash/power/renderer_freezer_unittest.cc index 358208fc8bfb4a13af31dd83b2e4a73eb4641aa1..a65be4a76549d547a3b65c3dc8910071d14ef3cb 100644 --- a/browser/ash/power/renderer_freezer_unittest.cc +++ b/browser/ash/power/renderer_freezer_unittest.cc @@ -272,8 +272,8 @@ class RendererFreezerTestWithExtensions : public RendererFreezerTest { rph_factory->CreateRenderProcessHost(profile_, site_instance.get()); // Fake that the RenderProcessHost is hosting the gcm app. - extensions::ProcessMap::Get(profile_) - ->Insert(extension->id(), rph->GetID(), site_instance->GetId()); + extensions::ProcessMap::Get(profile_)->Insert(extension->id(), + rph->GetID()); SimulateRenderProcessHostCreated(rph); } diff --git a/browser/ash/system_extensions/system_extensions_install_manager.cc b/browser/ash/system_extensions/system_extensions_install_manager.cc index 801fbc8934a41cec089b4f0da5b8a646839eadc1..19f9cdb7d0d3314598bead9f9421df4266bc32a0 100644 --- a/browser/ash/system_extensions/system_extensions_install_manager.cc +++ b/browser/ash/system_extensions/system_extensions_install_manager.cc @@ -182,13 +182,12 @@ void SystemExtensionsInstallManager::Uninstall( return; } - const url::Origin& origin = url::Origin::Create(system_extension->base_url); - // Uninstallation Step #1: Unregister the Service Worker. service_worker_manager_->UnregisterServiceWorker(system_extension_id); // Uninstallation Step #2: Remove the WebUIConfig for the System Extension. - content::WebUIConfigMap::GetInstance().RemoveConfig(origin); + content::WebUIConfigMap::GetInstance().RemoveConfig( + system_extension->base_url); // Installation Step #3: Remove the System Extension from persistent storage. persistent_storage_->Remove(system_extension_id); diff --git a/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc b/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc index 70e852d798de7f1b7180d1f7e7ff59deaaf12549..40a367103c3380fba6fb14b790dff4a558ad5ded 100644 --- a/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc +++ b/browser/ash/web_applications/demo_mode_app_integration_browsertest.cc @@ -76,7 +76,7 @@ class DemoModeAppIntegrationTestBase : public ash::SystemWebAppIntegrationTest { base::ScopedAllowBlockingForTesting allow_blocking; ASSERT_TRUE(component_dir_.CreateUniqueTempDir()); content::WebUIConfigMap::GetInstance().RemoveConfig( - url::Origin::Create(GURL(ash::kChromeUntrustedUIDemoModeAppURL))); + GURL(ash::kChromeUntrustedUIDemoModeAppURL)); content::WebUIConfigMap::GetInstance().AddUntrustedWebUIConfig( std::make_unique( base::BindLambdaForTesting( diff --git a/browser/chrome_content_browser_client.cc b/browser/chrome_content_browser_client.cc index 9b6b6e2fd59629101f6334f04e387680d8280c6d..a3d76974a3722879c4dadc9cba5abe8cc8278735 100644 --- a/browser/chrome_content_browser_client.cc +++ b/browser/chrome_content_browser_client.cc @@ -2437,15 +2437,6 @@ void ChromeContentBrowserClient::SiteInstanceGotProcess( extra_parts_[i]->SiteInstanceGotProcess(site_instance); } -void ChromeContentBrowserClient::SiteInstanceDeleting( - SiteInstance* site_instance) { - if (!site_instance->HasProcess()) - return; - - for (size_t i = 0; i < extra_parts_.size(); ++i) - extra_parts_[i]->SiteInstanceDeleting(site_instance); -} - bool ChromeContentBrowserClient::ShouldSwapBrowsingInstancesForNavigation( SiteInstance* site_instance, const GURL& current_effective_url, @@ -6672,8 +6663,8 @@ bool ChromeContentBrowserClient::HandleWebUI( if (!ChromeWebUIControllerFactory::GetInstance()->UseWebUIForURL( browser_context, *url) && - !content::WebUIConfigMap::GetInstance().GetConfig( - browser_context, url::Origin::Create(*url))) { + !content::WebUIConfigMap::GetInstance().GetConfig(browser_context, + *url)) { return false; } diff --git a/browser/chrome_content_browser_client.h b/browser/chrome_content_browser_client.h index bac07c047c425db81a377ca52ae84dfff0de4351..400e5995b8957dfa730f8706b1cfbcc6f7cf9fde 100644 --- a/browser/chrome_content_browser_client.h +++ b/browser/chrome_content_browser_client.h @@ -229,7 +229,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient { bool ShouldEmbeddedFramesTryToReuseExistingProcess( content::RenderFrameHost* outermost_main_frame) override; void SiteInstanceGotProcess(content::SiteInstance* site_instance) override; - void SiteInstanceDeleting(content::SiteInstance* site_instance) override; bool ShouldSwapBrowsingInstancesForNavigation( content::SiteInstance* site_instance, const GURL& current_effective_url, diff --git a/browser/chrome_content_browser_client_parts.h b/browser/chrome_content_browser_client_parts.h index 485e534532af666678f96cea463efa3c26ed1117..017b9d71284d663308511dc18282ac7a3f984747 100644 --- a/browser/chrome_content_browser_client_parts.h +++ b/browser/chrome_content_browser_client_parts.h @@ -47,7 +47,6 @@ class ChromeContentBrowserClientParts { virtual void RenderProcessWillLaunch(content::RenderProcessHost* host) {} virtual void SiteInstanceGotProcess(content::SiteInstance* site_instance) {} - virtual void SiteInstanceDeleting(content::SiteInstance* site_instance) {} virtual void OverrideWebkitPrefs(content::WebContents* web_contents, blink::web_pref::WebPreferences* web_prefs) { } diff --git a/browser/chromeos/app_mode/app_session_browser_window_handler.cc b/browser/chromeos/app_mode/app_session_browser_window_handler.cc index a069b33eb4e102cf155729d71bc1cf9e74f2a266..a599e9880f8378064dbb5df9e5dc78219d1752bd 100644 --- a/browser/chromeos/app_mode/app_session_browser_window_handler.cc +++ b/browser/chromeos/app_mode/app_session_browser_window_handler.cc @@ -5,6 +5,7 @@ #include "chrome/browser/chromeos/app_mode/app_session_browser_window_handler.h" #include +#include "base/check_deref.h" #include "base/metrics/histogram_functions.h" #include "base/task/single_thread_task_runner.h" #include "chrome/browser/chromeos/app_mode/app_session_policies.h" @@ -34,6 +35,20 @@ void MakeWindowResizable(BrowserWindow* window) { } } +std::string GetUrlOfActiveTab(const Browser* browser) { + content::WebContents* active_tab = + browser->tab_strip_model()->GetActiveWebContents(); + return active_tab ? active_tab->GetVisibleURL().spec() : std::string(); +} + +void CloseAllBrowserWindows() { + for (auto* browser : CHECK_DEREF(BrowserList::GetInstance())) { + LOG(WARNING) << "kiosk: Closing unexpected browser window with url: " + << GetUrlOfActiveTab(browser); + browser->window()->Close(); + } +} + } // namespace const char kKioskNewBrowserWindowHistogram[] = "Kiosk.NewBrowserWindow"; @@ -64,6 +79,11 @@ AppSessionBrowserWindowHandler::AppSessionBrowserWindowHandler( weak_ptr_factory_.GetWeakPtr())); #endif // BUILDFLAG(IS_CHROMEOS_LACROS) + if (!web_app_name.has_value()) { + // If this is ChromeApp kiosk, close all preexisting browser windows to + // avoid potential kiosk escapes. + CloseAllBrowserWindows(); + } BrowserList::AddObserver(this); } @@ -72,10 +92,7 @@ AppSessionBrowserWindowHandler::~AppSessionBrowserWindowHandler() { } void AppSessionBrowserWindowHandler::HandleNewBrowserWindow(Browser* browser) { - content::WebContents* active_tab = - browser->tab_strip_model()->GetActiveWebContents(); - std::string url_string = - active_tab ? active_tab->GetVisibleURL().spec() : std::string(); + std::string url_string = GetUrlOfActiveTab(browser); if (KioskSettingsNavigationThrottle::IsSettingsPage(url_string)) { base::UmaHistogramEnumeration(kKioskNewBrowserWindowHistogram, diff --git a/browser/chromeos/app_mode/app_session_unittest.cc b/browser/chromeos/app_mode/app_session_unittest.cc index c36fe446904c705134ab1908944404e2e61eb4fd..2f265f0eaf9a9a70759b9f0e59dc03c963ae9e17 100644 --- a/browser/chromeos/app_mode/app_session_unittest.cc +++ b/browser/chromeos/app_mode/app_session_unittest.cc @@ -13,6 +13,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/test/bind.h" #include "base/test/metrics/histogram_tester.h" +#include "base/test/test_future.h" #include "chrome/browser/chromeos/app_mode/app_session.h" #include "chrome/browser/chromeos/app_mode/app_session_browser_window_handler.h" #include "chrome/browser/chromeos/app_mode/app_session_metrics_service.h" @@ -362,6 +363,30 @@ TEST_F(AppSessionTest, WebKioskTracksBrowserCreation) { histogram()->ExpectTotalCount(kKioskSessionDurationInDaysNormalHistogram, 0); } +TEST_F(AppSessionTest, ChromeAppKioskShouldClosePreexistingBrowsers) { + std::unique_ptr preexisting_browser = CreateBrowserWithTestWindow(); + base::test::TestFuture closed_future; + + static_cast(preexisting_browser->window()) + ->SetCloseCallback(closed_future.GetCallback()); + + StartChromeAppKioskSession(); + + ASSERT_TRUE(closed_future.IsReady()); +} + +TEST_F(AppSessionTest, WebKioskShouldNotClosePreexistingBrowsers) { + std::unique_ptr preexisting_browser = CreateBrowserWithTestWindow(); + base::test::TestFuture closed_future; + + static_cast(preexisting_browser->window()) + ->SetCloseCallback(closed_future.GetCallback()); + + StartWebKioskSession(); + + ASSERT_FALSE(closed_future.IsReady()); +} + TEST_F(AppSessionTest, ChromeAppKioskSessionState) { StartChromeAppKioskSession(); histogram()->ExpectBucketCount(kKioskSessionStateHistogram, diff --git a/browser/devtools/protocol/devtools_protocol_browsertest.cc b/browser/devtools/protocol/devtools_protocol_browsertest.cc index db208a52807447178781e5ff98799bba33831dd3..0d5388b695662d4d7a6f4240f7ad50ab1af3ef74 100644 --- a/browser/devtools/protocol/devtools_protocol_browsertest.cc +++ b/browser/devtools/protocol/devtools_protocol_browsertest.cc @@ -667,6 +667,7 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, UntrustedClient) { "Memory.prepareForLeakDetection")); // Implemented in content EXPECT_FALSE(SendCommandSync("Cast.enable")); // Implemented in content EXPECT_FALSE(SendCommandSync("Storage.getCookies")); + EXPECT_FALSE(SendCommandSync("Network.getAllCookies")); EXPECT_TRUE(SendCommandSync("Accessibility.enable")); } diff --git a/browser/extensions/BUILD.gn b/browser/extensions/BUILD.gn index 7d42a77117dca9453da2632f5843ba1e105f15d8..aa1a7f0bf5c897e6536475d00bbb7b8d7f0f4769 100644 --- a/browser/extensions/BUILD.gn +++ b/browser/extensions/BUILD.gn @@ -990,6 +990,12 @@ static_library("extensions") { "//url", ] + if (is_ohos) { + deps += [ + "//components/services/unzip:in_process", + ] + } + if (is_linux || is_mac || is_win) { sources += [ "api/system_indicator/system_indicator_api.cc", diff --git a/browser/extensions/api/context_menus/extension_context_menu_browsertest.cc b/browser/extensions/api/context_menus/extension_context_menu_browsertest.cc index 85f8c9a059a5c13bdc6d21683d169b36fbf7e30d..585f572fd9029bc9a882e684923ebbfd71420c5c 100644 --- a/browser/extensions/api/context_menus/extension_context_menu_browsertest.cc +++ b/browser/extensions/api/context_menus/extension_context_menu_browsertest.cc @@ -576,10 +576,10 @@ IN_PROC_BROWSER_TEST_P(ExtensionContextMenuLazyTest, Patterns) { // Now check with a non-matching url. GURL test_url("http://www.test.com"); - ASSERT_FALSE( - MenuHasItemWithLabel(test_url, GURL(), false, std::string("test_item1"))); - ASSERT_FALSE( - MenuHasItemWithLabel(test_url, GURL(), false, std::string("test_item2"))); + ASSERT_FALSE(MenuHasItemWithLabel(test_url, GURL(), false, + std::string("test_item1"))); + ASSERT_FALSE(MenuHasItemWithLabel(test_url, GURL(), false, + std::string("test_item2"))); } // Tests registering an item with a very long title that should get truncated in @@ -736,7 +736,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionContextMenuPersistentTest, Separators) { GURL url("http://www.google.com/"); std::unique_ptr menu( - TestRenderViewContextMenu::Create(GetWebContents(), url)); + TestRenderViewContextMenu::Create(GetWebContents(), url,)); // The top-level item should be an "automagic parent" with the extension's // name. diff --git a/browser/extensions/api/developer_private/developer_private_api_unittest.cc b/browser/extensions/api/developer_private/developer_private_api_unittest.cc index 50d86b85f0667afd93e85604c43903d938cab58e..8463bf0232570778e86cce2e7b6ffc40e16b34fd 100644 --- a/browser/extensions/api/developer_private/developer_private_api_unittest.cc +++ b/browser/extensions/api/developer_private/developer_private_api_unittest.cc @@ -21,6 +21,7 @@ #include "base/values.h" #include "chrome/browser/extensions/chrome_test_extension_loader.h" #include "chrome/browser/extensions/error_console/error_console.h" +#include "chrome/browser/extensions/extension_install_prompt_show_params.h" #include "chrome/browser/extensions/extension_management.h" #include "chrome/browser/extensions/extension_management_test_util.h" #include "chrome/browser/extensions/extension_service.h" @@ -334,6 +335,11 @@ class DeveloperPrivateApiUnitTest : public ExtensionServiceTestWithInstall { void SetUp() override; void TearDown() override; + // This test does not create a root window. Because of this, + // ScopedDisableRootChecking needs to be used (which disables the root window + // check). + test::ScopedDisableRootChecking disable_root_checking_; + // The browser (and accompanying window). std::unique_ptr browser_window_; std::unique_ptr browser_; diff --git a/browser/extensions/api/management/management_api_unittest.cc b/browser/extensions/api/management/management_api_unittest.cc index 9096df8f9e714c990bdc8c8e64ebf4252fd8b7ea..1de0d6be151ed13556c1a54a9ff4d3004d8e8e17 100644 --- a/browser/extensions/api/management/management_api_unittest.cc +++ b/browser/extensions/api/management/management_api_unittest.cc @@ -14,6 +14,7 @@ #include "base/test/metrics/histogram_tester.h" #include "base/types/optional_ref.h" #include "build/chromeos_buildflags.h" +#include "chrome/browser/extensions/extension_install_prompt_show_params.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service_test_base.h" #include "chrome/browser/extensions/extension_service_test_with_install.h" @@ -115,6 +116,10 @@ class ManagementApiUnitTest : public ExtensionServiceTestWithInstall { void TearDown() override; private: + // This test does not create a root window. Because of this, + // ScopedDisableRootChecking needs to be used (which disables the root window + // check). + test::ScopedDisableRootChecking disable_root_checking_; // The browser (and accompanying window). std::unique_ptr browser_window_; std::unique_ptr browser_; diff --git a/browser/extensions/api/resources_private/resources_private_api.cc b/browser/extensions/api/resources_private/resources_private_api.cc index f4a857da15d609802d01a1992c5a35efe2ca6362..04643cd68e06449a58ff2a15f227f19a0f01b320 100644 --- a/browser/extensions/api/resources_private/resources_private_api.cc +++ b/browser/extensions/api/resources_private/resources_private_api.cc @@ -91,7 +91,7 @@ ExtensionFunction::ResponseAction ResourcesPrivateGetStringsFunction::Run() { enable_printing = IsUserProfile(profile); #endif // BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_OHOS) - // The printing service is temporarily not supported on mobile device. + // The printing service is temporarily supported on pc device. enable_printing = (*base::CommandLine::ForCurrentProcess()) .HasSwitch(switches::kEnablePrinting); #endif diff --git a/browser/extensions/api/webstore_private/webstore_private_unittest.cc b/browser/extensions/api/webstore_private/webstore_private_unittest.cc index 094233d53e858c7a40b21abb63b3f47e9e92b4ac..df101d0fa1a8bc317d03c8ec701d247ca1b010c7 100644 --- a/browser/extensions/api/webstore_private/webstore_private_unittest.cc +++ b/browser/extensions/api/webstore_private/webstore_private_unittest.cc @@ -15,6 +15,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "chrome/browser/extensions/extension_api_unittest.h" +#include "chrome/browser/extensions/extension_install_prompt_show_params.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/test_extension_system.h" #include "chrome/common/extensions/extension_constants.h" @@ -318,6 +319,10 @@ class WebstorePrivateBeginInstallWithManifest3Test ExtensionService* extension_service() { return service_; } private: + // This test does not create a root window. Because of this, + // ScopedDisableRootChecking needs to be used (which disables the root window + // check). + test::ScopedDisableRootChecking disable_root_checking_; raw_ptr service_ = nullptr; }; diff --git a/browser/extensions/chrome_content_browser_client_extensions_part.cc b/browser/extensions/chrome_content_browser_client_extensions_part.cc index 56b44aedab05b22c4ee0f364beb942ebed5ed91c..9260483115460dc29610224e881e8f315a641a60 100644 --- a/browser/extensions/chrome_content_browser_client_extensions_part.cc +++ b/browser/extensions/chrome_content_browser_client_extensions_part.cc @@ -727,27 +727,13 @@ void ChromeContentBrowserClientExtensionsPart::SiteInstanceGotProcess( if (site_instance->IsGuest()) return; + // Note that this may be called more than once for multiple instances + // of the same extension, such as when the same hosted app is opened in + // unrelated tabs. This call will ignore duplicate insertions, which is fine, + // since we only need to track if the extension is in the process, rather + // than how many instances it has in that process. ProcessMap::Get(context)->Insert(extension->id(), - site_instance->GetProcess()->GetID(), - site_instance->GetId()); -} - -void ChromeContentBrowserClientExtensionsPart::SiteInstanceDeleting( - SiteInstance* site_instance) { - BrowserContext* context = site_instance->GetBrowserContext(); - ExtensionRegistry* registry = ExtensionRegistry::Get(context); - if (!registry) - return; - - const Extension* extension = - registry->enabled_extensions().GetExtensionOrAppByURL( - site_instance->GetSiteURL()); - if (!extension) - return; - - ProcessMap::Get(context)->Remove(extension->id(), - site_instance->GetProcess()->GetID(), - site_instance->GetId()); + site_instance->GetProcess()->GetID()); } bool ChromeContentBrowserClientExtensionsPart:: diff --git a/browser/extensions/chrome_content_browser_client_extensions_part.h b/browser/extensions/chrome_content_browser_client_extensions_part.h index 83248cab4f4f5c16604f1b0255bbe7b16489ede6..4a117a4fc3713a8f14b38f3d411fcd7ec9a7cbf4 100644 --- a/browser/extensions/chrome_content_browser_client_extensions_part.h +++ b/browser/extensions/chrome_content_browser_client_extensions_part.h @@ -121,7 +121,6 @@ class ChromeContentBrowserClientExtensionsPart // ChromeContentBrowserClientParts: void RenderProcessWillLaunch(content::RenderProcessHost* host) override; void SiteInstanceGotProcess(content::SiteInstance* site_instance) override; - void SiteInstanceDeleting(content::SiteInstance* site_instance) override; void OverrideWebkitPrefs(content::WebContents* web_contents, blink::web_pref::WebPreferences* web_prefs) override; bool OverrideWebPreferencesAfterNavigation( diff --git a/browser/extensions/extension_install_prompt_show_params.cc b/browser/extensions/extension_install_prompt_show_params.cc index 0541b69a5d8413ac9e5e7474c996cfe8b0df587f..797e85a90dbb442b3b29dcb58fcb34444ef5ff37 100644 --- a/browser/extensions/extension_install_prompt_show_params.cc +++ b/browser/extensions/extension_install_prompt_show_params.cc @@ -6,32 +6,51 @@ #include +#include "base/check_is_test.h" #include "base/memory/raw_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/profiles/profile.h" #include "content/public/browser/web_contents.h" #include "ui/views/native_window_tracker.h" +#if defined(USE_AURA) +#include "ui/aura/window.h" +#endif + namespace { -gfx::NativeWindow NativeWindowForWebContents(content::WebContents* contents) { - if (!contents) - return nullptr; +#if defined(USE_AURA) +bool g_root_checking_enabled = true; +#endif - return contents->GetTopLevelNativeWindow(); +bool RootCheck(gfx::NativeWindow window) { +#if defined(USE_AURA) + // If the window is not contained in a root window, then it's not connected + // to a display and can't be used as the context. To do otherwise results in + // checks later on assuming context has a root. + return !g_root_checking_enabled || (window->GetRootWindow() != nullptr); +#else + return true; +#endif } - } // namespace ExtensionInstallPromptShowParams::ExtensionInstallPromptShowParams( content::WebContents* contents) - : profile_(contents - ? Profile::FromBrowserContext(contents->GetBrowserContext()) - : nullptr), - parent_web_contents_(contents ? contents->GetWeakPtr() : nullptr), - parent_window_(NativeWindowForWebContents(contents)) { - if (parent_window_) - native_window_tracker_ = views::NativeWindowTracker::Create(parent_window_); + : profile_(Profile::FromBrowserContext(contents->GetBrowserContext())), + parent_web_contents_(contents->GetWeakPtr()) { + DCHECK(profile_); + DCHECK(parent_web_contents_); + + if (!parent_web_contents_->GetTopLevelNativeWindow()) { + // Some tests construct this with a WebContents that has no window. If we + // keep web contents in this case, WasParentDestroyed() will always return + // true, even though there is no real window to check. Thus, there is no + // window to track here. Reset the web contents in this case, and just keep + // the profile. + CHECK_IS_TEST(); + parent_web_contents_.reset(); + } } ExtensionInstallPromptShowParams::ExtensionInstallPromptShowParams( @@ -40,8 +59,10 @@ ExtensionInstallPromptShowParams::ExtensionInstallPromptShowParams( : profile_(profile), parent_web_contents_(nullptr), parent_window_(parent_window) { - if (parent_window_) + DCHECK(profile); + if (parent_window_) { native_window_tracker_ = views::NativeWindowTracker::Create(parent_window_); + } } ExtensionInstallPromptShowParams::~ExtensionInstallPromptShowParams() = default; @@ -51,16 +72,57 @@ content::WebContents* ExtensionInstallPromptShowParams::GetParentWebContents() { } gfx::NativeWindow ExtensionInstallPromptShowParams::GetParentWindow() { - return (native_window_tracker_ && - !native_window_tracker_->WasNativeWindowDestroyed()) - ? parent_window_ - : nullptr; + if (WasParentDestroyed()) { + return nullptr; + } + + if (WasConfiguredForWebContents()) { + return parent_web_contents_->GetTopLevelNativeWindow(); + } + + return parent_window_; } bool ExtensionInstallPromptShowParams::WasParentDestroyed() { - const bool parent_web_contents_destroyed = - parent_web_contents_.WasInvalidated(); - return parent_web_contents_destroyed || - (native_window_tracker_ && - native_window_tracker_->WasNativeWindowDestroyed()); + if (profile_->ShutdownStarted()) { + return true; + } + + if (WasConfiguredForWebContents()) { + return !parent_web_contents_ || parent_web_contents_->IsBeingDestroyed() || + !parent_web_contents_->GetTopLevelNativeWindow() || + !RootCheck(parent_web_contents_->GetTopLevelNativeWindow()); + } + + if (native_window_tracker_) { + return native_window_tracker_->WasNativeWindowDestroyed() || + !RootCheck(parent_window_); + } + + return false; +} + +bool ExtensionInstallPromptShowParams::WasConfiguredForWebContents() { + // If we ever had a valid web contents, it means we were configured for it. + return parent_web_contents_ || parent_web_contents_.WasInvalidated(); } + +namespace test { + +ScopedDisableRootChecking::ScopedDisableRootChecking() { +#if defined(USE_AURA) + // There should be no need to support multiple ScopedDisableRootCheckings + // at a time. + DCHECK(g_root_checking_enabled); + g_root_checking_enabled = false; +#endif +} + +ScopedDisableRootChecking::~ScopedDisableRootChecking() { +#if defined(USE_AURA) + DCHECK(!g_root_checking_enabled); + g_root_checking_enabled = true; +#endif +} + +} // namespace test diff --git a/browser/extensions/extension_install_prompt_show_params.h b/browser/extensions/extension_install_prompt_show_params.h index 5909d5d63d7c96b85120638e951aed655e10e461..16b25d2540d00c2ea6cd3cf00fb8cfcd252b810d 100644 --- a/browser/extensions/extension_install_prompt_show_params.h +++ b/browser/extensions/extension_install_prompt_show_params.h @@ -25,6 +25,10 @@ class NativeWindowTracker; // - The dialog's parent window // - The browser window to use to open a new tab if a user clicks a link in the // dialog. +// +// This can either be created with a content::WebContents or a +// gfx::NativeWindow. If this is created for WebContents, GetParentWindow() will +// return the outermost window hosting the WebContents. class ExtensionInstallPromptShowParams { public: explicit ExtensionInstallPromptShowParams(content::WebContents* web_contents); @@ -57,12 +61,30 @@ class ExtensionInstallPromptShowParams { bool WasParentDestroyed(); private: + // Returns trues if the current object was configured for WebContents. + bool WasConfiguredForWebContents(); + raw_ptr profile_; + // Only one of these will be non-null. base::WeakPtr parent_web_contents_; - gfx::NativeWindow parent_window_; + + // Used to track the parent_window_'s lifetime. We need to explicitly track it + // because aura::Window does not expose a WeakPtr like WebContents. std::unique_ptr native_window_tracker_; }; +namespace test { + +// Unit test may use this to disable root window checking in +// ExtensionInstallPromptShowParams. +class ScopedDisableRootChecking { + public: + ScopedDisableRootChecking(); + ~ScopedDisableRootChecking(); +}; + +} // namespace test + #endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_PROMPT_SHOW_PARAMS_H_ diff --git a/browser/extensions/extension_install_prompt_show_params_unittest.cc b/browser/extensions/extension_install_prompt_show_params_unittest.cc new file mode 100644 index 0000000000000000000000000000000000000000..5d1b1e0a64ff472b2897ee38af9e68af68e8a74b --- /dev/null +++ b/browser/extensions/extension_install_prompt_show_params_unittest.cc @@ -0,0 +1,25 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/extensions/extension_install_prompt_show_params.h" + +#include "chrome/test/base/browser_with_test_window_test.h" +#include "ui/aura/test/test_windows.h" + +using ExtensionInstallPromptShowParamsTest = BrowserWithTestWindowTest; + +TEST_F(ExtensionInstallPromptShowParamsTest, WasParentDestroyedOutsideOfRoot) { + ExtensionInstallPromptShowParams params(profile(), GetContext()); + ASSERT_TRUE(GetContext()->GetRootWindow()); + // As the context window is parented to a root, the parent is valid. + EXPECT_FALSE(params.WasParentDestroyed()); + + std::unique_ptr window_with_no_root_ancestor( + aura::test::CreateTestWindowWithId(11, nullptr)); + ExtensionInstallPromptShowParams params2(profile(), + window_with_no_root_ancestor.get()); + // As `window_with_no_root_ancestor` is not parented to a root, it should + // return true from WasParentDestroyed(). + EXPECT_TRUE(params2.WasParentDestroyed()); +} diff --git a/browser/extensions/extension_install_prompt_unittest.cc b/browser/extensions/extension_install_prompt_unittest.cc index 051bb1647e7743b0b4304de236698f618f81cace..a2da8fa28ea4ea62d479a0ed31dae027f2f305f2 100644 --- a/browser/extensions/extension_install_prompt_unittest.cc +++ b/browser/extensions/extension_install_prompt_unittest.cc @@ -70,6 +70,10 @@ class ExtensionInstallPromptUnitTest : public testing::Test { Profile* profile() { return profile_.get(); } private: + // This test does not create a root window. Because of this, + // ScopedDisableRootChecking needs to be used (which disables the root window + // check). + test::ScopedDisableRootChecking disable_root_checking_; content::BrowserTaskEnvironment task_environment_; std::unique_ptr profile_; }; @@ -148,6 +152,10 @@ TEST_F(ExtensionInstallPromptUnitTest, using ExtensionInstallPromptTestWithService = ExtensionServiceTestWithInstall; TEST_F(ExtensionInstallPromptTestWithService, ExtensionInstallPromptIconsTest) { + // This test does not create a root window. Because of this, + // ScopedDisableRootChecking needs to be used (which disables the root window + // check). + test::ScopedDisableRootChecking disable_root_checking; InitializeEmptyExtensionService(); const Extension* extension = PackAndInstallCRX( diff --git a/browser/extensions/extension_system_impl.cc b/browser/extensions/extension_system_impl.cc index 1d98ff3f65dabd4da558e549772469da335d6ca6..82e2e84d3d8bbe5bc2a2f56b472c82deb6a8b02f 100644 --- a/browser/extensions/extension_system_impl.cc +++ b/browser/extensions/extension_system_impl.cc @@ -469,4 +469,11 @@ bool ExtensionSystemImpl::FinishDelayedInstallationIfReady( install_immediately); } +#if defined(OHOS_ARKWEB_EXTENSIONS) +void ExtensionSystemImpl::NotifyExtensionLoadedFromInternal( + const Extension* extension) {} +void ExtensionSystemImpl::NotifyExtensionUnLoadedFromInternal( + const Extension* extension) {} +#endif + } // namespace extensions diff --git a/browser/extensions/extension_system_impl.h b/browser/extensions/extension_system_impl.h index 769da163d00814fc7b5d6e964415d4b13a8cf700..bd4b665db9c59e0f017e1326926a2fd3183619a2 100644 --- a/browser/extensions/extension_system_impl.h +++ b/browser/extensions/extension_system_impl.h @@ -83,6 +83,12 @@ class ExtensionSystemImpl : public ExtensionSystem { const base::Value& attributes) override; bool FinishDelayedInstallationIfReady(const std::string& extension_id, bool install_immediately) override; +#if defined(OHOS_ARKWEB_EXTENSIONS) + void NotifyExtensionLoadedFromInternal( + const Extension* extension) override; + void NotifyExtensionUnLoadedFromInternal( + const Extension* extension) override; +#endif private: friend class ExtensionSystemSharedFactory; diff --git a/browser/extensions/updater/chrome_update_client_config.cc b/browser/extensions/updater/chrome_update_client_config.cc index db4da3100099daff023cfc25f4f03c0b908d7ee6..8def77aa6bc4144419826df6a2a4012bd31e114f 100644 --- a/browser/extensions/updater/chrome_update_client_config.cc +++ b/browser/extensions/updater/chrome_update_client_config.cc @@ -30,6 +30,9 @@ #include "components/prefs/pref_service.h" #include "components/services/patch/content/patch_service.h" #include "components/services/unzip/content/unzip_service.h" +#if defined(OHOS_ARKWEB_EXTENSIONS) +#include "components/services/unzip/in_process_unzipper.h" +#endif #include "components/update_client/activity_data_service.h" #include "components/update_client/buildflags.h" #include "components/update_client/crx_downloader_factory.h" @@ -241,7 +244,11 @@ ChromeUpdateClientConfig::GetUnzipperFactory() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); if (!unzip_factory_) { unzip_factory_ = base::MakeRefCounted( +#if defined(OHOS_ARKWEB_EXTENSIONS) + base::BindRepeating(&unzip::LaunchInProcessUnzipper)); +#else base::BindRepeating(&unzip::LaunchUnzipper)); +#endif } return unzip_factory_; } diff --git a/browser/extensions/updater/extension_updater.cc b/browser/extensions/updater/extension_updater.cc index fefe5d686e4fd22e8ddbfd7fcd551bbe3db702ac..d314ea0f94e3fd8a1bc02f064d1b1f2cd7a5eec3 100644 --- a/browser/extensions/updater/extension_updater.cc +++ b/browser/extensions/updater/extension_updater.cc @@ -329,7 +329,7 @@ void ExtensionUpdater::AddToDownloader( const Extension& extension = **extension_iter; const ExtensionId& extension_id = extension.id(); if (!Manifest::IsAutoUpdateableLocation(extension.location())) { - VLOG(2) << "Extension " << extension_id << " is not auto updateable"; + LOG(INFO) << "Extension " << extension_id << " is not auto updateable"; continue; } // An extension might be overwritten by policy, and have its update url @@ -389,7 +389,7 @@ void ExtensionUpdater::CheckNow(CheckParams params) { int request_id = next_request_id_++; - VLOG(2) << "Starting update check " << request_id; + LOG(INFO) << "Starting update check " << request_id; if (params.ids.empty()) NotifyStarted(); @@ -461,7 +461,7 @@ void ExtensionUpdater::CheckNow(CheckParams params) { << " is not a corrupt reinstall"; update_check_params.update_info[pending_id] = ExtensionUpdateData(); } else if (!Manifest::IsAutoUpdateableLocation(info->install_source())) { - VLOG(2) << "Extension " << pending_id << " is not auto updateable"; + LOG(INFO) << "Extension " << pending_id << " is not auto updateable"; continue; } // We have to mark high-priority extensions (such as policy-forced @@ -592,10 +592,14 @@ void ExtensionUpdater::OnExtensionDownloadFailed( switch (error) { case Error::CRX_FETCH_FAILED: + LOG(WARNING) << "Extension download failed as CRX_FETCH_FAILED, id:" + << id; install_stage_tracker->ReportFetchError( id, InstallStageTracker::FailureReason::CRX_FETCH_FAILED, data); break; case Error::CRX_FETCH_URL_EMPTY: + LOG(WARNING) << "Extension download failed as CRX_FETCH_URL_EMPTY, id:" + << id; DCHECK(data.additional_info); install_stage_tracker->ReportInfoOnNoUpdatesFailure( id, data.additional_info.value()); @@ -603,22 +607,32 @@ void ExtensionUpdater::OnExtensionDownloadFailed( id, InstallStageTracker::FailureReason::CRX_FETCH_URL_EMPTY); break; case Error::CRX_FETCH_URL_INVALID: + LOG(WARNING) << "Extension download failed as CRX_FETCH_URL_INVALID, id:" + << id; install_stage_tracker->ReportFailure( id, InstallStageTracker::FailureReason::CRX_FETCH_URL_INVALID); break; case Error::MANIFEST_FETCH_FAILED: + LOG(WARNING) << "Extension download failed as MANIFEST_FETCH_FAILED, id:" + << id; install_stage_tracker->ReportFetchError( id, InstallStageTracker::FailureReason::MANIFEST_FETCH_FAILED, data); break; case Error::MANIFEST_INVALID: + LOG(WARNING) << "Extension download failed as MANIFEST_INVALID, id:" + << id; DCHECK(data.manifest_invalid_error); install_stage_tracker->ReportManifestInvalidFailure(id, data); break; case Error::NO_UPDATE_AVAILABLE: + LOG(WARNING) << "Extension download failed as NO_UPDATE_AVAILABLE, id:" + << id; install_stage_tracker->ReportFailure( id, InstallStageTracker::FailureReason::NO_UPDATE); break; case Error::DISABLED: + LOG(WARNING) << "Extension download failed as DISABLED, id:" + << id; // Error::DISABLED corresponds to the browser having disabled extension // updates, the extension updater does not actually run when this error // code is emitted. @@ -659,7 +673,7 @@ void ExtensionUpdater::OnExtensionDownloadFinished( file.extension_id, InstallStageTracker::Stage::INSTALLING); UpdatePingData(file.extension_id, ping); - VLOG(2) << download_url << " written to " << file.path.value(); + LOG(INFO) << download_url << " written to " << file.path.value(); FetchedCRXFile fetched(file, file_ownership_passed, request_ids, std::move(callback)); @@ -768,7 +782,7 @@ bool ExtensionUpdater::CanUseUpdateService( void ExtensionUpdater::InstallCRXFile(FetchedCRXFile crx_file) { std::set request_ids; - VLOG(2) << "updating " << crx_file.info.extension_id << " with " + LOG(INFO) << "updating " << crx_file.info.extension_id << " with " << crx_file.info.path.value(); // The ExtensionService is now responsible for cleaning up the temp file @@ -877,7 +891,7 @@ void ExtensionUpdater::NotifyIfFinished(int request_id) { InProgressCheck& request = requests_in_progress_[request_id]; if (!request.in_progress_ids.empty() || request.awaiting_update_service) return; // This request is not done yet. - VLOG(2) << "Finished update check " << request_id; + LOG(INFO) << "Finished update check " << request_id; if (!request.callback.is_null()) std::move(request.callback).Run(); requests_in_progress_.erase(request_id); diff --git a/browser/external_protocol/external_protocol_handler.cc b/browser/external_protocol/external_protocol_handler.cc index e0acf5eccef5aef764ca21245fd8065a552445c6..fcd2fd63025b56e7cc1417c85c7f24de64a160bd 100644 --- a/browser/external_protocol/external_protocol_handler.cc +++ b/browser/external_protocol/external_protocol_handler.cc @@ -10,6 +10,7 @@ #include "base/check_op.h" #include "base/functional/bind.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/notreached.h" #include "base/strings/escape.h" @@ -86,10 +87,6 @@ constexpr const char* kDeniedSchemes[] = { "vnd.ms.radio", }; -constexpr const char* kAllowedSchemes[] = { - "mailto", "news", "snews", -}; - void AddMessageToConsole(const content::WeakDocumentPtr& document, blink::mojom::ConsoleMessageLevel level, const std::string& message) { @@ -304,6 +301,8 @@ bool IsSchemeOriginPairAllowedByPolicy(const std::string& scheme, } // namespace +const char ExternalProtocolHandler::kBlockStateMetric[] = + "BrowserDialogs.ExternalProtocol.BlockState"; const char ExternalProtocolHandler::kHandleStateMetric[] = "BrowserDialogs.ExternalProtocol.HandleState"; @@ -337,21 +336,28 @@ ExternalProtocolHandler::BlockState ExternalProtocolHandler::GetBlockState( } // Always block the hard-coded denied schemes. - for (size_t i = 0; i < std::size(kDeniedSchemes); ++i) { - if (kDeniedSchemes[i] == scheme) + for (const auto* candidate : kDeniedSchemes) { + if (candidate == scheme) { + base::UmaHistogramEnumeration(kBlockStateMetric, + BlockStateMetric::kDeniedDefault); return BLOCK; + } } - // Always allow the hard-coded allowed schemes. - for (size_t i = 0; i < std::size(kAllowedSchemes); ++i) { - if (kAllowedSchemes[i] == scheme) - return DONT_BLOCK; + // The mailto scheme is allowed explicitly because of its ubiquity on the web + // and because every platform provides a default handler for it. + if (scheme == "mailto") { + base::UmaHistogramEnumeration(kBlockStateMetric, + BlockStateMetric::kAllowedDefaultMail); + return DONT_BLOCK; } PrefService* profile_prefs = profile->GetPrefs(); if (profile_prefs) { // May be NULL during testing. if (IsSchemeOriginPairAllowedByPolicy(scheme, initiating_origin, profile_prefs)) { + base::UmaHistogramEnumeration( + kBlockStateMetric, BlockStateMetric::kAllowedByEnterprisePolicy); return DONT_BLOCK; } @@ -366,12 +372,16 @@ ExternalProtocolHandler::BlockState ExternalProtocolHandler::GetBlockState( if (allowed_protocols_for_origin) { absl::optional allow = allowed_protocols_for_origin->FindBool(scheme); - if (allow.has_value() && allow.value()) + if (allow.has_value() && allow.value()) { + base::UmaHistogramEnumeration(kBlockStateMetric, + BlockStateMetric::kAllowedByPreference); return DONT_BLOCK; + } } } } + base::UmaHistogramEnumeration(kBlockStateMetric, BlockStateMetric::kPrompt); return UNKNOWN; } diff --git a/browser/external_protocol/external_protocol_handler.h b/browser/external_protocol/external_protocol_handler.h index a05021f37b54dbab8af88eda08c933a004c1ae16..4df6233eb6bc9c7bb98a47263eef106ed1400d14 100644 --- a/browser/external_protocol/external_protocol_handler.h +++ b/browser/external_protocol/external_protocol_handler.h @@ -35,6 +35,23 @@ class ExternalProtocolHandler { UNKNOWN, }; + // This is used to back a UMA histogram, so it should be treated as + // append-only. + // This metric is related to BlockState but is only used for metrics + // reporting, and it differentiates multiple possible states that map to + // BlockState. + enum class BlockStateMetric { + kDeniedDefault, + kAllowedDefaultMail, + kAllowedDefaultNews_Deprecated, // No longer emitted. + kNewsNotDefault_Deprecated, // No longer emitted. + kAllowedByEnterprisePolicy, + kAllowedByPreference, + kPrompt, + // Insert new metric values above this line and update kMaxValue. + kMaxValue = kPrompt, + }; + // This is used to back a UMA histogram, so it should be treated as // append-only. Any new values should be inserted immediately prior to // HANDLE_STATE_LAST. @@ -73,6 +90,7 @@ class ExternalProtocolHandler { }; // UMA histogram metric names. + static const char kBlockStateMetric[]; static const char kHandleStateMetric[]; ExternalProtocolHandler(const ExternalProtocolHandler&) = delete; diff --git a/browser/external_protocol/external_protocol_handler_unittest.cc b/browser/external_protocol/external_protocol_handler_unittest.cc index 8a43ae9651582d767246247c1b60530697d4729d..e8a1ef09c9865bf69cc12580a3da857091ba9ef1 100644 --- a/browser/external_protocol/external_protocol_handler_unittest.cc +++ b/browser/external_protocol/external_protocol_handler_unittest.cc @@ -8,6 +8,7 @@ #include #include "base/run_loop.h" +#include "base/test/metrics/histogram_tester.h" #include "base/values.h" #include "build/build_config.h" #include "chrome/browser/prefs/browser_prefs.h" @@ -405,15 +406,29 @@ TEST_F(ExternalProtocolHandlerTest, TestUrlEscapeNoChecks) { } TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateUnknown) { + base::HistogramTester histogram_tester; + ExternalProtocolHandler::BlockState block_state = ExternalProtocolHandler::GetBlockState("tel", nullptr, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); + block_state = + ExternalProtocolHandler::GetBlockState("news", nullptr, profile_.get()); + EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); + block_state = + ExternalProtocolHandler::GetBlockState("snews", nullptr, profile_.get()); + EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); + EXPECT_TRUE(profile_->GetPrefs() ->GetDict(prefs::kProtocolHandlerPerOriginAllowedProtocols) .empty()); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kPrompt, 3); } TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateDefaultBlock) { + base::HistogramTester histogram_tester; + ExternalProtocolHandler::BlockState block_state = ExternalProtocolHandler::GetBlockState("afp", nullptr, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state); @@ -427,21 +442,33 @@ TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateDefaultBlock) { block_state = ExternalProtocolHandler::GetBlockState("mk", nullptr, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::BLOCK, block_state); + EXPECT_TRUE(profile_->GetPrefs() ->GetDict(prefs::kProtocolHandlerPerOriginAllowedProtocols) .empty()); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kDeniedDefault, 4); } TEST_F(ExternalProtocolHandlerTest, TestGetBlockStateDefaultDontBlock) { + base::HistogramTester histogram_tester; + ExternalProtocolHandler::BlockState block_state = ExternalProtocolHandler::GetBlockState("mailto", nullptr, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state); + EXPECT_TRUE(profile_->GetPrefs() ->GetDict(prefs::kProtocolHandlerPerOriginAllowedProtocols) .empty()); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kAllowedDefaultMail, 1); } TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) { + base::HistogramTester histogram_tester; + const char kScheme_1[] = "custom1"; const char kScheme_2[] = "custom2"; url::Origin example_origin_1 = @@ -464,6 +491,9 @@ TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) { EXPECT_TRUE(profile_->GetPrefs() ->GetDict(prefs::kProtocolHandlerPerOriginAllowedProtocols) .empty()); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kPrompt, 4); // Set to DONT_BLOCK for {kScheme_1, example_origin_1}, and make sure it is // written to prefs. @@ -482,6 +512,12 @@ TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) { block_state = ExternalProtocolHandler::GetBlockState( kScheme_2, &example_origin_2, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::UNKNOWN, block_state); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kAllowedByPreference, 1); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kPrompt, 7); // Set to DONT_BLOCK for {kScheme_2, example_origin_2}, and make sure it is // written to prefs independently of {kScheme_1, example_origin_1}. @@ -500,6 +536,12 @@ TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) { block_state = ExternalProtocolHandler::GetBlockState( kScheme_2, &example_origin_2, profile_.get()); EXPECT_EQ(ExternalProtocolHandler::DONT_BLOCK, block_state); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kAllowedByPreference, 3); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kPrompt, 9); const base::Value::Dict& protocol_origin_pairs = profile_->GetPrefs()->GetDict( @@ -536,6 +578,12 @@ TEST_F(ExternalProtocolHandlerTest, TestSetBlockState) { EXPECT_TRUE(profile_->GetPrefs() ->GetDict(prefs::kProtocolHandlerPerOriginAllowedProtocols) .empty()); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kAllowedByPreference, 3); + histogram_tester.ExpectBucketCount( + ExternalProtocolHandler::kBlockStateMetric, + ExternalProtocolHandler::BlockStateMetric::kPrompt, 11); } TEST_F(ExternalProtocolHandlerTest, TestSetBlockStateWithUntrustowrthyOrigin) { diff --git a/browser/file_select_helper.cc b/browser/file_select_helper.cc index 93f96e1aa4542285c80ae66374c4de64cb1ee8cf..34d8cf59a51066f821388ee865f8e5bea36d0fa5 100644 --- a/browser/file_select_helper.cc +++ b/browser/file_select_helper.cc @@ -297,8 +297,8 @@ void FileSelectHelper::OnListDone(int error) { } else { std::vector chooser_files; for (const auto& file_path : entry->results_) { -#ifdef OHOS_FILE_UPLOAD std::u16string display_name = std::u16string(); +#ifdef OHOS_FILE_UPLOAD if (file_path.IsDataShareUri()) { display_name = base::GetFileDisplayName(file_path); } @@ -348,8 +348,8 @@ void FileSelectHelper::ConvertToFileChooserFileInfoList( display_name = base::GetFileDisplayName(file.local_path); } chooser_files.push_back( - FileChooserFileInfo::NewNativeFile(blink::mojom::NativeFileInfo::New( - file.local_path, display_name))); + FileChooserFileInfo::NewNativeFile(blink::mojom::NativeFileInfo::New( + file.local_path, display_name))); #else chooser_files.push_back( FileChooserFileInfo::NewNativeFile(blink::mojom::NativeFileInfo::New( diff --git a/browser/lens/region_search/lens_region_search_controller.cc b/browser/lens/region_search/lens_region_search_controller.cc index 968fbbdef4af472a730957ebed521c37614747f2..62ea01ac124bf6f6f913b055263fed65fe30d900 100644 --- a/browser/lens/region_search/lens_region_search_controller.cc +++ b/browser/lens/region_search/lens_region_search_controller.cc @@ -50,7 +50,7 @@ void LensRegionSearchController::Start(content::WebContents* web_contents, if (!web_contents || in_capture_mode_) { return; } - Browser* browser = chrome::FindBrowserWithTab(web_contents); + Browser* browser = chrome::FindBrowserWithWebContents(web_contents); if (!browser) { return; } diff --git a/browser/metrics/chrome_metrics_extensions_helper_unittest.cc b/browser/metrics/chrome_metrics_extensions_helper_unittest.cc index 8c72ada1a7b98a07c66bb834c546ac807ac42d00..8bc875f970a62249f0590a41bd88c41bba1884ed 100644 --- a/browser/metrics/chrome_metrics_extensions_helper_unittest.cc +++ b/browser/metrics/chrome_metrics_extensions_helper_unittest.cc @@ -39,8 +39,7 @@ TEST(ChromeMetricsExtensionsHelperTest, Basic) { #if BUILDFLAG(ENABLE_EXTENSIONS) // Tag |host| so that it's an extensions host. - extensions::ProcessMap::Get(profile)->Insert("1", host->GetID(), - site_instance->GetId()); + extensions::ProcessMap::Get(profile)->Insert("1", host->GetID()); EXPECT_TRUE(extensions_helper.IsExtensionProcess(host)); #endif rph_factory.reset(); diff --git a/browser/metrics/tab_stats/tab_stats_data_store.cc b/browser/metrics/tab_stats/tab_stats_data_store.cc index 03463e113dccc6b0a7e6471460e2acf6da9a02c1..7f1e8268fd520ea6c58a164849f3adc612e02877 100644 --- a/browser/metrics/tab_stats/tab_stats_data_store.cc +++ b/browser/metrics/tab_stats/tab_stats_data_store.cc @@ -4,31 +4,18 @@ #include "chrome/browser/metrics/tab_stats/tab_stats_data_store.h" -#include #include #include "base/containers/contains.h" -#include "base/notreached.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_list.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" -#include "content/public/browser/visibility.h" #include "content/public/browser/web_contents.h" namespace metrics { -namespace { - -// Computes a new, unique, TabID. -TabStatsDataStore::TabID GetNewTabId() { - static TabStatsDataStore::TabID web_contents_id = 0U; - return ++web_contents_id; -} - -} // namespace - TabStatsDataStore::TabsStats::TabsStats() : total_tab_count(0U), total_tab_count_max(0U), @@ -90,73 +77,23 @@ void TabStatsDataStore::OnWindowRemoved() { void TabStatsDataStore::OnTabAdded(content::WebContents* web_contents) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(web_contents); - DCHECK(!base::Contains(existing_tabs_, web_contents)); ++tab_stats_.total_tab_count; RecordSamplingMetaData(); - TabID tab_id = GetNewTabId(); - existing_tabs_.insert(std::make_pair(web_contents, tab_id)); - for (auto& interval_map : interval_maps_) { - AddTabToIntervalMap(web_contents, tab_id, - /* existed_before_interval */ false, - interval_map.get()); - } UpdateTotalTabCountMaxIfNeeded(); } void TabStatsDataStore::OnTabRemoved(content::WebContents* web_contents) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(web_contents); - DCHECK(base::Contains(existing_tabs_, web_contents)); DCHECK_GT(tab_stats_.total_tab_count, 0U); --tab_stats_.total_tab_count; RecordSamplingMetaData(); - TabID web_contents_id = GetTabID(web_contents); - existing_tabs_.erase(web_contents); - for (auto& interval_map : interval_maps_) { - auto iter = interval_map->find(web_contents_id); - DCHECK(iter != interval_map->end()); - iter->second.exists_currently = false; - } -} - -void TabStatsDataStore::OnTabReplaced(content::WebContents* old_contents, - content::WebContents* new_contents) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(old_contents); - DCHECK(new_contents); - DCHECK(base::Contains(existing_tabs_, old_contents)); - DCHECK_GT(tab_stats_.total_tab_count, 0U); - TabID old_contents_id = existing_tabs_[old_contents]; - existing_tabs_.erase(old_contents); - existing_tabs_[new_contents] = old_contents_id; -} - -void TabStatsDataStore::OnTabInteraction(content::WebContents* web_contents) { - DCHECK(base::Contains(existing_tabs_, web_contents)); - TabID web_contents_id = GetTabID(web_contents); - // Mark the tab as interacted with in all the intervals. - for (auto& interval_map : interval_maps_) { - DCHECK(base::Contains(*interval_map, web_contents_id)); - (*interval_map)[web_contents_id].interacted_during_interval = true; - } -} - -void TabStatsDataStore::OnTabIsAudibleChanged( - content::WebContents* web_contents) { - if (web_contents->IsCurrentlyAudible()) - OnTabAudibleOrVisible(web_contents); } void TabStatsDataStore::RecordSamplingMetaData() { tab_number_sample_meta_data_.Set(tab_stats_.total_tab_count); } -void TabStatsDataStore::OnTabVisibilityChanged( - content::WebContents* web_contents) { - if (web_contents->GetVisibility() == content::Visibility::VISIBLE) - OnTabAudibleOrVisible(web_contents); -} - void TabStatsDataStore::UpdateMaxTabsPerWindowIfNeeded(size_t value) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (value <= tab_stats_.max_tab_per_window) @@ -184,25 +121,6 @@ void TabStatsDataStore::ResetMaximumsToCurrentState() { } } -TabStatsDataStore::TabsStateDuringIntervalMap* -TabStatsDataStore::AddInterval() { - // Creates the interval and initialize its data. - std::unique_ptr interval_map = - std::make_unique(); - ResetIntervalData(interval_map.get()); - interval_maps_.emplace_back(std::move(interval_map)); - return interval_maps_.back().get(); -} - -void TabStatsDataStore::ResetIntervalData( - TabsStateDuringIntervalMap* interval_map) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(interval_map); - interval_map->clear(); - for (auto& iter : existing_tabs_) - AddTabToIntervalMap(iter.first, GetTabID(iter.first), true, interval_map); -} - void TabStatsDataStore::OnTabDiscardStateChange( LifecycleUnitDiscardReason discard_reason, bool is_discarded) { @@ -245,13 +163,6 @@ void TabStatsDataStore::ClearTabDiscardAndReloadCounts() { pref_service_->SetInteger(::prefs::kTabStatsReloadsProactive, 0); } -absl::optional TabStatsDataStore::GetTabIDForTesting( - content::WebContents* web_contents) { - if (!base::Contains(existing_tabs_, web_contents)) - return absl::nullopt; - return GetTabID(web_contents); -} - void TabStatsDataStore::UpdateTotalTabCountMaxIfNeeded() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (tab_stats_.total_tab_count <= tab_stats_.total_tab_count_max) @@ -270,40 +181,4 @@ void TabStatsDataStore::UpdateWindowCountMaxIfNeeded() { tab_stats_.window_count_max); } -void TabStatsDataStore::AddTabToIntervalMap( - content::WebContents* web_contents, - TabID tab_id, - bool existed_before_interval, - TabsStateDuringIntervalMap* interval_map) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(interval_map); - DCHECK(web_contents); - bool visible_or_audible = - web_contents->GetVisibility() == content::Visibility::VISIBLE || - web_contents->IsCurrentlyAudible(); - - auto& tab_state = (*interval_map)[tab_id]; - tab_state.existed_before_interval = existed_before_interval; - tab_state.exists_currently = true; - tab_state.visible_or_audible_during_interval = visible_or_audible; - tab_state.interacted_during_interval = false; -} - -TabStatsDataStore::TabID TabStatsDataStore::GetTabID( - content::WebContents* web_contents) { - DCHECK(base::Contains(existing_tabs_, web_contents)); - return existing_tabs_[web_contents]; -} - -void TabStatsDataStore::OnTabAudibleOrVisible( - content::WebContents* web_contents) { - DCHECK(base::Contains(existing_tabs_, web_contents)); - TabID web_contents_id = GetTabID(web_contents); - // Mark the tab as visible or audible in all the intervals. - for (auto& interval_map : interval_maps_) { - DCHECK(base::Contains(*interval_map, web_contents_id)); - (*interval_map)[web_contents_id].visible_or_audible_during_interval = true; - } -} - } // namespace metrics diff --git a/browser/metrics/tab_stats/tab_stats_data_store.h b/browser/metrics/tab_stats/tab_stats_data_store.h index da4afd8ea8b289dd843a286e0dd70f3914e76de8..1a3e6d806653eef56f37a01ad00d409ec0fde81b 100644 --- a/browser/metrics/tab_stats/tab_stats_data_store.h +++ b/browser/metrics/tab_stats/tab_stats_data_store.h @@ -71,30 +71,6 @@ class TabStatsDataStore : public TabStatsObserver { tab_reload_counts; }; - // Structure describing the state of a tab during an interval of time. - struct TabStateDuringInterval { - // Indicates if the tab exists at the beginning of the interval. - bool existed_before_interval; - // Indicates if the tab still exists. - bool exists_currently; - // Indicates if the tab has been visible or audible at any moment during the - // interval. - bool visible_or_audible_during_interval; - // Indicates if the tab has been interacted with or active at any moment - // during the interval. See the |OnTabInteraction| for the list of possible - // interactions. - bool interacted_during_interval; - }; - - // A TabID is used instead of a WebContents* because: - // - The WebContents of a tab can change during its lifetime. - // - A WebContents* can be reused for a separate tab after a tab has been - // closed. - using TabID = size_t; - // Represents the state of a set of tabs during an interval of time. - using TabsStateDuringIntervalMap = - base::flat_map; - explicit TabStatsDataStore(PrefService* pref_service); TabStatsDataStore(const TabStatsDataStore&) = delete; @@ -107,11 +83,6 @@ class TabStatsDataStore : public TabStatsObserver { void OnWindowRemoved() override; void OnTabAdded(content::WebContents* web_contents) override; void OnTabRemoved(content::WebContents* web_contents) override; - void OnTabReplaced(content::WebContents* old_contents, - content::WebContents* new_contents) override; - void OnTabInteraction(content::WebContents* web_contents) override; - void OnTabIsAudibleChanged(content::WebContents* web_contents) override; - void OnTabVisibilityChanged(content::WebContents* web_contents) override; // Update the maximum number of tabs in a single window if |value| exceeds // this. @@ -123,12 +94,6 @@ class TabStatsDataStore : public TabStatsObserver { // metrics have been reported. void ResetMaximumsToCurrentState(); - // Creates a new interval map. The returned pointer is owned by |this|. - TabsStateDuringIntervalMap* AddInterval(); - - // Reset |interval_map| with the list of current tabs. - void ResetIntervalData(TabsStateDuringIntervalMap* interval_map); - // Updates discard/reload counts when the discarded state of a tab changes. // Updates the discard count when is_discarded is true. Updates the reload // count when is_discarded is false. @@ -140,10 +105,6 @@ class TabStatsDataStore : public TabStatsObserver { void ClearTabDiscardAndReloadCounts(); const TabsStats& tab_stats() const { return tab_stats_; } - absl::optional GetTabIDForTesting(content::WebContents* web_contents); - base::flat_map* existing_tabs_for_testing() { - return &existing_tabs_; - } protected: FRIEND_TEST_ALL_PREFIXES(TabStatsTrackerBrowserTest, @@ -153,18 +114,7 @@ class TabStatsDataStore : public TabStatsObserver { void UpdateTotalTabCountMaxIfNeeded(); void UpdateWindowCountMaxIfNeeded(); - // Adds a tab to an interval map. - void AddTabToIntervalMap(content::WebContents* web_contents, - TabID tab_id, - bool existed_before_interval, - TabsStateDuringIntervalMap* interval_map); - - // Returns the TabID associated with a tab. - TabID GetTabID(content::WebContents* web_contents); - private: - void OnTabAudibleOrVisible(content::WebContents* web_contents); - // Record the stack sampling meta data with the current tab count; void RecordSamplingMetaData(); @@ -178,12 +128,6 @@ class TabStatsDataStore : public TabStatsObserver { // A raw pointer to the PrefService used to read and write the statistics. raw_ptr pref_service_; - // The interval maps, one per period of time that we want to observe. - std::vector> interval_maps_; - - // The tabs that currently exist. - base::flat_map existing_tabs_; - SEQUENCE_CHECKER(sequence_checker_); }; diff --git a/browser/metrics/tab_stats/tab_stats_data_store_unittest.cc b/browser/metrics/tab_stats/tab_stats_data_store_unittest.cc index 5701bb40d0162256485eb98f638c821b4ad195c6..b20caab38f00b233c4ffd074ed6308e84a19988b 100644 --- a/browser/metrics/tab_stats/tab_stats_data_store_unittest.cc +++ b/browser/metrics/tab_stats/tab_stats_data_store_unittest.cc @@ -129,146 +129,4 @@ TEST_F(TabStatsDataStoreTest, DiscardsFromLocalState) { EXPECT_EQ(kExpectedReloadsProactive, stats2.tab_reload_counts[proactive]); } -TEST_F(TabStatsDataStoreTest, TrackTabUsageDuringInterval) { - std::vector> web_contents_vec; - for (size_t i = 0; i < 3; ++i) { - web_contents_vec.emplace_back(CreateTestWebContents()); - // Make sure that these WebContents are initially not visible. - web_contents_vec.back()->WasHidden(); - } - - // Creates a test interval. - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map = - data_store_->AddInterval(); - EXPECT_TRUE(interval_map->empty()); - - std::vector web_contents_id_vec; - // Adds the tabs to the data store. - for (auto& iter : web_contents_vec) { - data_store_->OnTabAdded(iter.get()); - web_contents_id_vec.push_back( - data_store_->GetTabIDForTesting(iter.get()).value()); - } - - EXPECT_EQ(web_contents_vec.size(), interval_map->size()); - - for (const auto& iter : web_contents_id_vec) { - auto map_iter = interval_map->find(iter); - EXPECT_TRUE(map_iter != interval_map->end()); - - // The tabs have been inserted after the creation of the interval. - EXPECT_FALSE(map_iter->second.existed_before_interval); - EXPECT_FALSE(map_iter->second.visible_or_audible_during_interval); - EXPECT_FALSE(map_iter->second.interacted_during_interval); - EXPECT_TRUE(map_iter->second.exists_currently); - } - - // Interact with a tab. - data_store_->OnTabInteraction(web_contents_vec[1].get()); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[0]) - ->second.interacted_during_interval); - EXPECT_TRUE(interval_map->find(web_contents_id_vec[1]) - ->second.interacted_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[2]) - ->second.interacted_during_interval); - - data_store_->ResetIntervalData(interval_map); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[0]) - ->second.interacted_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[1]) - ->second.interacted_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[2]) - ->second.interacted_during_interval); - - // Make the first WebContents visible. - web_contents_vec[0].get()->WasShown(); - data_store_->OnTabVisibilityChanged(web_contents_vec[0].get()); - EXPECT_TRUE(interval_map->find(web_contents_id_vec[0]) - ->second.visible_or_audible_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[1]) - ->second.visible_or_audible_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[2]) - ->second.visible_or_audible_during_interval); - - // Make one of the WebContents audible. - content::WebContentsTester::For(web_contents_vec[2].get()) - ->SetIsCurrentlyAudible(true); - data_store_->ResetIntervalData(interval_map); - data_store_->OnTabIsAudibleChanged(web_contents_vec[2].get()); - EXPECT_TRUE(interval_map->find(web_contents_id_vec[0]) - ->second.visible_or_audible_during_interval); - EXPECT_FALSE(interval_map->find(web_contents_id_vec[1]) - ->second.visible_or_audible_during_interval); - EXPECT_TRUE(interval_map->find(web_contents_id_vec[2]) - ->second.visible_or_audible_during_interval); - - // Make sure that the tab stats get properly copied when a tab is replaced. - TabStatsDataStore::TabStateDuringInterval tab_stats_copy = - interval_map->find(web_contents_id_vec[1])->second; - std::unique_ptr new_contents = CreateTestWebContents(); - content::WebContents* old_contents = web_contents_vec[1].get(); - data_store_->OnTabReplaced(old_contents, new_contents.get()); - EXPECT_EQ(data_store_->GetTabIDForTesting(new_contents.get()).value(), - web_contents_id_vec[1]); - web_contents_vec[1] = std::move(new_contents); - // |old_contents| is invalid starting from here. - EXPECT_FALSE(data_store_->GetTabIDForTesting(old_contents)); - auto iter = interval_map->find(web_contents_id_vec[1]); - EXPECT_TRUE(iter != interval_map->end()); - EXPECT_EQ(tab_stats_copy.existed_before_interval, - iter->second.existed_before_interval); - EXPECT_EQ(tab_stats_copy.exists_currently, iter->second.exists_currently); - EXPECT_EQ(tab_stats_copy.visible_or_audible_during_interval, - iter->second.visible_or_audible_during_interval); - EXPECT_EQ(tab_stats_copy.interacted_during_interval, - iter->second.interacted_during_interval); -} - -TEST_F(TabStatsDataStoreTest, OnTabReplaced) { - // Creates a tab and make sure that it's not visible. - std::unique_ptr web_contents_1(CreateTestWebContents()); - web_contents_1->WasHidden(); - // Creates a test interval. - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map = - data_store_->AddInterval(); - - data_store_->OnTabAdded(web_contents_1.get()); - TabStatsDataStore::TabID tab_id = - data_store_->GetTabIDForTesting(web_contents_1.get()).value(); - - // Interact with the tab. - data_store_->OnTabInteraction(web_contents_1.get()); - EXPECT_TRUE(interval_map->find(tab_id)->second.interacted_during_interval); - EXPECT_FALSE( - interval_map->find(tab_id)->second.visible_or_audible_during_interval); - - std::unique_ptr web_contents_2(CreateTestWebContents()); - web_contents_2->WasHidden(); - - // Replace the tab, make sure that the |visible_or_audible_during_interval| - // bit is still set. - data_store_->OnTabReplaced(web_contents_1.get(), web_contents_2.get()); - EXPECT_FALSE(data_store_->GetTabIDForTesting(web_contents_1.get())); - EXPECT_EQ(tab_id, data_store_->GetTabIDForTesting(web_contents_2.get())); - EXPECT_EQ(1U, interval_map->size()); - EXPECT_TRUE(interval_map->find(tab_id)->second.interacted_during_interval); - - // Mark the tab as audible and verify that the corresponding bit is set. - content::WebContentsTester::For(web_contents_2.get()) - ->SetIsCurrentlyAudible(true); - data_store_->OnTabIsAudibleChanged(web_contents_2.get()); - EXPECT_TRUE( - interval_map->find(tab_id)->second.visible_or_audible_during_interval); - - // Close the tab and make sure that the |exists_currently| bit gets cleared. - data_store_->OnTabRemoved(web_contents_2.get()); - EXPECT_FALSE(interval_map->find(tab_id)->second.exists_currently); - - // Add a new tab with a WebContents pointer that has already been used in the - // past, make sure that this gets treated as a new tab. - data_store_->OnTabAdded(web_contents_1.get()); - EXPECT_EQ(2U, interval_map->size()); - EXPECT_NE(tab_id, data_store_->GetTabIDForTesting(web_contents_1.get())); -} - } // namespace metrics diff --git a/browser/metrics/tab_stats/tab_stats_tracker.cc b/browser/metrics/tab_stats/tab_stats_tracker.cc index d733739e97fbc523a6a00677b839c7efc5455430..6b52d9c5b7f718ea4809b74e15dae8977a544f8b 100644 --- a/browser/metrics/tab_stats/tab_stats_tracker.cc +++ b/browser/metrics/tab_stats/tab_stats_tracker.cc @@ -55,27 +55,12 @@ namespace { // called. constexpr base::TimeDelta kDailyEventIntervalTimeDelta = base::Minutes(30); -// The intervals at which we report the number of unused tabs. This is used for -// all the tab usage histograms listed below. -// -// The 'Tabs.TabUsageIntervalLength' histogram suffixes entry in histograms.xml -// should be kept in sync with these values. -constexpr base::TimeDelta kTabUsageReportingIntervals[] = { - base::Seconds(30), base::Minutes(1), base::Minutes(10), - base::Hours(1), base::Hours(5), base::Hours(12)}; - // The interval at which the heartbeat tab metrics should be reported. const base::TimeDelta kTabsHeartbeatReportingInterval = base::Minutes(5); // The global TabStatsTracker instance. TabStatsTracker* g_tab_stats_tracker_instance = nullptr; -// Ensure that an interval is a valid one (i.e. listed in -// |kTabUsageReportingIntervals|). -bool IsValidInterval(base::TimeDelta interval) { - return base::Contains(kTabUsageReportingIntervals, interval); -} - void UmaHistogramCounts10000WithBatteryStateVariant(const char* histogram_name, size_t value) { DCHECK(base::PowerMonitor::IsInitialized()); @@ -103,18 +88,6 @@ const char TabStatsTracker::UmaStatsReportingDelegate:: const char TabStatsTracker::UmaStatsReportingDelegate:: kMaxWindowsInADayHistogramName[] = "Tabs.MaxWindowsInADay"; -// Tab usage histograms. -const char TabStatsTracker::UmaStatsReportingDelegate:: - kUnusedAndClosedInIntervalHistogramNameBase[] = - "Tabs.UnusedAndClosedInInterval.Count"; -const char TabStatsTracker::UmaStatsReportingDelegate:: - kUnusedTabsInIntervalHistogramNameBase[] = "Tabs.UnusedInInterval.Count"; -const char TabStatsTracker::UmaStatsReportingDelegate:: - kUsedAndClosedInIntervalHistogramNameBase[] = - "Tabs.UsedAndClosedInInterval.Count"; -const char TabStatsTracker::UmaStatsReportingDelegate:: - kUsedTabsInIntervalHistogramNameBase[] = "Tabs.UsedInInterval.Count"; - const char TabStatsTracker::UmaStatsReportingDelegate::kTabCountHistogramName[] = "Tabs.TabCount"; @@ -183,22 +156,6 @@ TabStatsTracker::TabStatsTracker(PrefService* pref_service) daily_event_timer_.Start(FROM_HERE, kDailyEventIntervalTimeDelta, daily_event_.get(), &DailyEvent::CheckInterval); - // Initialize the interval maps and timers associated with them. - // Only |tab_stats_data_store_| (and not other observers) is registered for - // callbacks since only it computes intervals. - for (base::TimeDelta interval : kTabUsageReportingIntervals) { - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map = - tab_stats_data_store_->AddInterval(); - // Setup the timer associated with this interval. - std::unique_ptr timer = - std::make_unique(); - timer->Start( - FROM_HERE, interval, - base::BindRepeating(&TabStatsTracker::OnInterval, - base::Unretained(this), interval, interval_map)); - usage_interval_timers_.push_back(std::move(timer)); - } - heartbeat_timer_.Start(FROM_HERE, kTabsHeartbeatReportingInterval, base::BindRepeating(&TabStatsTracker::OnHeartbeatEvent, base::Unretained(this))); @@ -455,18 +412,6 @@ void TabStatsTracker::OnAutoDiscardableStateChange( content::WebContents* contents, bool is_auto_discardable) {} -void TabStatsTracker::OnInterval( - base::TimeDelta interval, - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map) { - DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(interval_map); - reporting_delegate_->ReportUsageDuringInterval(*interval_map, interval); - - // Only |tab_stats_data_store_| (and not other obsevers) resets since only it - // computes intervals. - tab_stats_data_store_->ResetIntervalData(interval_map); -} - void TabStatsTracker::OnInitialOrInsertedTab( content::WebContents* web_contents) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -583,67 +528,6 @@ void TabStatsTracker::UmaStatsReportingDelegate::ReportHeartbeatMetrics( } } -void TabStatsTracker::UmaStatsReportingDelegate::ReportUsageDuringInterval( - const TabStatsDataStore::TabsStateDuringIntervalMap& interval_map, - base::TimeDelta interval) { - // Counts the number of used/unused tabs during this interval, a tabs counts - // as unused if it hasn't been interacted with or visible during the duration - // of the interval. - size_t used_tabs = 0; - size_t used_and_closed_tabs = 0; - size_t unused_tabs = 0; - size_t unused_and_closed_tabs = 0; - for (const auto& iter : interval_map) { - // There's currently no distinction between a visible/audible tab and one - // that has been interacted with in these metrics. - // TODO(sebmarchand): Add a metric that track the number of tab that have - // been visible/audible but not interacted with during an interval, - // https://crbug.com/800828. - if (iter.second.interacted_during_interval || - iter.second.visible_or_audible_during_interval) { - if (iter.second.exists_currently) - ++used_tabs; - else - ++used_and_closed_tabs; - } else { - if (iter.second.exists_currently) - ++unused_tabs; - else - ++unused_and_closed_tabs; - } - } - - std::string used_and_closed_histogram_name = GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedAndClosedInIntervalHistogramNameBase, - interval); - std::string used_histogram_name = GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedTabsInIntervalHistogramNameBase, - interval); - std::string unused_and_closed_histogram_name = GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedAndClosedInIntervalHistogramNameBase, - interval); - std::string unused_histogram_name = GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedTabsInIntervalHistogramNameBase, - interval); - - base::UmaHistogramCounts10000(used_and_closed_histogram_name, - used_and_closed_tabs); - base::UmaHistogramCounts10000(used_histogram_name, used_tabs); - base::UmaHistogramCounts10000(unused_and_closed_histogram_name, - unused_and_closed_tabs); - base::UmaHistogramCounts10000(unused_histogram_name, unused_tabs); -} - -// static -std::string -TabStatsTracker::UmaStatsReportingDelegate::GetIntervalHistogramName( - const char* base_name, - base::TimeDelta interval) { - DCHECK(IsValidInterval(interval)); - return base::StringPrintf("%s_%zu", base_name, - static_cast(interval.InSeconds())); -} - bool TabStatsTracker::UmaStatsReportingDelegate:: IsChromeBackgroundedWithoutWindows() { #if BUILDFLAG(ENABLE_BACKGROUND_MODE) diff --git a/browser/metrics/tab_stats/tab_stats_tracker.h b/browser/metrics/tab_stats/tab_stats_tracker.h index f224c8f7e4b83ac05f5df9dc68129de7ade46e41..657084a6e7290fbeb86d853b7214212b379992ad 100644 --- a/browser/metrics/tab_stats/tab_stats_tracker.h +++ b/browser/metrics/tab_stats/tab_stats_tracker.h @@ -118,10 +118,6 @@ class TabStatsTracker : public TabStripModelObserver, UmaStatsReportingDelegate* reporting_delegate_for_testing() { return reporting_delegate_.get(); } - std::vector>* - usage_interval_timers_for_testing() { - return &usage_interval_timers_; - } base::RepeatingTimer* heartbeat_timer_for_testing() { return &heartbeat_timer_; } @@ -163,10 +159,6 @@ class TabStatsTracker : public TabStripModelObserver, void OnAutoDiscardableStateChange(content::WebContents* contents, bool is_auto_discardable) override; - // Callback when an interval timer triggers. - void OnInterval(base::TimeDelta interval, - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map); - // Functions to call to start tracking a new tab. void OnInitialOrInsertedTab(content::WebContents* web_contents); @@ -203,10 +195,6 @@ class TabStatsTracker : public TabStripModelObserver, // triggered. base::RepeatingTimer daily_event_timer_; - // The timers used to analyze how tabs are used during a given interval of - // time. - std::vector> usage_interval_timers_; - // The timer used to report the heartbeat metrics at regular interval. base::RepeatingTimer heartbeat_timer_; @@ -236,14 +224,6 @@ class TabStatsTracker::UmaStatsReportingDelegate { // opened in a day. static const char kMaxWindowsInADayHistogramName[]; - // The name of the histograms that records how tabs have been used during a - // given period of time. Will be appended with '_T' with T being the interval - // window (in seconds). - static const char kUnusedAndClosedInIntervalHistogramNameBase[]; - static const char kUnusedTabsInIntervalHistogramNameBase[]; - static const char kUsedAndClosedInIntervalHistogramNameBase[]; - static const char kUsedTabsInIntervalHistogramNameBase[]; - // The name of the histograms that records the current number of tabs/windows. static const char kTabCountHistogramName[]; static const char kWindowCountHistogramName[]; @@ -277,18 +257,7 @@ class TabStatsTracker::UmaStatsReportingDelegate { // Report the tab heartbeat metrics. void ReportHeartbeatMetrics(const TabStatsDataStore::TabsStats& tab_stats); - // Report some information about how tabs have been used during a given - // interval of time. - void ReportUsageDuringInterval( - const TabStatsDataStore::TabsStateDuringIntervalMap& interval_map, - base::TimeDelta interval); - protected: - // Generates the name of the histograms that will track tab usage during a - // given period of time. - static std::string GetIntervalHistogramName(const char* base_name, - base::TimeDelta interval); - // Checks if Chrome is running in background with no visible windows, virtual // for unittesting. virtual bool IsChromeBackgroundedWithoutWindows(); diff --git a/browser/metrics/tab_stats/tab_stats_tracker_browsertest.cc b/browser/metrics/tab_stats/tab_stats_tracker_browsertest.cc index e352cc5a5c571ecaa34a95c5f913abe76d3c6b95..16cd8871b3b83d7f53a084e47eeab9084106a764 100644 --- a/browser/metrics/tab_stats/tab_stats_tracker_browsertest.cc +++ b/browser/metrics/tab_stats/tab_stats_tracker_browsertest.cc @@ -213,49 +213,6 @@ IN_PROC_BROWSER_TEST_F(TabStatsTrackerBrowserTest, DCHECK_EQ(second_observer->window_count(), expected_stats.window_count); } -IN_PROC_BROWSER_TEST_F(TabStatsTrackerBrowserTest, - TabDeletionGetsHandledProperly) { - // Assert that the |TabStatsTracker| instance is initialized during the - // creation of the main browser. - ASSERT_TRUE(tab_stats_tracker_ != nullptr); - - constexpr base::TimeDelta kValidLongInterval = base::Hours(12); - - TabStatsDataStore* data_store = tab_stats_tracker_->tab_stats_data_store(); - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map = - data_store->AddInterval(); - - ASSERT_TRUE(AddTabAtIndex(1, GURL("about:blank"), ui::PAGE_TRANSITION_TYPED)); - - EXPECT_EQ(2U, interval_map->size()); - - content::WebContents* web_contents = - data_store->existing_tabs_for_testing()->begin()->first; - - // Delete one of the WebContents without calling the |OnTabRemoved| function, - // the WebContentsObserver owned by |tab_stats_tracker_| should be notified - // and this should be handled correctly. - TabStatsDataStore::TabID tab_id = - data_store->GetTabIDForTesting(web_contents).value(); - browser()->tab_strip_model()->DetachAndDeleteWebContentsAt( - browser()->tab_strip_model()->GetIndexOfWebContents(web_contents)); - EXPECT_TRUE(base::Contains(*interval_map, tab_id)); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - EXPECT_EQ(1U, interval_map->size()); - EXPECT_FALSE(base::Contains(*interval_map, tab_id)); - - web_contents = data_store->existing_tabs_for_testing()->begin()->first; - - // Do this a second time, ensures that the situation where there's no existing - // tabs is handled properly. - tab_id = data_store->GetTabIDForTesting(web_contents).value(); - browser()->tab_strip_model()->DetachAndDeleteWebContentsAt( - browser()->tab_strip_model()->GetIndexOfWebContents(web_contents)); - EXPECT_TRUE(base::Contains(*interval_map, tab_id)); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - EXPECT_EQ(0U, interval_map->size()); -} - namespace { class LenientMockTabStatsObserver : public TabStatsObserver { diff --git a/browser/metrics/tab_stats/tab_stats_tracker_unittest.cc b/browser/metrics/tab_stats/tab_stats_tracker_unittest.cc index 935430b8507e2d08ddfede1c6e621926d96c24af..1f2bd4a6232f37cc407c0e7afc9903b3714bbea3 100644 --- a/browser/metrics/tab_stats/tab_stats_tracker_unittest.cc +++ b/browser/metrics/tab_stats/tab_stats_tracker_unittest.cc @@ -61,7 +61,6 @@ class TestTabStatsTracker : public TabStatsTracker { public: using TabStatsTracker::OnHeartbeatEvent; using TabStatsTracker::OnInitialOrInsertedTab; - using TabStatsTracker::OnInterval; using TabStatsTracker::TabChangedAt; using UmaStatsReportingDelegate = TabStatsTracker::UmaStatsReportingDelegate; @@ -152,8 +151,6 @@ class TestTabStatsTracker : public TabStatsTracker { class TestUmaStatsReportingDelegate : public TestTabStatsTracker::UmaStatsReportingDelegate { public: - using TestTabStatsTracker::UmaStatsReportingDelegate:: - GetIntervalHistogramName; TestUmaStatsReportingDelegate() {} TestUmaStatsReportingDelegate(const TestUmaStatsReportingDelegate&) = delete; @@ -220,10 +217,6 @@ TestTabStatsTracker::TestTabStatsTracker(PrefService* pref_service) EXPECT_TRUE(daily_event_timer_for_testing()->IsRunning()); daily_event_timer_for_testing()->Stop(); - // Stop the usage interval timers so they don't trigger while running the - // tests. - usage_interval_timers_for_testing()->clear(); - reset_reporting_delegate_for_testing(new TestUmaStatsReportingDelegate()); // Stop the heartbeat timer to ensure that it doesn't interfere with the @@ -534,126 +527,6 @@ TEST_F(TabStatsTrackerTest, DailyDiscards) { kExpectedReloadsProactive2, 1); } -TEST_F(TabStatsTrackerTest, TabUsageGetsReported) { - constexpr base::TimeDelta kValidLongInterval = base::Hours(12); - TabStatsDataStore::TabsStateDuringIntervalMap* interval_map = - tab_stats_tracker_->data_store()->AddInterval(); - - std::vector> web_contentses; - for (size_t i = 0; i < 4; ++i) { - web_contentses.emplace_back(CreateTestWebContents()); - // Make sure that these WebContents are initially not visible. - web_contentses[i]->WasHidden(); - tab_stats_tracker_->OnInitialOrInsertedTab(web_contentses[i].get()); - } - - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - - histogram_tester_.ExpectUniqueSample( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate:: - kUnusedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 0, 1); - histogram_tester_.ExpectUniqueSample( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedTabsInIntervalHistogramNameBase, - kValidLongInterval), - web_contentses.size(), 1); - histogram_tester_.ExpectUniqueSample( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 0, 1); - histogram_tester_.ExpectUniqueSample( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedTabsInIntervalHistogramNameBase, - kValidLongInterval), - 0, 1); - - // Mark one tab as visible and make sure that it get reported properly. - web_contentses[0]->WasShown(); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate:: - kUnusedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 0, 2); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedTabsInIntervalHistogramNameBase, - kValidLongInterval), - web_contentses.size() - 1, 1); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 0, 2); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedTabsInIntervalHistogramNameBase, - kValidLongInterval), - 1, 1); - - // Mark a tab as audible and make sure that we now have 2 tabs marked as used. - content::WebContentsTester::For(web_contentses[1].get()) - ->SetIsCurrentlyAudible(true); - tab_stats_tracker_->TabChangedAt(web_contentses[1].get(), 1, - TabChangeType::kAll); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedTabsInIntervalHistogramNameBase, - kValidLongInterval), - web_contentses.size() - 2, 1); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedTabsInIntervalHistogramNameBase, - kValidLongInterval), - 2, 1); - - // Simulate an interaction on a tab, we should now see 3 tabs being marked as - // used. - content::WebContentsTester::For(web_contentses[2].get()) - ->TestDidReceiveMouseDownEvent(); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUnusedTabsInIntervalHistogramNameBase, - kValidLongInterval), - web_contentses.size() - 3, 1); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedTabsInIntervalHistogramNameBase, - kValidLongInterval), - 3, 1); - - // Remove the last WebContents, which should be reported as an unused tab. - web_contentses.pop_back(); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate:: - kUnusedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 1, 1); - - // Remove an active WebContents and make sure that this get reported properly. - // - // We need to re-interact with the WebContents as each call to |OnInterval| - // reset the interval and clear the interaction bit. - content::WebContentsTester::For(web_contentses.back().get()) - ->TestDidReceiveMouseDownEvent(); - web_contentses.pop_back(); - tab_stats_tracker_->OnInterval(kValidLongInterval, interval_map); - histogram_tester_.ExpectBucketCount( - TestUmaStatsReportingDelegate::GetIntervalHistogramName( - UmaStatsReportingDelegate::kUsedAndClosedInIntervalHistogramNameBase, - kValidLongInterval), - 1, 1); -} - TEST_F(TabStatsTrackerTest, HeartbeatMetrics) { size_t expected_tab_count = tab_stats_tracker_->AddTabs(12, this, tab_strip_model_); diff --git a/browser/offline_pages/background_loader_offliner.cc b/browser/offline_pages/background_loader_offliner.cc index 996b9d83a364887dbcafc74e76e0de6c9bbc9435..5385af5a973f210b852f14f00efc8b618222badf 100644 --- a/browser/offline_pages/background_loader_offliner.cc +++ b/browser/offline_pages/background_loader_offliner.cc @@ -16,12 +16,14 @@ #include "base/system/sys_info.h" #include "base/task/single_thread_task_runner.h" #include "base/time/time.h" +#include "chrome/browser/content_settings/page_specific_content_settings_delegate.h" #include "chrome/browser/offline_pages/offline_page_mhtml_archiver.h" #include "chrome/browser/offline_pages/offliner_helper.h" #include "chrome/browser/offline_pages/offliner_user_data.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ssl/security_state_tab_helper.h" #include "chrome/common/chrome_isolated_world_ids.h" +#include "components/content_settings/browser/page_specific_content_settings.h" #include "components/offline_pages/core/background/offliner_policy.h" #include "components/offline_pages/core/background/save_page_request.h" #include "components/offline_pages/core/client_namespace_constants.h" @@ -480,6 +482,10 @@ void BackgroundLoaderOffliner::ResetLoader() { loader_ = std::make_unique( browser_context_); loader_->SetDelegate(this); + content_settings::PageSpecificContentSettings::CreateForWebContents( + loader_->web_contents(), + std::make_unique( + loader_->web_contents())); } void BackgroundLoaderOffliner::AttachObservers() { diff --git a/browser/optimization_guide/chrome_browser_main_extra_parts_optimization_guide.cc b/browser/optimization_guide/chrome_browser_main_extra_parts_optimization_guide.cc index d0b3390f5bea260709c5bf05ba1847297ceeaf58..04d761df47bdda7eeeec07b99c9a0244f5d5b429 100644 --- a/browser/optimization_guide/chrome_browser_main_extra_parts_optimization_guide.cc +++ b/browser/optimization_guide/chrome_browser_main_extra_parts_optimization_guide.cc @@ -6,13 +6,12 @@ #include "base/files/file_path.h" #include "base/path_service.h" -#include "chrome/browser/browser_process.h" +#include "chrome/browser/optimization_guide/chrome_prediction_model_store.h" #include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h" #include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h" #include "chrome/common/chrome_paths.h" #include "components/optimization_guide/core/optimization_guide_features.h" #include "components/optimization_guide/core/prediction_manager.h" -#include "components/optimization_guide/core/prediction_model_store.h" namespace { @@ -31,6 +30,6 @@ void ChromeBrowserMainExtraPartsOptimizationGuide::PreCreateThreads() { model_downloads_dir = model_downloads_dir.Append(kOptimizationGuideModelStoreDirPrefix); // Create and initialize the install-wide model store. - optimization_guide::PredictionModelStore::GetInstance()->Initialize( - g_browser_process->local_state(), model_downloads_dir); + optimization_guide::ChromePredictionModelStore::GetInstance()->Initialize( + model_downloads_dir); } diff --git a/browser/optimization_guide/chrome_prediction_model_store.cc b/browser/optimization_guide/chrome_prediction_model_store.cc new file mode 100644 index 0000000000000000000000000000000000000000..e0bdd06f571af7e07ab4fa04bf89e9e5e22ed954 --- /dev/null +++ b/browser/optimization_guide/chrome_prediction_model_store.cc @@ -0,0 +1,24 @@ +// Copyright 2024 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/optimization_guide/chrome_prediction_model_store.h" + +#include "chrome/browser/browser_process.h" + +namespace optimization_guide { + +// static +ChromePredictionModelStore* ChromePredictionModelStore::GetInstance() { + static base::NoDestructor model_store; + return model_store.get(); +} + +ChromePredictionModelStore::ChromePredictionModelStore() = default; +ChromePredictionModelStore::~ChromePredictionModelStore() = default; + +PrefService* ChromePredictionModelStore::GetLocalState() const { + return g_browser_process->local_state(); +} + +} // namespace optimization_guide diff --git a/browser/optimization_guide/chrome_prediction_model_store.h b/browser/optimization_guide/chrome_prediction_model_store.h new file mode 100644 index 0000000000000000000000000000000000000000..74341c99e2ef580374b4841b0f133666aa2039a1 --- /dev/null +++ b/browser/optimization_guide/chrome_prediction_model_store.h @@ -0,0 +1,36 @@ +// Copyright 2024 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_OPTIMIZATION_GUIDE_CHROME_PREDICTION_MODEL_STORE_H_ +#define CHROME_BROWSER_OPTIMIZATION_GUIDE_CHROME_PREDICTION_MODEL_STORE_H_ + +#include "base/no_destructor.h" +#include "components/optimization_guide/core/prediction_model_store.h" + +class PrefService; + +namespace optimization_guide { + +class ChromePredictionModelStore : public PredictionModelStore { + public: + // Returns the singleton model store. + static ChromePredictionModelStore* GetInstance(); + + ChromePredictionModelStore(); + ~ChromePredictionModelStore() override; + + ChromePredictionModelStore(const ChromePredictionModelStore&) = delete; + ChromePredictionModelStore& operator=(const ChromePredictionModelStore&) = + delete; + + // optimization_guide::PredictionModelStore: + PrefService* GetLocalState() const override; + + private: + friend base::NoDestructor; +}; + +} // namespace optimization_guide + +#endif // CHROME_BROWSER_OPTIMIZATION_GUIDE_CHROME_PREDICTION_MODEL_STORE_H_ diff --git a/browser/optimization_guide/optimization_guide_keyed_service.cc b/browser/optimization_guide/optimization_guide_keyed_service.cc index f6f4a042634628dd18f2f672aba9da50c00d6505..e819173b2df85c5c630e4a929ff9a3bc21990651 100644 --- a/browser/optimization_guide/optimization_guide_keyed_service.cc +++ b/browser/optimization_guide/optimization_guide_keyed_service.cc @@ -19,6 +19,7 @@ #include "chrome/browser/download/background_download_service_factory.h" #include "chrome/browser/metrics/chrome_metrics_service_accessor.h" #include "chrome/browser/optimization_guide/chrome_hints_manager.h" +#include "chrome/browser/optimization_guide/chrome_prediction_model_store.h" #include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_key.h" @@ -36,7 +37,6 @@ #include "components/optimization_guide/core/optimization_guide_store.h" #include "components/optimization_guide/core/optimization_guide_util.h" #include "components/optimization_guide/core/prediction_manager.h" -#include "components/optimization_guide/core/prediction_model_store.h" #include "components/optimization_guide/core/tab_url_provider.h" #include "components/optimization_guide/core/top_host_provider.h" #include "components/optimization_guide/proto/models.pb.h" @@ -246,7 +246,7 @@ void OptimizationGuideKeyedService::Initialize() { prediction_manager_ = std::make_unique( prediction_model_and_features_store, optimization_guide::features::IsInstallWideModelStoreEnabled() - ? optimization_guide::PredictionModelStore::GetInstance() + ? optimization_guide::ChromePredictionModelStore::GetInstance() : nullptr, url_loader_factory, profile->GetPrefs(), profile->IsOffTheRecord(), g_browser_process->GetApplicationLocale(), model_downloads_dir, diff --git a/browser/picture_in_picture/picture_in_picture_window_manager.cc b/browser/picture_in_picture/picture_in_picture_window_manager.cc index 1f5fe33d59dc7e632ff18cb9665d805c238d96e9..e4c1a6981101a0bbfde0a0346474c5b23557ed6d 100644 --- a/browser/picture_in_picture/picture_in_picture_window_manager.cc +++ b/browser/picture_in_picture/picture_in_picture_window_manager.cc @@ -131,9 +131,12 @@ PictureInPictureWindowManager::EnterVideoPictureInPicture( return content::PictureInPictureResult::kSuccess; } -void PictureInPictureWindowManager::ExitPictureInPicture() { - if (pip_window_controller_) +bool PictureInPictureWindowManager::ExitPictureInPicture() { + if (pip_window_controller_) { CloseWindowInternal(); + return true; + } + return false; } void PictureInPictureWindowManager::FocusInitiator() { diff --git a/browser/picture_in_picture/picture_in_picture_window_manager.h b/browser/picture_in_picture/picture_in_picture_window_manager.h index 3a77e7ad86ee2b583420e2a41739600e20bf1646..7d2332132b4e44df22e1679bf09783a545e75203 100644 --- a/browser/picture_in_picture/picture_in_picture_window_manager.h +++ b/browser/picture_in_picture/picture_in_picture_window_manager.h @@ -62,7 +62,10 @@ class PictureInPictureWindowManager { void EnterPictureInPictureWithController( content::PictureInPictureWindowController* pip_window_controller); - void ExitPictureInPicture(); + // Closes any existing picture-in-picture windows (video or document pip). + // Returns true if a picture-in-picture window was closed, and false if there + // were no picture-in-picture windows to close. + bool ExitPictureInPicture(); // Called to notify that the initiator web contents should be focused. void FocusInitiator(); diff --git a/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc b/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc index f75ca4087aa91e89d8da92801b46a30dbae34691..4750a7bef8c44ea2c05528860caba2dc6216b5bf 100644 --- a/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc +++ b/browser/picture_in_picture/picture_in_picture_window_manager_unittest.cc @@ -4,11 +4,37 @@ #include "chrome/browser/picture_in_picture/picture_in_picture_window_manager.h" +#include "content/public/browser/picture_in_picture_window_controller.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/display/display.h" +namespace { + +class MockPictureInPictureWindowController + : public content::PictureInPictureWindowController { + public: + MockPictureInPictureWindowController() = default; + MockPictureInPictureWindowController( + const MockPictureInPictureWindowController&) = delete; + MockPictureInPictureWindowController& operator=( + const MockPictureInPictureWindowController&) = delete; + ~MockPictureInPictureWindowController() override = default; + + MOCK_METHOD(void, Show, (), (override)); + MOCK_METHOD(void, FocusInitiator, (), (override)); + MOCK_METHOD(void, Close, (bool), (override)); + MOCK_METHOD(void, CloseAndFocusInitiator, (), (override)); + MOCK_METHOD(void, OnWindowDestroyed, (bool), (override)); + MOCK_METHOD(content::WebContents*, GetWebContents, (), (override)); + MOCK_METHOD(absl::optional, GetWindowBounds, (), (override)); + MOCK_METHOD(content::WebContents*, GetChildWebContents, (), (override)); +}; + using PictureInPictureWindowManagerTest = testing::Test; +}; + TEST_F(PictureInPictureWindowManagerTest, RespectsMinAndMaxSize) { // The max window size should be 80% of the screen. display::Display display(/*id=*/1, gfx::Rect(0, 0, 1000, 1000)); @@ -54,3 +80,19 @@ TEST_F(PictureInPictureWindowManagerTest, RespectsMinAndMaxSize) { CalculateInitialPictureInPictureWindowBounds(pip_options, display) .size()); } + +TEST_F(PictureInPictureWindowManagerTest, + ExitPictureInPictureReturnsFalseWhenThereIsNoWindow) { + EXPECT_FALSE( + PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture()); +} + +TEST_F(PictureInPictureWindowManagerTest, + ExitPictureInPictureReturnsTrueAndClosesWindow) { + MockPictureInPictureWindowController controller; + PictureInPictureWindowManager::GetInstance() + ->EnterPictureInPictureWithController(&controller); + EXPECT_CALL(controller, Close(/*should_pause_video=*/false)); + EXPECT_TRUE( + PictureInPictureWindowManager::GetInstance()->ExitPictureInPicture()); +} \ No newline at end of file diff --git a/browser/policy/chrome_browser_policy_connector.cc b/browser/policy/chrome_browser_policy_connector.cc index 277c1ebf7fb14e9c0dabb6498b934fea98c8fb1a..797cf4d8fdd2dacca40266f65b7549799f7a50fa 100644 --- a/browser/policy/chrome_browser_policy_connector.cc +++ b/browser/policy/chrome_browser_policy_connector.cc @@ -275,7 +275,7 @@ ChromeBrowserPolicyConnector::CreatePlatformProvider() { ManagementServiceFactory::GetForPlatform(), kRegistryChromePolicyKey)); return std::make_unique(GetSchemaRegistry(), std::move(loader)); -#elif BUILDFLAG(IS_OHOS) && defined(ohos_edm_policy) +#elif BUILDFLAG(IS_OHOS) std::unique_ptr loader = std::make_unique( base::ThreadPool::CreateSequencedTaskRunner( diff --git a/browser/renderer_context_menu/render_view_context_menu.cc b/browser/renderer_context_menu/render_view_context_menu.cc index e0befaee3924b1d0c138033968c5ecdda0ae3dc7..030a21ee91761ddae7a7ed706c14c899d86fee75 100644 --- a/browser/renderer_context_menu/render_view_context_menu.cc +++ b/browser/renderer_context_menu/render_view_context_menu.cc @@ -609,7 +609,7 @@ bool ExtensionPatternMatch(const extensions::URLPatternSet& patterns, content::Referrer CreateReferrer(const GURL& url, const content::ContextMenuParams& params) { - const GURL& referring_url = params.frame_url; + const GURL& referring_url = params.frame_url;; return content::Referrer::SanitizeForRequest( url, content::Referrer(referring_url.GetAsReferrer(), params.referrer_policy)); @@ -820,10 +820,9 @@ bool RenderViewContextMenu::ExtensionContextAndPatternMatch( if (contexts.Contains(MenuItem::ALL) || (has_selection && contexts.Contains(MenuItem::SELECTION)) || (params.is_editable && contexts.Contains(MenuItem::EDITABLE)) || - (in_subframe && contexts.Contains(MenuItem::FRAME))) { + (in_subframe && contexts.Contains(MenuItem::FRAME))){ return true; } - if (has_link && contexts.Contains(MenuItem::LINK) && ExtensionPatternMatch(target_url_patterns, params.link_url)) return true; @@ -2722,8 +2721,8 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { OpenURLParams params = GetOpenURLParamsWithExtraHeaders( params_.link_url, params_.frame_url, params_.frame_origin, - new_tab_disposition, ui::PAGE_TRANSITION_LINK, - /*extra_headers=*/std::string(), /*started_from_context_menu=*/true); + new_tab_disposition, ui::PAGE_TRANSITION_LINK, + /*extra_headers=*/std::string(), /*started_from_context_menu=*/true); if (browser) { browser->OpenURL(params); @@ -2735,11 +2734,11 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { case IDC_CONTENT_CONTEXT_OPENLINKNEWWINDOW: DCHECK(!IsInProgressiveWebApp()); - OpenURLWithExtraHeaders(params_.link_url, params_.frame_url,params_.frame_origin, - WindowOpenDisposition::NEW_WINDOW, - ui::PAGE_TRANSITION_LINK, - /*extra_headers=*/std::string(), - /*started_from_context_menu=*/true); + OpenURLWithExtraHeaders( + params_.link_url, params_.frame_url, params_.frame_origin, + WindowOpenDisposition::NEW_WINDOW, ui::PAGE_TRANSITION_LINK, + /*extra_headers=*/std::string(), + /*started_from_context_menu=*/true); break; case IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD: @@ -2747,11 +2746,11 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { // that this won't and shouldn't be sent via the referrer header. // Note that PWA app windows are never incognito, we always open an // incognito browser tab. - OpenURLWithExtraHeaders(params_.link_url, params_.frame_url, params_.frame_origin, - WindowOpenDisposition::OFF_THE_RECORD, - ui::PAGE_TRANSITION_LINK, - /*extra_headers=*/std::string(), - /*started_from_context_menu=*/true); + OpenURLWithExtraHeaders( + params_.link_url, params_.frame_url, params_.frame_origin, + WindowOpenDisposition::NEW_WINDOW, ui::PAGE_TRANSITION_LINK, + /*extra_headers=*/std::string(), + /*started_from_context_menu=*/true); base::UmaHistogramEnumeration(kOpenLinkAsProfileHistogram, OpenLinkAs::kOpenLinkAsIncognitoClicked); break; @@ -2819,7 +2818,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { case IDC_CONTENT_CONTEXT_OPEN_ORIGINAL_IMAGE_NEW_TAB: OpenURLWithExtraHeaders(params_.src_url, params_.frame_url, - params_.frame_origin, + params_.frame_origin, WindowOpenDisposition::NEW_BACKGROUND_TAB, ui::PAGE_TRANSITION_LINK, std::string(), false); break; diff --git a/browser/renderer_context_menu/render_view_context_menu_browsertest.cc b/browser/renderer_context_menu/render_view_context_menu_browsertest.cc index 4d186c3502aa64c09725661ba7f5e4c4e66700c5..136e7769bad980e7130ba23724efd3471da32b1f 100644 --- a/browser/renderer_context_menu/render_view_context_menu_browsertest.cc +++ b/browser/renderer_context_menu/render_view_context_menu_browsertest.cc @@ -2855,7 +2855,7 @@ static std::string BuildCrossOriginChildFrameHTML(const GURL& link) { )"; - constexpr char kCrossOriginChildFrameSource[] = R"( + constexpr char kCrossOriginChildFrameSource[] = R"(
$i18n{propertiesDialogTitle}
diff --git a/browser/resources/pdf/elements/viewer-thumbnail.html b/browser/resources/pdf/elements/viewer-thumbnail.html index 2449a55d9644ee27ed9646db3f0692175c460cf6..20e0f6240b3e41856633186ac4c1ff5c5c3be42d 100644 --- a/browser/resources/pdf/elements/viewer-thumbnail.html +++ b/browser/resources/pdf/elements/viewer-thumbnail.html @@ -1,6 +1,7 @@