diff --git a/arm/third_party/libaom/cmake_update.sh b/arm/third_party/libaom/cmake_update.sh index 6472deca..ee0cb2e2 100755 --- a/arm/third_party/libaom/cmake_update.sh +++ b/arm/third_party/libaom/cmake_update.sh @@ -132,6 +132,8 @@ all_platforms+=" -DCONFIG_AV1_HIGHBITDEPTH=0" # Use real-time only build. all_platforms+=" -DCONFIG_REALTIME_ONLY=1" all_platforms+=" -DCONFIG_AV1_TEMPORAL_DENOISING=1" +# Disable Quantization Matrix. +all_platforms+=" -DCONFIG_QUANT_MATRIX=0" # avx2 optimizations account for ~0.3mb of the decoder. #all_platforms+=" -DENABLE_AVX2=0" toolchain="-DCMAKE_TOOLCHAIN_FILE=${SRC}/build/cmake/toolchains" diff --git a/arm/third_party/libvpx/BUILD.gn b/arm/third_party/libvpx/BUILD.gn index 38ec8fe4..5968f8bc 100644 --- a/arm/third_party/libvpx/BUILD.gn +++ b/arm/third_party/libvpx/BUILD.gn @@ -104,6 +104,42 @@ config("libvpx_public_config") { ] } +executable("decode_encode_profile_test") { + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + include_dirs = libvpx_include_dirs + [ + "source/libvpx/third_party/libwebm/", + "source/libvpx/third_party/googletest/src/include/", + "source/libvpx/third_party/googletest/src/", + ] + + testonly = true + sources = [ + "source/libvpx/test/decode_test_driver.cc", + "source/libvpx/test/encode_test_driver.cc", + "source/libvpx/test/init_vpx_test.cc", + "source/libvpx/test/test_libvpx.cc", + "source/libvpx/test/test_vectors.cc", + "source/libvpx/test/test_vectors.h", + "source/libvpx/third_party/googletest/src/src/gtest-all.cc", + "source/libvpx/third_party/libwebm/mkvparser/mkvparser.cc", + "source/libvpx/third_party/libwebm/mkvparser/mkvreader.cc", + "source/libvpx/tools_common.h", + "source/libvpx/webmdec.cc", + "source/libvpx/y4minput.c", + "tests/pgo/decode_encode_profile_test.cc", + ] + deps = [ ":libvpx" ] + + # gtest-death-test dependency on fdio for fuchsia builds + if (is_fuchsia) { + deps += [ + "//third_party/fuchsia-sdk/sdk/pkg/fdio", + "//third_party/fuchsia-sdk/sdk/pkg/zx", + ] + } +} + if (current_cpu == "x86" || (current_cpu == "x64" && !is_msan)) { nasm_assemble("libvpx_asm") { if (current_cpu == "x86") { diff --git a/arm/third_party/xnnpack/BUILD.gn b/arm/third_party/xnnpack/BUILD.gn index b61c3178..9bf3ce70 100644 --- a/arm/third_party/xnnpack/BUILD.gn +++ b/arm/third_party/xnnpack/BUILD.gn @@ -30,17 +30,13 @@ config("xnnpack_config") { ] defines = [ + "CHROMIUM", + # Don't enable this without first talking to Chrome Security! # XNNPACK runs in the browser process. The hardening and fuzzing needed # to ensure JIT can be used safely is not in place yet. "XNN_ENABLE_JIT=0", - # TODO: b/327013106 - Before enabling this and removing - # --define=xnn_enable_avx512amx=false from the generation script, ensure - # the detection has been updated to remove use of syscall() or that the - # function has been allowed in the sandbox. - "XNN_ENABLE_AVX512AMX=0", - "XNN_ENABLE_ASSEMBLY=1", "XNN_ENABLE_GEMM_M_SPECIALIZATION=1", "XNN_ENABLE_MEMOPT=1", @@ -68,13 +64,13 @@ if (current_cpu == "x64" || current_cpu == "x86") { ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vbmi", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni", + ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni-amx-tile-amx-int8", ":amalgam_f16c-fma-no-avx2", ":amalgam_f16c-no-avx2-no-fma", ":amalgam_sse2-no-sse3", ":amalgam_sse4.1-no-sse4.2", ":amalgam_ssse3-no-sse4.1", ":amalgam_x64", - ":amalgam_xop-no-avx2", ":configs_x64", ":enums_x64", ":operators_x64", @@ -88,6 +84,7 @@ if (current_cpu == "x64" || current_cpu == "x86") { ":amalgam_avx512f_standalone", ":amalgam_f16c-fma-avx2_standalone", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vbmi_standalone", + ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni-amx-tile-amx-int8_standalone", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni_standalone", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni_standalone", ":amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl_standalone", @@ -97,7 +94,6 @@ if (current_cpu == "x64" || current_cpu == "x86") { ":amalgam_sse4.1-no-sse4.2_standalone", ":amalgam_ssse3-no-sse4.1_standalone", ":amalgam_x64_standalone", - ":amalgam_xop-no-avx2_standalone", ":configs_x64_standalone", ":enums_x64_standalone", ":operators_x64_standalone", @@ -704,6 +700,75 @@ if (current_cpu == "x64" || current_cpu == "x86") { } } + source_set( + "amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni-amx-tile-amx-int8") { + cflags = [ + "-mamx-int8", + "-mamx-tile", + "-mavx512bw", + "-mavx512cd", + "-mavx512dq", + "-mavx512f", + "-mavx512vl", + "-mavx512vnni", + "-mf16c", + "-mfma", + "-mgfni", + ] + + sources = [ "src/src/amalgam/gen/avx512amx.c" ] + + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + configs += [ "//build/config/sanitizers:cfi_icall_generalize_pointers" ] + + deps = [ + "//third_party/cpuinfo", + "//third_party/fp16", + "//third_party/fxdiv", + "//third_party/pthreadpool", + ] + + public_configs = [ ":xnnpack_config" ] + } + + # This is a target that cannot depend on //base. + source_set( + "amalgam_f16c-fma-avx512f-avx512cd-avx512bw-avx512dq-avx512vl-avx512vnni-gfni-amx-tile-amx-int8_standalone") { + cflags = [ + "-mamx-int8", + "-mamx-tile", + "-mavx512bw", + "-mavx512cd", + "-mavx512dq", + "-mavx512f", + "-mavx512vl", + "-mavx512vnni", + "-mf16c", + "-mfma", + "-mgfni", + ] + + sources = [ "src/src/amalgam/gen/avx512amx.c" ] + + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + configs += [ "//build/config/sanitizers:cfi_icall_generalize_pointers" ] + + deps = [ + "//third_party/cpuinfo", + "//third_party/fp16", + "//third_party/fxdiv", + "//third_party/pthreadpool:pthreadpool_standalone", + ] + + public_configs = [ ":xnnpack_config" ] + + if (!(is_android && use_order_profiling)) { + assert_no_deps = [ "//base" ] + } + } + source_set("amalgam_f16c-fma-no-avx2") { cflags = [ "-mf16c", @@ -1002,55 +1067,6 @@ if (current_cpu == "x64" || current_cpu == "x86") { } } - source_set("amalgam_xop-no-avx2") { - cflags = [ - "-mno-avx2", - "-mxop", - ] - - sources = [ "src/src/amalgam/gen/xop.c" ] - - configs -= [ "//build/config/compiler:chromium_code" ] - configs += [ "//build/config/compiler:no_chromium_code" ] - configs += [ "//build/config/sanitizers:cfi_icall_generalize_pointers" ] - - deps = [ - "//third_party/cpuinfo", - "//third_party/fp16", - "//third_party/fxdiv", - "//third_party/pthreadpool", - ] - - public_configs = [ ":xnnpack_config" ] - } - - # This is a target that cannot depend on //base. - source_set("amalgam_xop-no-avx2_standalone") { - cflags = [ - "-mno-avx2", - "-mxop", - ] - - sources = [ "src/src/amalgam/gen/xop.c" ] - - configs -= [ "//build/config/compiler:chromium_code" ] - configs += [ "//build/config/compiler:no_chromium_code" ] - configs += [ "//build/config/sanitizers:cfi_icall_generalize_pointers" ] - - deps = [ - "//third_party/cpuinfo", - "//third_party/fp16", - "//third_party/fxdiv", - "//third_party/pthreadpool:pthreadpool_standalone", - ] - - public_configs = [ ":xnnpack_config" ] - - if (!(is_android && use_order_profiling)) { - assert_no_deps = [ "//base" ] - } - } - source_set("configs_x64") { cflags = [] @@ -1160,6 +1176,7 @@ if (current_cpu == "x64" || current_cpu == "x86") { cflags = [] sources = [ + "src/src/enums/allocation-type.c", "src/src/enums/datatype-strings.c", "src/src/enums/microkernel-type.c", "src/src/enums/node-type.c", @@ -1185,6 +1202,7 @@ if (current_cpu == "x64" || current_cpu == "x86") { cflags = [] sources = [ + "src/src/enums/allocation-type.c", "src/src/enums/datatype-strings.c", "src/src/enums/microkernel-type.c", "src/src/enums/node-type.c", @@ -1343,6 +1361,7 @@ if (current_cpu == "x64" || current_cpu == "x86") { "src/src/subgraph/multiply2.c", "src/src/subgraph/negate.c", "src/src/subgraph/prelu.c", + "src/src/subgraph/reciprocal-square-root.c", "src/src/subgraph/reshape-2d.c", "src/src/subgraph/reshape-helpers.c", "src/src/subgraph/rope.c", @@ -1415,6 +1434,7 @@ if (current_cpu == "x64" || current_cpu == "x86") { "src/src/subgraph/multiply2.c", "src/src/subgraph/negate.c", "src/src/subgraph/prelu.c", + "src/src/subgraph/reciprocal-square-root.c", "src/src/subgraph/reshape-2d.c", "src/src/subgraph/reshape-helpers.c", "src/src/subgraph/rope.c", @@ -1872,6 +1892,7 @@ if (current_cpu == "arm64") { cflags = [] sources = [ + "src/src/enums/allocation-type.c", "src/src/enums/datatype-strings.c", "src/src/enums/microkernel-type.c", "src/src/enums/node-type.c", @@ -1897,6 +1918,7 @@ if (current_cpu == "arm64") { cflags = [] sources = [ + "src/src/enums/allocation-type.c", "src/src/enums/datatype-strings.c", "src/src/enums/microkernel-type.c", "src/src/enums/node-type.c", @@ -3155,12 +3177,6 @@ if (current_cpu == "arm64") { "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-cortex-a75.S", "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64-prfm.S", "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-fp32-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-fp32-asm-aarch64-neondot-ld128.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", ] configs -= [ "//build/config/compiler:chromium_code" ] @@ -3188,12 +3204,6 @@ if (current_cpu == "arm64") { "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-cortex-a75.S", "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64-prfm.S", "src/src/qu8-gemm/gen/qu8-gemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-fp32-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-fp32-asm-aarch64-neondot-ld128.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-gemm/gen/qu8-gemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", ] configs -= [ "//build/config/compiler:chromium_code" ] @@ -3224,12 +3234,6 @@ if (current_cpu == "arm64") { "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-cortex-a75.S", "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64-prfm.S", "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-fp32-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-fp32-asm-aarch64-neondot-ld128.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", ] configs -= [ "//build/config/compiler:chromium_code" ] @@ -3257,12 +3261,6 @@ if (current_cpu == "arm64") { "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-cortex-a75.S", "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64-prfm.S", "src/src/qu8-igemm/gen/qu8-igemm-4x16-minmax-rndnu-asm-aarch64-neon-mlal-lane-ld64.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-fp32-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-fp32-asm-aarch64-neondot-ld128.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x16c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-cortex-a55.S", - "src/src/qu8-igemm/gen/qu8-igemm-4x8c4-minmax-rndnu-asm-aarch64-neondot-ld128.S", ] configs -= [ "//build/config/compiler:chromium_code" ] @@ -3318,6 +3316,7 @@ if (current_cpu == "arm64") { "src/src/subgraph/multiply2.c", "src/src/subgraph/negate.c", "src/src/subgraph/prelu.c", + "src/src/subgraph/reciprocal-square-root.c", "src/src/subgraph/reshape-2d.c", "src/src/subgraph/reshape-helpers.c", "src/src/subgraph/rope.c", @@ -3390,6 +3389,7 @@ if (current_cpu == "arm64") { "src/src/subgraph/multiply2.c", "src/src/subgraph/negate.c", "src/src/subgraph/prelu.c", + "src/src/subgraph/reciprocal-square-root.c", "src/src/subgraph/reshape-2d.c", "src/src/subgraph/reshape-helpers.c", "src/src/subgraph/rope.c", diff --git a/arm/third_party/xnnpack/generate_build_gn.py b/arm/third_party/xnnpack/generate_build_gn.py index 4943ac45..d454c1ec 100755 --- a/arm/third_party/xnnpack/generate_build_gn.py +++ b/arm/third_party/xnnpack/generate_build_gn.py @@ -36,6 +36,7 @@ import atexit import collections +import io import json import logging import os @@ -43,6 +44,8 @@ import platform import shutil import subprocess import sys +import urllib.request +import zipfile from dataclasses import dataclass, field @@ -79,17 +82,13 @@ config("xnnpack_config") { ] defines = [ + "CHROMIUM", + # Don't enable this without first talking to Chrome Security! # XNNPACK runs in the browser process. The hardening and fuzzing needed # to ensure JIT can be used safely is not in place yet. "XNN_ENABLE_JIT=0", - # TODO: b/327013106 - Before enabling this and removing - # --define=xnn_enable_avx512amx=false from the generation script, ensure - # the detection has been updated to remove use of syscall() or that the - # function has been allowed in the sandbox. - "XNN_ENABLE_AVX512AMX=0", - "XNN_ENABLE_ASSEMBLY=1", "XNN_ENABLE_GEMM_M_SPECIALIZATION=1", "XNN_ENABLE_MEMOPT=1", @@ -530,7 +529,6 @@ def GenerateObjectBuilds(cpu): 'mnemonic("CppCompile", filter("//:", deps(:xnnpack_for_tflite)))', '--define', 'xnn_enable_jit=false', - '--define=xnn_enable_avx512amx=false', "--output=jsonproto", ]) logging.info('parsing actions from bazel aquery...') @@ -633,6 +631,17 @@ def MakeXNNPACKDepsList(target_sss): return deps_list +def EnsureAndroidNDK(): + """ + Ensures that the Android NDK is available and bazel can find it later. + """ + if 'ANDROID_NDK_HOME' in os.environ: + return + logging.info('Downloading a copy of the Android NDK for bazel') + resp = urllib.request.urlopen('https://dl.google.com/android/repository/android-ndk-r19c-linux-x86_64.zip') + logging.info('Unpacking the Android NDK') + zipfile.ZipFile(io.BytesIO(resp.read())).extractall(path='/tmp/') + os.environ['ANDROID_NDK_HOME'] = '/tmp/android-ndk-r19c' def MakeXNNPACKSourceSet(ss): """ @@ -657,6 +666,8 @@ def main(): logging.error('On x86-64 Debian, install gcc-aarch64-linux-gnu and gcc.') sys.exit(1) + EnsureAndroidNDK() + CreateToolchainFiles() # Create SourceSet's for each target architecture. diff --git a/src/ash/public/cpp/accelerators.cc b/src/ash/public/cpp/accelerators.cc index a48f88b4..db1fc8cd 100644 --- a/src/ash/public/cpp/accelerators.cc +++ b/src/ash/public/cpp/accelerators.cc @@ -313,6 +313,10 @@ const AcceleratorData kAcceleratorData[] = { // Projector shortcuts. {true, ui::VKEY_OEM_3, ui::EF_COMMAND_DOWN, AcceleratorAction::kToggleProjectorMarker}, + + // Accessibility key. + {true, ui::VKEY_ACCESSIBILITY, ui::EF_NONE, + AcceleratorAction::kAccessibilityAction}, }; const size_t kAcceleratorDataLength = std::size(kAcceleratorData); @@ -397,11 +401,21 @@ const size_t kToggleGameDashboardAcceleratorDataLength = std::size(kToggleGameDashboardAcceleratorData); const AcceleratorData kTogglePickerAcceleratorData[] = { - {true, ui::VKEY_S, ui::EF_COMMAND_DOWN, AcceleratorAction::kTogglePicker}}; + {true, ui::VKEY_RIGHT_ALT, ui::EF_NONE, AcceleratorAction::kTogglePicker}, + {true, ui::VKEY_F, ui::EF_COMMAND_DOWN, AcceleratorAction::kTogglePicker}, +}; const size_t kTogglePickerAcceleratorDataLength = std::size(kTogglePickerAcceleratorData); +const AcceleratorData kTogglePickerFlipAcceleratorData[] = { + {false, ui::VKEY_RIGHT_ALT, ui::EF_NONE, AcceleratorAction::kTogglePicker}, + {true, ui::VKEY_F, ui::EF_COMMAND_DOWN, AcceleratorAction::kTogglePicker}, +}; + +const size_t kTogglePickerFlipAcceleratorDataLength = + std::size(kTogglePickerAcceleratorData); + // static AcceleratorController* AcceleratorController::Get() { return g_instance; diff --git a/src/chrome/app/vector_icons/BUILD.gn b/src/chrome/app/vector_icons/BUILD.gn index 519de401..e21f4dc8 100644 --- a/src/chrome/app/vector_icons/BUILD.gn +++ b/src/chrome/app/vector_icons/BUILD.gn @@ -11,8 +11,8 @@ import("//ui/webui/webui_features.gni") aggregate_vector_icons("chrome_vector_icons") { icon_directory = "." - # Keep sorted alphabetically. sources = [ + # go/keep-sorted start "account_add_chrome_refresh.icon", "account_box.icon", "account_child.icon", @@ -57,13 +57,13 @@ aggregate_vector_icons("chrome_vector_icons") { "click_to_call_illustration.icon", "click_to_call_illustration_dark.icon", "close_chrome_refresh.icon", - "close_group.icon", "close_group_refresh.icon", "close_tab_chrome_refresh.icon", "computer_with_circle_background.icon", "copy.icon", "copy_menu.icon", "crashed_tab.icon", + "create_new_tab_group.icon", "credit_card.icon", "credit_card_chrome_refresh.icon", "cut_menu.icon", @@ -108,6 +108,7 @@ aggregate_vector_icons("chrome_vector_icons") { "incognito_menu_art.icon", "incognito_profile.icon", "incognito_refresh_menu.icon", + "info.icon", "ink_highlighter.icon", "input.icon", "install_desktop_chrome_refresh.icon", @@ -135,7 +136,6 @@ aggregate_vector_icons("chrome_vector_icons") { "menu_book_chrome_refresh.icon", "mixed_content.icon", "more_tools_menu.icon", - "move_group_to_new_window.icon", "move_group_to_new_window_refresh.icon", "my_location.icon", "name_window.icon", @@ -145,7 +145,6 @@ aggregate_vector_icons("chrome_vector_icons") { "navigate_stop.icon", "navigate_stop_chrome_refresh.icon", "navigate_stop_touch.icon", - "new_tab_in_group.icon", "new_tab_in_group_refresh.icon", "new_tab_refresh.icon", "new_window.icon", @@ -164,6 +163,7 @@ aggregate_vector_icons("chrome_vector_icons") { "payments/save_card_and_vcn_success_confirmation.icon", "payments/save_card_and_vcn_success_confirmation_dark.icon", "performance.icon", + "person.icon", "person_filled_padded_large.icon", "person_filled_padded_small.icon", "photo_camera.icon", @@ -184,7 +184,6 @@ aggregate_vector_icons("chrome_vector_icons") { "read_anything_line_spacing_very_loose.icon", "read_anything_links_disabled.icon", "read_anything_links_enabled.icon", - "read_later.icon", "read_later_add.icon", "reading_list.icon", "release_alert.icon", @@ -198,7 +197,6 @@ aggregate_vector_icons("chrome_vector_icons") { "right_panel_close.icon", "sad_tab.icon", "safety_check.icon", - "save_group.icon", "save_group_refresh.icon", "save_page.icon", "saved_tab_group_bar_everything.icon", @@ -257,7 +255,6 @@ aggregate_vector_icons("chrome_vector_icons") { "trash_can_refresh.icon", "tv.icon", "unbranded_translate.icon", - "ungroup.icon", "ungroup_refresh.icon", "usb_cable.icon", "user_account_avatar.icon", @@ -274,8 +271,6 @@ aggregate_vector_icons("chrome_vector_icons") { "webauthn/passkey_error_dark.icon", "webauthn/passkey_fingerprint.icon", "webauthn/passkey_fingerprint_dark.icon", - "webauthn/passkey_header.icon", - "webauthn/passkey_header_dark.icon", "webauthn/passkey_phone.icon", "webauthn/passkey_phone_dark.icon", "webauthn/passkey_usb.icon", @@ -283,6 +278,8 @@ aggregate_vector_icons("chrome_vector_icons") { "webauthn/usb_security_key.icon", "webauthn/webauthn_error.icon", "webauthn/webauthn_error_dark.icon", + "webauthn/windows_hello.icon", + "webid_globe.icon", "zoom_in.icon", "zoom_minus.icon", "zoom_minus_chrome_refresh.icon", @@ -290,6 +287,8 @@ aggregate_vector_icons("chrome_vector_icons") { "zoom_plus.icon", "zoom_plus_chrome_refresh.icon", "zoom_plus_menu_refresh.icon", + + # go/keep-sorted end ] if (is_thorium_build) { diff --git a/src/chrome/app/vector_icons/info.icon b/src/chrome/app/vector_icons/info.icon new file mode 100644 index 00000000..ee102622 --- /dev/null +++ b/src/chrome/app/vector_icons/info.icon @@ -0,0 +1,53 @@ +// 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. + +CANVAS_DIMENSIONS, 960, +FILL_RULE_NONZERO, +PATH_COLOR_ARGB, 0xFF, 0x1A, 0x73, 0xE8, +MOVE_TO, 440, 680, +R_H_LINE_TO, 80, +R_V_LINE_TO, -240, +R_H_LINE_TO, -80, +R_V_LINE_TO, 240, +CLOSE, +R_MOVE_TO, 40, -320, +R_QUADRATIC_TO, 17, 0, 28.5f, -11.5f, +QUADRATIC_TO_SHORTHAND, 520, 320, +R_QUADRATIC_TO, 0, -17, -11.5f, -28.5f, +QUADRATIC_TO_SHORTHAND, 480, 280, +R_QUADRATIC_TO, -17, 0, -28.5f, 11.5f, +QUADRATIC_TO_SHORTHAND, 440, 320, +R_QUADRATIC_TO, 0, 17, 11.5f, 28.5f, +QUADRATIC_TO_SHORTHAND, 480, 360, +CLOSE, +R_MOVE_TO, 0, 520, +R_QUADRATIC_TO, -83, 0, -156, -31.5f, +QUADRATIC_TO_SHORTHAND, 197, 763, +R_QUADRATIC_TO, -54, -54, -85.5f, -127, +QUADRATIC_TO_SHORTHAND, 80, 480, +R_QUADRATIC_TO, 0, -83, 31.5f, -156, +QUADRATIC_TO_SHORTHAND, 197, 197, +R_QUADRATIC_TO, 54, -54, 127, -85.5f, +QUADRATIC_TO_SHORTHAND, 480, 80, +R_QUADRATIC_TO, 83, 0, 156, 31.5f, +QUADRATIC_TO_SHORTHAND, 763, 197, +R_QUADRATIC_TO, 54, 54, 85.5f, 127, +QUADRATIC_TO_SHORTHAND, 880, 480, +R_QUADRATIC_TO, 0, 83, -31.5f, 156, +QUADRATIC_TO_SHORTHAND, 763, 763, +R_QUADRATIC_TO, -54, 54, -127, 85.5f, +QUADRATIC_TO_SHORTHAND, 480, 880, +CLOSE, +R_MOVE_TO, 0, -80, +R_QUADRATIC_TO, 134, 0, 227, -93, +R_QUADRATIC_TO, 93, -93, 93, -227, +R_QUADRATIC_TO, 0, -134, -93, -227, +R_QUADRATIC_TO, -93, -93, -227, -93, +R_QUADRATIC_TO, -134, 0, -227, 93, +R_QUADRATIC_TO, -93, 93, -93, 227, +R_QUADRATIC_TO, 0, 134, 93, 227, +R_QUADRATIC_TO, 93, 93, 227, 93, +CLOSE, +R_MOVE_TO, 0, -320, +CLOSE diff --git a/src/chrome/browser/download/bubble/download_bubble_prefs.cc b/src/chrome/browser/download/bubble/download_bubble_prefs.cc index ea3d59cf..547399da 100644 --- a/src/chrome/browser/download/bubble/download_bubble_prefs.cc +++ b/src/chrome/browser/download/bubble/download_bubble_prefs.cc @@ -4,8 +4,8 @@ #include "chrome/browser/download/bubble/download_bubble_prefs.h" -#include "base/feature_list.h" #include "base/command_line.h" +#include "base/feature_list.h" #include "build/chromeos_buildflags.h" #include "chrome/browser/download/download_core_service.h" #include "chrome/browser/download/download_core_service_factory.h" diff --git a/src/chrome/browser/download/chrome_download_manager_delegate.cc b/src/chrome/browser/download/chrome_download_manager_delegate.cc index 2def04e4..28ef1664 100644 --- a/src/chrome/browser/download/chrome_download_manager_delegate.cc +++ b/src/chrome/browser/download/chrome_download_manager_delegate.cc @@ -57,7 +57,6 @@ #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_features.h" #include "chrome/common/chrome_paths.h" -#include "chrome/common/pdf_util.h" #include "chrome/common/pref_names.h" #include "chrome/grit/generated_resources.h" #include "components/download/public/common/download_danger_type.h" @@ -67,6 +66,7 @@ #include "components/download/public/common/download_stats.h" #include "components/offline_pages/buildflags/buildflags.h" #include "components/pdf/common/constants.h" +#include "components/pdf/common/pdf_util.h" #include "components/policy/core/common/policy_pref_names.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_member.h" @@ -75,6 +75,7 @@ #include "components/safe_browsing/content/browser/download/download_stats.h" #include "components/safe_browsing/content/browser/web_ui/safe_browsing_ui.h" #include "components/safe_browsing/content/common/file_type_policies.h" +#include "components/safe_browsing/core/common/features.h" #include "components/safe_search_api/safe_search_util.h" #include "components/services/quarantine/public/mojom/quarantine.mojom.h" #include "components/services/quarantine/quarantine_impl.h" @@ -110,6 +111,7 @@ #include "chrome/browser/download/android/insecure_download_dialog_bridge.h" #include "chrome/browser/download/android/insecure_download_infobar_delegate.h" #include "chrome/browser/flags/android/chrome_feature_list.h" +#include "chrome/browser/ui/android/pdf/pdf_jni_headers/PdfUtils_jni.h" #include "chrome/browser/ui/android/tab_model/tab_model.h" #include "chrome/browser/ui/android/tab_model/tab_model_list.h" #include "components/infobars/content/content_infobar_manager.h" @@ -168,6 +170,8 @@ namespace { // there is no user interaction). constexpr base::TimeDelta kEphemeralWarningLifetimeBeforeCancel = base::Hours(1); +#else +const char kPdfDirName[] = "pdfs"; #endif // Used with GetPlatformDownloadPath() to indicate which platform path to @@ -433,8 +437,6 @@ download::DownloadDangerType SavePackageDangerType( return download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS; case safe_browsing::DownloadCheckResult::DEEP_SCANNED_SAFE: return download::DOWNLOAD_DANGER_TYPE_DEEP_SCANNED_SAFE; - case safe_browsing::DownloadCheckResult::BLOCKED_UNSUPPORTED_FILE_TYPE: - return download::DOWNLOAD_DANGER_TYPE_BLOCKED_UNSUPPORTED_FILETYPE; case safe_browsing::DownloadCheckResult::BLOCKED_PASSWORD_PROTECTED: return download::DOWNLOAD_DANGER_TYPE_BLOCKED_PASSWORD_PROTECTED; case safe_browsing::DownloadCheckResult::BLOCKED_TOO_LARGE: @@ -549,6 +551,14 @@ void ChromeDownloadManagerDelegate::Shutdown() { } } +void ChromeDownloadManagerDelegate::OnDownloadCanceledAtShutdown( + download::DownloadItem* item) { + // Be careful, limited objects are still alive at this point. This function is + // called at profile shutdown. Only keyed service, downloadItem and objects + // directly owned by the browser process are available. + MaybeSendDangerousDownloadCanceledReport(item, /*is_shutdown=*/true); +} + content::DownloadIdCallback ChromeDownloadManagerDelegate::GetDownloadIdReceiverCallback() { return base::BindOnce(&ChromeDownloadManagerDelegate::SetNextId, @@ -616,20 +626,24 @@ bool ChromeDownloadManagerDelegate::DetermineDownloadTarget( DownloadPathReservationTracker::FilenameConflictAction action = kDefaultPlatformConflictAction; #if BUILDFLAG(IS_ANDROID) - if (download->IsTransient() && download_path.empty() && - download->GetMimeType() == pdf::kPDFMimeType && - !download->IsMustDownload()) { - base::FilePath generated_filename = net::GenerateFileName( - download->GetURL(), download->GetContentDisposition(), - profile_->GetPrefs()->GetString(prefs::kDefaultCharset), - download->GetSuggestedFilename(), download->GetMimeType(), - l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME)); - base::FilePath cache_dir; - base::android::GetCacheDirectory(&cache_dir); - download_path = cache_dir.Append(generated_filename); - } - if (!download_path.empty()) + if (download->IsTransient()) { + if (download_path.empty() && download->GetMimeType() == pdf::kPDFMimeType && + !download->IsMustDownload()) { + base::FilePath generated_filename = net::GenerateFileName( + download->GetURL(), download->GetContentDisposition(), + profile_->GetPrefs()->GetString(prefs::kDefaultCharset), + download->GetSuggestedFilename(), download->GetMimeType(), + l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME)); + base::FilePath cache_dir; + base::android::GetCacheDirectory(&cache_dir); + download_path = cache_dir.Append(kPdfDirName).Append(generated_filename); + action = DownloadPathReservationTracker::UNIQUIFY; + } else { + action = DownloadPathReservationTracker::OVERWRITE; + } + } else if (!download_path.empty()) { action = DownloadPathReservationTracker::UNIQUIFY; + } #endif DownloadTargetDeterminer::Start(download, download_path, action, download_prefs_.get(), this, @@ -644,7 +658,7 @@ bool ChromeDownloadManagerDelegate::ShouldAutomaticallyOpenFile( if (path.Extension().empty()) return false; #if BUILDFLAG(ENABLE_EXTENSIONS) - // TODO(crbug.com/1077929): This determination is done based on |path|, while + // TODO(crbug.com/40129365): This determination is done based on |path|, while // ShouldOpenDownload() detects extension downloads based on the // characteristics of the download. Reconcile this. if (path.MatchesExtension(extensions::kExtensionFileExtension)) @@ -672,7 +686,7 @@ bool ChromeDownloadManagerDelegate::ShouldAutomaticallyOpenFileByPolicy( if (path.Extension().empty()) return false; #if BUILDFLAG(ENABLE_EXTENSIONS) - // TODO(crbug.com/1077929): This determination is done based on |path|, while + // TODO(crbug.com/40129365): This determination is done based on |path|, while // ShouldOpenDownload() detects extension downloads based on the // characteristics of the download. Reconcile this. if (path.MatchesExtension(extensions::kExtensionFileExtension)) @@ -714,11 +728,14 @@ bool ChromeDownloadManagerDelegate::IsDownloadReadyForCompletion( DCHECK_CURRENTLY_ON(BrowserThread::UI); #if BUILDFLAG(FULL_SAFE_BROWSING) // If this is a chrome triggered download, return true; - if (!item->RequireSafetyChecks()) + if (!item->RequireSafetyChecks()) { return true; + } if (!download_prefs_->safebrowsing_for_trusted_sources_enabled() && - download_prefs_->IsFromTrustedSource(*item)) { + download_prefs_->IsFromTrustedSource(*item) && + !safe_browsing::DeepScanningRequest::ShouldUploadBinary(item) + .has_value()) { return true; } @@ -888,8 +905,7 @@ bool ChromeDownloadManagerDelegate::InterceptDownloadIfApplicable( } } - if (base::FeatureList::IsEnabled(features::kAndroidOpenPdfInline) && - mime_type == pdf::kPDFMimeType) { + if (ShouldOpenPdfInline() && mime_type == pdf::kPDFMimeType) { // If this is already a file, there is no need to download. if (url.SchemeIsFile() || url.SchemeIs("content")) { return true; @@ -995,10 +1011,11 @@ void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) { content::Referrer(), WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK, false); - if (download->GetMimeType() == "application/x-x509-user-cert") + if (download->GetMimeType() == "application/x-x509-user-cert") { chrome::ShowSettingsSubPage(browser, "certificates"); - else - browser->OpenURL(params); + } else { + browser->OpenURL(params, /*navigation_handle_callback=*/{}); + } RecordDownloadOpen(DOWNLOAD_OPEN_METHOD_DEFAULT_BROWSER, download->GetMimeType()); @@ -1456,10 +1473,6 @@ void ChromeDownloadManagerDelegate::CheckClientDownloadDone( danger_type = download::DOWNLOAD_DANGER_TYPE_PROMPT_FOR_SCANNING; is_pending_scanning = true; break; - case safe_browsing::DownloadCheckResult::BLOCKED_UNSUPPORTED_FILE_TYPE: - danger_type = - download::DOWNLOAD_DANGER_TYPE_BLOCKED_UNSUPPORTED_FILETYPE; - break; case safe_browsing::DownloadCheckResult::DANGEROUS_ACCOUNT_COMPROMISE: danger_type = download::DOWNLOAD_DANGER_TYPE_DANGEROUS_ACCOUNT_COMPROMISE; @@ -1477,14 +1490,16 @@ void ChromeDownloadManagerDelegate::CheckClientDownloadDone( danger_type = download::DOWNLOAD_DANGER_TYPE_BLOCKED_SCAN_FAILED; break; case safe_browsing::DownloadCheckResult::IMMEDIATE_DEEP_SCAN: - is_pending_scanning = true; - danger_type = download::DOWNLOAD_DANGER_TYPE_ASYNC_SCANNING; safe_browsing::DownloadProtectionService::UploadForConsumerDeepScanning( item, DownloadItemWarningData::DeepScanTrigger:: TRIGGER_IMMEDIATE_DEEP_SCAN, /*password=*/std::nullopt); - break; + // We return early because starting deep scanning immediately triggers + // this function with a `DownloadCheckResult` of `ASYNC_SCANNING`. Doing + // two updates would lead to two announced accessible alerts. See + // https://crbug.com/40926583. + return; } DCHECK_NE(danger_type, download::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT); @@ -1600,7 +1615,6 @@ void ChromeDownloadManagerDelegate::CheckSavePackageScanningDone( /*allowed*/ true); break; - case safe_browsing::DownloadCheckResult::BLOCKED_UNSUPPORTED_FILE_TYPE: case safe_browsing::DownloadCheckResult::BLOCKED_PASSWORD_PROTECTED: case safe_browsing::DownloadCheckResult::BLOCKED_TOO_LARGE: case safe_browsing::DownloadCheckResult::SENSITIVE_CONTENT_BLOCK: @@ -1732,10 +1746,6 @@ bool ChromeDownloadManagerDelegate::IsOpenInBrowserPreferredForFile( bool ChromeDownloadManagerDelegate::ShouldBlockFile( download::DownloadItem* item, download::DownloadDangerType danger_type) const { - // Don't block downloads if flag is set - if (base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads")) { - return false; - } // Chrome-initiated background downloads should not be blocked. if (item && !item->RequireSafetyChecks()) { return false; @@ -1747,12 +1757,6 @@ bool ChromeDownloadManagerDelegate::ShouldBlockFile( if (IsDangerTypeBlocked(danger_type)) return true; - // TODO(crbug/1061111): Move this into IsDangerTypeBlocked once the UX is - // ready. - if (danger_type == - download::DOWNLOAD_DANGER_TYPE_BLOCKED_UNSUPPORTED_FILETYPE) - return true; - bool file_type_dangerous = (item && DownloadItemModel(item).GetDangerLevel() != DownloadFileType::NOT_DANGEROUS); @@ -1813,6 +1817,39 @@ void ChromeDownloadManagerDelegate::MaybeSendDangerousDownloadOpenedReport( } } +void ChromeDownloadManagerDelegate::MaybeSendDangerousDownloadCanceledReport( + DownloadItem* download, + bool is_shutdown) { +#if BUILDFLAG(FULL_SAFE_BROWSING) + if (!DownloadProtectionService::ShouldSendDangerousDownloadReport(download) || + !base::FeatureList::IsEnabled( + safe_browsing::kDownloadReportWithoutUserDecision)) { + return; + } + safe_browsing::SafeBrowsingService* sb_service = + g_browser_process->safe_browsing_service(); + if (!sb_service) { + return; + } + // Note: We cannot go through download_protection_service here, because this + // function may be called at shutdown. The download_protection_service + // object may already be deleted at this point. + if (is_shutdown) { + sb_service->PersistDownloadReportAndSendOnNextStartup( + download, + safe_browsing::ClientSafeBrowsingReportRequest:: + DANGEROUS_DOWNLOAD_PROFILE_CLOSED, + /*did_proceed=*/false, std::nullopt); + } else { + sb_service->SendDownloadReport( + download, + safe_browsing::ClientSafeBrowsingReportRequest:: + DANGEROUS_DOWNLOAD_AUTO_DELETED, + /*did_proceed=*/false, std::nullopt); + } +#endif +} + void ChromeDownloadManagerDelegate::CheckDownloadAllowed( const content::WebContents::Getter& web_contents_getter, const GURL& url, @@ -1946,6 +1983,11 @@ bool ChromeDownloadManagerDelegate::IsFromExternalApp( return false; } + +bool ChromeDownloadManagerDelegate::ShouldOpenPdfInline() { + JNIEnv* env = base::android::AttachCurrentThread(); + return Java_PdfUtils_shouldOpenPdfInline(env); +} #endif // BUILDFLAG(IS_ANDROID) #if BUILDFLAG(FULL_SAFE_BROWSING) @@ -2013,6 +2055,7 @@ void ChromeDownloadManagerDelegate::CancelForEphemeralWarning( LogCancelEphemeralWarningEvent( CancelEphemeralWarningEvent::kCancellationSucceeded); download->Cancel(/*user_cancel=*/false); + MaybeSendDangerousDownloadCanceledReport(download, /*is_shutdown=*/false); } else { LogCancelEphemeralWarningEvent( CancelEphemeralWarningEvent::kCancellationFailedDownloadNotEphemeral); diff --git a/src/chrome/browser/download/download_target_determiner.cc b/src/chrome/browser/download/download_target_determiner.cc index 282ea89d..d758e76d 100644 --- a/src/chrome/browser/download/download_target_determiner.cc +++ b/src/chrome/browser/download/download_target_determiner.cc @@ -247,7 +247,6 @@ DownloadTargetDeterminer::Result return QUIT_DOLOOP; } - conflict_action_ = DownloadPathReservationTracker::OVERWRITE; DCHECK(virtual_path_.IsAbsolute()); return CONTINUE; } @@ -483,7 +482,7 @@ void DownloadTargetDeterminer::ReserveVirtualPathDone( << "Transient download should not ask the user for confirmation."; DCHECK(result != download::PathValidationResult::CONFLICT) << "Transient download" - "should always overwrite the file."; + "should always overwrite or uniquify the file."; switch (result) { case download::PathValidationResult::PATH_NOT_WRITABLE: case download::PathValidationResult::NAME_TOO_LONG: @@ -496,8 +495,8 @@ void DownloadTargetDeterminer::ReserveVirtualPathDone( case download::PathValidationResult::SUCCESS: case download::PathValidationResult::SUCCESS_RESOLVED_CONFLICT: case download::PathValidationResult::SAME_AS_SOURCE: - DCHECK_EQ(virtual_path_, path) << "Transient download path should not" - "be changed."; + DCHECK(virtual_path_ == path || + conflict_action_ == DownloadPathReservationTracker::UNIQUIFY); break; case download::PathValidationResult::COUNT: NOTREACHED(); @@ -510,7 +509,7 @@ void DownloadTargetDeterminer::ReserveVirtualPathDone( case download::PathValidationResult::SAME_AS_SOURCE: break; - // TODO(crbug.com/1361503): This should trigger a duplicate download + // TODO(crbug.com/40863725): This should trigger a duplicate download // prompt. case download::PathValidationResult::SUCCESS_RESOLVED_CONFLICT: break; @@ -960,11 +959,6 @@ DownloadTargetDeterminer::Result DCHECK_CURRENTLY_ON(BrowserThread::UI); next_state_ = STATE_DETERMINE_INTERMEDIATE_PATH; - // Allow all downloads with this Thorium flag - if (base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads")) { - return CONTINUE; - } - // Checking if there are prior visits to the referrer is only necessary if the // danger level of the download depends on the file type. if (danger_type_ != download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS && @@ -1271,6 +1265,13 @@ DownloadFileType::DangerLevel DownloadTargetDeterminer::GetDangerLevel( PriorVisitsToReferrer visits) const { DCHECK_CURRENTLY_ON(BrowserThread::UI); + // Allow all downloads with this Thorium flag + static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); + if (allow_insecure_downloads_) { + return DownloadFileType::NOT_DANGEROUS; + } + // If the user has has been prompted or will be, assume that the user has // approved the download. A programmatic download is considered safe unless it // contains malware. diff --git a/src/chrome/browser/download/insecure_download_blocking.cc b/src/chrome/browser/download/insecure_download_blocking.cc index 9050b17c..31b1cbf6 100644 --- a/src/chrome/browser/download/insecure_download_blocking.cc +++ b/src/chrome/browser/download/insecure_download_blocking.cc @@ -36,6 +36,9 @@ using InsecureDownloadStatus = download::DownloadItem::InsecureDownloadStatus; namespace { +static const bool allow_insecure_downloads_ = + base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads"); + // Configuration for which extensions to warn/block. These parameters are set // differently for testing, so the listed defaults are only used when the flag // is manually enabled (and in unit tests). @@ -278,9 +281,9 @@ struct InsecureDownloadData { // - anything extension related, // - etc. // - // TODO(1029062): INTERNAL_API is also used for background fetch. That - // probably isn't the correct behavior, since INTERNAL_API is otherwise used - // for Chrome stuff. Background fetch should probably be HTTPS-only. + // TODO(crbug.com/40661154): INTERNAL_API is also used for background fetch. + // That probably isn't the correct behavior, since INTERNAL_API is otherwise + // used for Chrome stuff. Background fetch should probably be HTTPS-only. auto download_source = item->GetDownloadSource(); auto transition_type = item->GetTransitionType(); if (download_source == DownloadSource::RETRY || @@ -334,10 +337,10 @@ struct InsecureDownloadData { download_source == DownloadSource::INTERNAL_API || download_source == DownloadSource::EXTENSION_API || download_source == DownloadSource::EXTENSION_INSTALLER || - base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads")) { + allow_insecure_downloads_) { is_insecure_download_ = false; } else { // Not ignorable download. - // TODO(crbug.com/1352598): Add blocking metrics. + // TODO(crbug.com/40857867): Add blocking metrics. // insecure downloads are either delivered insecurely, or we can't trust // who told us to download them (i.e. have an insecure initiator). is_insecure_download_ = @@ -413,7 +416,7 @@ void PrintConsoleMessage(const InsecureDownloadData& data) { bool IsDownloadPermittedByContentSettings( Profile* profile, const std::optional& initiator) { - // TODO(crbug.com/1048957): Checking content settings crashes unit tests on + // TODO(crbug.com/40117459): Checking content settings crashes unit tests on // Android. It shouldn't. #if !BUILDFLAG(IS_ANDROID) HostContentSettingsMap* host_content_settings_map = @@ -454,7 +457,12 @@ InsecureDownloadStatus GetInsecureDownloadStatusForDownload( InsecureDownloadData data(path, item); // If the download is fully secure, early abort. - if (!data.is_insecure_download_ || base::CommandLine::ForCurrentProcess()->HasSwitch("allow-insecure-downloads")) { + if (!data.is_insecure_download_) { + return InsecureDownloadStatus::SAFE; + } + + // Don't nag + if (allow_insecure_downloads_) { return InsecureDownloadStatus::SAFE; } diff --git a/src/chrome/browser/feed/android/java/res/layout/new_tab_page_feed_v2_expandable_header.xml b/src/chrome/browser/feed/android/java/res/layout/new_tab_page_feed_v2_expandable_header.xml index 0c0243bd..cd12a0aa 100644 --- a/src/chrome/browser/feed/android/java/res/layout/new_tab_page_feed_v2_expandable_header.xml +++ b/src/chrome/browser/feed/android/java/res/layout/new_tab_page_feed_v2_expandable_header.xml @@ -36,16 +36,17 @@ found in the LICENSE file. + app:tint="@color/header_title_text_color_list" /> diff --git a/src/chrome/browser/feed/android/java/res/layout/new_tab_page_multi_feed_header.xml b/src/chrome/browser/feed/android/java/res/layout/new_tab_page_multi_feed_header.xml index 036ed4a9..a7482c65 100644 --- a/src/chrome/browser/feed/android/java/res/layout/new_tab_page_multi_feed_header.xml +++ b/src/chrome/browser/feed/android/java/res/layout/new_tab_page_multi_feed_header.xml @@ -63,14 +63,15 @@ found in the LICENSE file. + app:tint="@color/header_title_text_color_list" /> diff --git a/src/chrome/browser/net/profile_network_context_service.cc b/src/chrome/browser/net/profile_network_context_service.cc index d94fb3e0..ffd8007a 100644 --- a/src/chrome/browser/net/profile_network_context_service.cc +++ b/src/chrome/browser/net/profile_network_context_service.cc @@ -5,6 +5,7 @@ #include "chrome/browser/net/profile_network_context_service.h" #include +#include #include "base/base64.h" #include "base/check_op.h" @@ -38,6 +39,8 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ssl/sct_reporting_service.h" #include "chrome/browser/ssl/sct_reporting_service_factory.h" +#include "chrome/browser/webid/federated_identity_permission_context.h" +#include "chrome/browser/webid/federated_identity_permission_context_factory.h" #include "chrome/common/buildflags.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_content_client.h" @@ -132,6 +135,13 @@ #include "chromeos/startup/browser_params_proxy.h" #endif +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) +#include "chrome/browser/enterprise/client_certificates/certificate_provisioning_service_factory.h" +#include "components/enterprise/client_certificates/core/certificate_provisioning_service.h" +#include "components/enterprise/client_certificates/core/client_certificates_service.h" +#include "components/enterprise/client_certificates/core/features.h" +#endif + namespace { bool* g_discard_domain_reliability_uploads_for_testing = nullptr; @@ -154,33 +164,6 @@ std::string ComputeAcceptLanguageFromPref(const std::string& language_pref) { return net::HttpUtil::GenerateAcceptLanguageHeader(accept_languages_str); } -#if BUILDFLAG(IS_CHROMEOS) -cert_verifier::mojom::AdditionalCertificatesPtr GetAdditionalCertificates( - const policy::PolicyCertService* policy_cert_service, - const base::FilePath& storage_partition_path) { - auto additional_certificates = - cert_verifier::mojom::AdditionalCertificates::New(); - net::CertificateList all_certificates; - net::CertificateList trust_anchors; - policy_cert_service->GetPolicyCertificatesForStoragePartition( - storage_partition_path, &all_certificates, &trust_anchors); - - for (const auto& cert : all_certificates) { - base::span cert_bytes = - net::x509_util::CryptoBufferAsSpan(cert->cert_buffer()); - additional_certificates->all_certificates.push_back( - std::vector(cert_bytes.begin(), cert_bytes.end())); - } - for (const auto& cert : trust_anchors) { - base::span cert_bytes = - net::x509_util::CryptoBufferAsSpan(cert->cert_buffer()); - additional_certificates->trust_anchors.push_back( - std::vector(cert_bytes.begin(), cert_bytes.end())); - } - return additional_certificates; -} -#endif // defined (OS_CHROMEOS) - // Tests allowing ambient authentication with default credentials based on the // profile type. bool IsAmbientAuthAllowedForProfile(Profile* profile) { @@ -246,9 +229,25 @@ void UpdateCookieSettings(Profile* profile, ContentSettingsType type) { if (!IsContentSettingsTypeEnabled(type)) { return; } - ContentSettingsForOneType settings = - HostContentSettingsMapFactory::GetForProfile(profile) - ->GetSettingsForOneType(type); + + ContentSettingsForOneType settings; + if (type == ContentSettingsType::FEDERATED_IDENTITY_SHARING) { + // Note: FederatedIdentityPermissionContext also syncs the permissions + // directly, in order to avoid a race condition. (Namely, + // FederatedIdentityPermissionContext must guarantee that the permissions + // have propagated before it calls its callback. However, the syncing that + // occurs in this class is unsynchronized, so it would be racy to rely on + // this update finishing before calling the context's callback.) This + // unfortunately triggers a double-update here. + if (FederatedIdentityPermissionContext* fedcm_context = + FederatedIdentityPermissionContextFactory::GetForProfile(profile); + fedcm_context) { + settings = fedcm_context->GetSharingPermissionGrantsAsContentSettings(); + } + } else { + settings = HostContentSettingsMapFactory::GetForProfile(profile) + ->GetSettingsForOneType(type); + } profile->ForEachLoadedStoragePartition( [&](content::StoragePartition* storage_partition) { storage_partition->GetCookieManagerForBrowserProcess() @@ -256,6 +255,27 @@ void UpdateCookieSettings(Profile* profile, ContentSettingsType type) { }); } +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) +std::unique_ptr GetWrappedCertStore( + Profile* profile, + std::unique_ptr platform_store) { + if (!profile || !client_certificates::features:: + IsManagedClientCertificateForUserEnabled()) { + return platform_store; + } + + auto* provisioning_service = + client_certificates::CertificateProvisioningServiceFactory::GetForProfile( + profile); + if (!provisioning_service) { + return platform_store; + } + + return client_certificates::ClientCertificatesService::Create( + provisioning_service, std::move(platform_store)); +} +#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) + } // namespace ProfileNetworkContextService::ProfileNetworkContextService(Profile* profile) @@ -276,8 +296,6 @@ ProfileNetworkContextService::ProfileNetworkContextService(Profile* profile) base::Unretained(this))); cookie_settings_ = CookieSettingsFactory::GetForProfile(profile); cookie_settings_observation_.Observe(cookie_settings_.get()); - privacy_sandbox_settings_observer_.Observe( - PrivacySandboxSettingsFactory::GetForProfile(profile)); DisableQuicIfNotAllowed(); @@ -314,9 +332,11 @@ ProfileNetworkContextService::ProfileNetworkContextService(Profile* profile) schedule_update_cert_policy); pref_change_registrar_.Add(prefs::kCAHintCertificates, schedule_update_cert_policy); +#if !BUILDFLAG(IS_CHROMEOS) pref_change_registrar_.Add(prefs::kCAPlatformIntegrationEnabled, schedule_update_cert_policy); #endif +#endif // BUILDFLAG(CHROME_CERTIFICATE_POLICIES_SUPPORTED) pref_change_registrar_.Add( prefs::kGloballyScopeHTTPAuthCacheEnabled, @@ -363,22 +383,6 @@ void ProfileNetworkContextService::ConfigureNetworkContextParams( } } -#if BUILDFLAG(IS_CHROMEOS) -void ProfileNetworkContextService::UpdateAdditionalCertificates() { - const policy::PolicyCertService* policy_cert_service = - policy::PolicyCertServiceFactory::GetForProfile(profile_); - if (!policy_cert_service) - return; - profile_->ForEachLoadedStoragePartition( - [&](content::StoragePartition* storage_partition) { - auto additional_certificates = GetAdditionalCertificates( - policy_cert_service, storage_partition->GetPath()); - storage_partition->GetCertVerifierServiceUpdater() - ->UpdateAdditionalCertificates(std::move(additional_certificates)); - }); -} -#endif - void ProfileNetworkContextService::RegisterProfilePrefs( user_prefs::PrefRegistrySyncable* registry) { registry->RegisterBooleanPref(embedder_support::kAlternateErrorPagesEnabled, @@ -392,9 +396,11 @@ void ProfileNetworkContextService::RegisterProfilePrefs( registry->RegisterListPref(prefs::kCACertificatesWithConstraints); registry->RegisterListPref(prefs::kCADistrustedCertificates); registry->RegisterListPref(prefs::kCAHintCertificates); +#if !BUILDFLAG(IS_CHROMEOS) // Include user added platform certs by default. registry->RegisterBooleanPref(prefs::kCAPlatformIntegrationEnabled, true); #endif +#endif // BUILDFLAG(CHROME_CERTIFICATE_POLICIES_SUPPORTED) } // static @@ -470,14 +476,6 @@ void ProfileNetworkContextService::OnTruncatedCookieBlockingChanged() { }); } -void ProfileNetworkContextService::OnFirstPartySetsEnabledChanged( - bool enabled) { - // Update all FPS Access Delegates on the FPS service to be `enabled`. - first_party_sets::FirstPartySetsPolicyServiceFactory::GetForBrowserContext( - profile_) - ->OnFirstPartySetsEnabledChanged(enabled); -} - std::string ProfileNetworkContextService::ComputeAcceptLanguage() const { // If reduce accept language is enabled, only return the first language // without expanding the language list. @@ -546,11 +544,36 @@ void ProfileNetworkContextService::ScheduleUpdateCTPolicy() { #if BUILDFLAG(CHROME_CERTIFICATE_POLICIES_SUPPORTED) cert_verifier::mojom::AdditionalCertificatesPtr -ProfileNetworkContextService::GetCertificatePolicy() { +ProfileNetworkContextService::GetCertificatePolicy( + const base::FilePath& storage_partition_path) { auto* prefs = profile_->GetPrefs(); auto additional_certificates = cert_verifier::mojom::AdditionalCertificates::New(); +#if BUILDFLAG(IS_CHROMEOS) + const policy::PolicyCertService* policy_cert_service = + policy::PolicyCertServiceFactory::GetForProfile(profile_); + if (policy_cert_service) { + net::CertificateList all_certificates; + net::CertificateList trust_anchors; + policy_cert_service->GetPolicyCertificatesForStoragePartition( + storage_partition_path, &all_certificates, &trust_anchors); + + for (const auto& cert : all_certificates) { + base::span cert_bytes = + net::x509_util::CryptoBufferAsSpan(cert->cert_buffer()); + additional_certificates->all_certificates.push_back( + std::vector(cert_bytes.begin(), cert_bytes.end())); + } + for (const auto& cert : trust_anchors) { + base::span cert_bytes = + net::x509_util::CryptoBufferAsSpan(cert->cert_buffer()); + additional_certificates->trust_anchors.push_back( + std::vector(cert_bytes.begin(), cert_bytes.end())); + } + } +#endif // BUILDFLAG(IS_CHROMEOS) + for (const base::Value& cert_b64 : prefs->GetList(prefs::kCAHintCertificates)) { std::optional> decoded_opt = @@ -680,7 +703,7 @@ ProfileNetworkContextService::GetCertificatePolicy() { if (!base::Base64Decode(cert_b64.GetString(), &decoded)) { continue; } - base::StringPiece spki_piece; + std::string_view spki_piece; bool success = net::asn1::ExtractSPKIFromDERCert(decoded, &spki_piece); if (success) { additional_certificates->distrusted_spkis.emplace_back(spki_piece.begin(), @@ -688,30 +711,29 @@ ProfileNetworkContextService::GetCertificatePolicy() { } } +#if !BUILDFLAG(IS_CHROMEOS) additional_certificates->include_system_trust_store = prefs->GetBoolean(prefs::kCAPlatformIntegrationEnabled); +#endif return additional_certificates; } -void ProfileNetworkContextService::UpdateCertificatePolicy() { - std::vector updaters; +void ProfileNetworkContextService::UpdateAdditionalCertificates() { profile_->ForEachLoadedStoragePartition( [&](content::StoragePartition* storage_partition) { - updaters.push_back(storage_partition->GetCertVerifierServiceUpdater()); + storage_partition->GetCertVerifierServiceUpdater() + ->UpdateAdditionalCertificates( + GetCertificatePolicy(storage_partition->GetPath())); }); - - for (auto* updater : updaters) { - updater->UpdateAdditionalCertificates(GetCertificatePolicy()); - } } void ProfileNetworkContextService::ScheduleUpdateCertificatePolicy() { cert_policy_update_timer_.Start( FROM_HERE, base::Seconds(0), this, - &ProfileNetworkContextService::UpdateCertificatePolicy); + &ProfileNetworkContextService::UpdateAdditionalCertificates); } -#endif +#endif // BUILDFLAG(CHROME_CERTIFICATE_POLICIES_SUPPORTED) bool ProfileNetworkContextService::ShouldSplitAuthCacheByNetworkIsolationKey() const { @@ -778,8 +800,19 @@ ProfileNetworkContextService::CreateCookieManagerParams( if (!IsContentSettingsTypeEnabled(type)) { continue; } - out->content_settings[type] = - host_content_settings_map->GetSettingsForOneType(type); + if (type == ContentSettingsType::FEDERATED_IDENTITY_SHARING) { + if (FederatedIdentityPermissionContext* fedcm_context = + FederatedIdentityPermissionContextFactory::GetForProfile(profile); + fedcm_context) { + out->content_settings[type] = + fedcm_context->GetSharingPermissionGrantsAsContentSettings(); + } else { + out->content_settings[type] = ContentSettingsForOneType(); + } + } else { + out->content_settings[type] = + host_content_settings_map->GetSettingsForOneType(type); + } } out->cookie_access_delegate_type = @@ -792,7 +825,8 @@ ProfileNetworkContextService::CreateCookieManagerParams( cookie_settings.MitigationsEnabledFor3pcd(); out->tracking_protection_enabled_for_3pcd = - cookie_settings.TrackingProtectionEnabledFor3pcd(); + TrackingProtectionSettingsFactory::GetForProfile(profile) + ->IsTrackingProtection3pcdEnabled(); return out; } @@ -807,6 +841,15 @@ void ProfileNetworkContextService::FlushCachedClientCertIfNeeded( }); } +void ProfileNetworkContextService::FlushMatchingCachedClientCert( + const scoped_refptr& certificate) { + profile_->ForEachLoadedStoragePartition( + [&](content::StoragePartition* storage_partition) { + storage_partition->GetNetworkContext()->FlushMatchingCachedClientCert( + certificate); + }); +} + void ProfileNetworkContextService::FlushProxyConfigMonitorForTesting() { proxy_config_monitor_.FlushForTesting(); } @@ -871,7 +914,7 @@ ProfileNetworkContextService::CreateClientCertStore() { if (!Profile::FromBrowserContext( chrome::GetBrowserContextRedirectedInIncognito(profile_)) ->IsMainProfile()) { - // TODO(crbug.com/1148298): At the moment client certs are only enabled for + // TODO(crbug.com/40156976): At the moment client certs are only enabled for // the main profile and its incognito profile (similarly to how it worked in // Ash-Chrome). Return some cert store for secondary profiles in // Lacros-Chrome when certs are supported there. @@ -886,18 +929,16 @@ ProfileNetworkContextService::CreateClientCertStore() { return store; #elif BUILDFLAG(IS_WIN) - return std::make_unique(); + return GetWrappedCertStore(profile_, + std::make_unique()); #elif BUILDFLAG(IS_MAC) - return std::make_unique(); + return GetWrappedCertStore(profile_, + std::make_unique()); #elif BUILDFLAG(IS_ANDROID) // Android does not use the ClientCertStore infrastructure. On Android client // cert matching is done by the OS as part of the call to show the cert // selection dialog. return nullptr; -#elif BUILDFLAG(IS_FUCHSIA) - // TODO(crbug.com/1380609): Implement ClientCertStore support. - NOTIMPLEMENTED_LOG_ONCE(); - return nullptr; #else #error Unknown platform. #endif @@ -950,21 +991,6 @@ bool GetHttpCacheBackendResetParam(PrefService* local_state) { current_field_trial_status != previous_field_trial_status; } -#if BUILDFLAG(IS_CHROMEOS) -void ProfileNetworkContextService::PopulateInitialAdditionalCerts( - const base::FilePath& relative_partition_path, - cert_verifier::mojom::CertVerifierCreationParams* creation_params) { - if (policy::PolicyCertServiceFactory::CreateAndStartObservingForProfile( - profile_)) { - const policy::PolicyCertService* policy_cert_service = - policy::PolicyCertServiceFactory::GetForProfile(profile_); - creation_params->initial_additional_certificates = - GetAdditionalCertificates(policy_cert_service, - GetPartitionPath(relative_partition_path)); - } -} -#endif // BUILDFLAG(IS_CHROMEOS) - void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( bool in_memory, const base::FilePath& relative_partition_path, @@ -1052,6 +1078,7 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( // network service. network_context_params->enable_locking_cookie_database = base::FeatureList::IsEnabled(features::kLockProfileCookieDatabase); +#endif // BUILDFLAG(IS_WIN) if (base::FeatureList::IsEnabled( features::kUseOsCryptAsyncForCookieEncryption)) { @@ -1060,7 +1087,6 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( network_context_params); } -#endif // BUILDFLAG(IS_WIN) network_context_params->file_paths->trust_token_database_name = base::FilePath(chrome::kTrustTokenFilename); @@ -1136,8 +1162,7 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( } } - PopulateInitialAdditionalCerts(relative_partition_path, - cert_verifier_creation_params); + policy::PolicyCertServiceFactory::CreateAndStartObservingForProfile(profile_); #endif #if BUILDFLAG(IS_CHROMEOS_ASH) @@ -1161,20 +1186,17 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( } } if (profile_supports_policy_certs) { - PopulateInitialAdditionalCerts(relative_partition_path, - cert_verifier_creation_params); + policy::PolicyCertServiceFactory::CreateAndStartObservingForProfile( + profile_); } #endif #if BUILDFLAG(CHROME_CERTIFICATE_POLICIES_SUPPORTED) - // TODO(crbug.com/1477317): check to see if IsManaged() ensures the pref isn't - // set in user profiles, or if that does something else. If that's true, add - // an isManaged() check here. - // TODO(crbug.com/1477317): when adding ChromeOS support for these policies - // figure out how to integrate in the ChromeOS enterprise policy support with - // these policies. + // TODO(crbug.com/40928765): check to see if IsManaged() ensures the pref + // isn't set in user profiles, or if that does something else. If that's true, + // add an isManaged() check here. cert_verifier_creation_params->initial_additional_certificates = - GetCertificatePolicy(); + GetCertificatePolicy(GetPartitionPath(relative_partition_path)); #endif #if BUILDFLAG(IS_CHROMEOS) @@ -1236,6 +1258,9 @@ void ProfileNetworkContextService::ConfigureNetworkContextParamsInternal( network_context_params->enable_ip_protection = ipp_config_provider->IsIpProtectionEnabled(); } + + network_context_params->device_bound_sessions_enabled = + base::FeatureList::IsEnabled(net::features::kDeviceBoundSessions); } base::FilePath ProfileNetworkContextService::GetPartitionPath( diff --git a/src/chrome/browser/ui/tabs/tab_strip_model.cc b/src/chrome/browser/ui/tabs/tab_strip_model.cc index 78556b92..9fef79ee 100644 --- a/src/chrome/browser/ui/tabs/tab_strip_model.cc +++ b/src/chrome/browser/ui/tabs/tab_strip_model.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -26,6 +27,7 @@ #include "base/trace_event/trace_event.h" #include "build/build_config.h" #include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/commerce/browser_utils.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/extensions/tab_helper.h" #include "chrome/browser/lifetime/browser_shutdown.h" @@ -44,15 +46,17 @@ #include "chrome/browser/ui/tabs/organization/tab_organization_service.h" #include "chrome/browser/ui/tabs/organization/tab_organization_service_factory.h" #include "chrome/browser/ui/tabs/organization/tab_organization_session.h" -#include "chrome/browser/ui/tabs/saved_tab_groups/saved_tab_group_keyed_service.h" -#include "chrome/browser/ui/tabs/saved_tab_groups/saved_tab_group_service_factory.h" +#include "chrome/browser/ui/tabs/pinned_tab_collection.h" +#include "chrome/browser/ui/tabs/tab_collection.h" #include "chrome/browser/ui/tabs/tab_enums.h" #include "chrome/browser/ui/tabs/tab_group.h" #include "chrome/browser/ui/tabs/tab_group_model.h" #include "chrome/browser/ui/tabs/tab_model.h" +#include "chrome/browser/ui/tabs/tab_strip_collection.h" #include "chrome/browser/ui/tabs/tab_strip_model_delegate.h" #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" #include "chrome/browser/ui/tabs/tab_utils.h" +#include "chrome/browser/ui/tabs/unpinned_tab_collection.h" #include "chrome/browser/ui/thumbnails/thumbnail_tab_helper.h" #include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/user_notes/user_notes_controller.h" @@ -63,10 +67,10 @@ #include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_tab_helper.h" #include "chrome/common/webui_url_constants.h" +#include "components/commerce/core/commerce_utils.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "components/feature_engagement/public/feature_constants.h" #include "components/reading_list/core/reading_list_model.h" -#include "components/saved_tab_groups/saved_tab_group_model.h" #include "components/tab_groups/tab_group_id.h" #include "components/tab_groups/tab_group_visual_data.h" #include "components/web_modal/web_contents_modal_dialog_manager.h" @@ -97,8 +101,8 @@ class RenderWidgetHostVisibilityTracker; class ReentrancyCheck { public: explicit ReentrancyCheck(bool* guard_flag) : guard_flag_(guard_flag) { - CHECK_CURRENTLY_ON(content::BrowserThread::UI, base::NotFatalUntil::M126); - CHECK(!*guard_flag_, base::NotFatalUntil::M126); + CHECK_CURRENTLY_ON(content::BrowserThread::UI, base::NotFatalUntil::M130); + CHECK(!*guard_flag_, base::NotFatalUntil::M130); *guard_flag_ = true; } @@ -188,6 +192,16 @@ class RenderWidgetHostVisibilityTracker final base::ElapsedTimer timer_; }; +tabs::TabInterface::DetachReason RemoveReasonToDetachReason( + TabStripModelChange::RemoveReason reason) { + switch (reason) { + case TabStripModelChange::RemoveReason::kDeleted: + return tabs::TabInterface::DetachReason::kDelete; + case TabStripModelChange::RemoveReason::kInsertedIntoOtherTabStrip: + return tabs::TabInterface::DetachReason::kInsertIntoOtherWindow; + } +} + } // namespace TabGroupModelFactory::TabGroupModelFactory() { @@ -263,16 +277,21 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, : delegate_(delegate), profile_(profile) { DCHECK(delegate_); + if (base::FeatureList::IsEnabled(features::kTabStripCollectionStorage)) { + contents_data_ = std::make_unique(); + } else { + contents_data_ = std::vector>(); + } + if (group_model_factory) group_model_ = group_model_factory->Create(this); scrubbing_metrics_.Init(); } TabStripModel::~TabStripModel() { - for (auto& observer : observers_) + for (auto& observer : observers_) { observer.ModelDestroyed(TabStripModelObserver::ModelPasskey(), this); - - contents_data_.clear(); + } } void TabStripModel::SetTabStripUI(TabStripModelObserver* observer) { @@ -301,29 +320,81 @@ void TabStripModel::RemoveObserver(TabStripModelObserver* observer) { observers_.RemoveObserver(observer); } +int TabStripModel::count() const { + return IsContentsDataVector() + ? static_cast(GetContentsDataAsVector().size()) + : static_cast( + GetContentsDataAsCollection()->TabCountRecursive()); +} + +bool TabStripModel::empty() const { + return IsContentsDataVector() + ? GetContentsDataAsVector().empty() + : (GetContentsDataAsCollection()->TabCountRecursive() == 0); +} + int TabStripModel::GetIndexOfTab(tabs::TabHandle tab_handle) const { const tabs::TabModel* tab_model = tab_handle.Get(); if (tab_model == nullptr) { return kNoTab; } + if (!IsContentsDataVector()) { + std::optional index_of_tab = + GetContentsDataAsCollection()->GetIndexOfTabRecursive(tab_model); + return index_of_tab.value_or(kNoTab); + } + const auto is_same_tab = [tab_model](const std::unique_ptr& other) { return other.get() == tab_model; }; - const auto iter = - std::find_if(contents_data_.cbegin(), contents_data_.cend(), is_same_tab); - if (iter == contents_data_.cend()) { + const auto iter = std::find_if(GetContentsDataAsVector().cbegin(), + GetContentsDataAsVector().cend(), is_same_tab); + if (iter == GetContentsDataAsVector().cend()) { return kNoTab; } - return iter - contents_data_.begin(); + return iter - GetContentsDataAsVector().begin(); } tabs::TabHandle TabStripModel::GetTabHandleAt(int index) const { CHECK(ContainsIndex(index)); - return contents_data_[index]->GetHandle(); + return GetTabAtIndex(index)->GetHandle(); +} + +const std::vector>& +TabStripModel::GetContentsDataAsVector() const { + CHECK(IsContentsDataVector()); + const std::vector>* contents_data_ptr = + std::get_if>>( + &contents_data_); + return *contents_data_ptr; +} + +const tabs::TabStripCollection* TabStripModel::GetContentsDataAsCollection() + const { + CHECK(!IsContentsDataVector()); + const std::unique_ptr* tab_strip_collection_ptr = + std::get_if>(&contents_data_); + return tab_strip_collection_ptr->get(); +} + +std::vector>& +TabStripModel::GetContentsDataAsVector() { + CHECK(IsContentsDataVector()); + std::vector>* contents_data_ptr = + std::get_if>>( + &contents_data_); + return *contents_data_ptr; +} + +tabs::TabStripCollection* TabStripModel::GetContentsDataAsCollection() { + CHECK(!IsContentsDataVector()); + std::unique_ptr* tab_strip_collection_ptr = + std::get_if>(&contents_data_); + return tab_strip_collection_ptr->get(); } bool TabStripModel::ContainsIndex(int index) const { @@ -365,7 +436,7 @@ int TabStripModel::InsertDetachedTabAt( return InsertTabAtImpl(index, std::move(tab), add_types, group); } -std::unique_ptr TabStripModel::ReplaceWebContentsAt( +std::unique_ptr TabStripModel::DiscardWebContentsAt( int index, std::unique_ptr new_contents) { ReentrancyCheck reentrancy_check(&reentrancy_guard_); @@ -379,7 +450,7 @@ std::unique_ptr TabStripModel::ReplaceWebContentsAt( TabStripSelectionChange selection(GetActiveWebContents(), selection_model_); WebContents* raw_new_contents = new_contents.get(); std::unique_ptr old_contents = - contents_data_[index]->ReplaceContents(std::move(new_contents)); + GetTabAtIndex(index)->DiscardContents(std::move(new_contents)); // When the active WebContents is replaced send out a selection notification // too. We do this as nearly all observers need to treat a replacement of the @@ -406,13 +477,6 @@ std::unique_ptr TabStripModel::DetachTabAtForInsertion( return std::move(dwc->tab); } -std::unique_ptr -TabStripModel::DetachWebContentsAtForInsertion(int index) { - auto dwc = DetachWebContentsWithReasonAt( - index, TabStripModelChange::RemoveReason::kInsertedIntoOtherTabStrip); - return dwc->tab->ReplaceContents(nullptr); -} - void TabStripModel::DetachAndDeleteWebContentsAt(int index) { // Drops the returned unique pointer. DetachWebContentsWithReasonAt(index, @@ -430,6 +494,12 @@ TabStripModel::DetachWebContentsWithReasonAt( "trying to detach web contents."; WebContents* initially_active_web_contents = GetWebContentsAtImpl(active_index()); + if (index == active_index() && !closing_all_) { + GetTabAtIndex(active_index()) + ->WillEnterBackground(base::PassKey()); + } + GetTabAtIndex(index)->WillDetach(base::PassKey(), + RemoveReasonToDetachReason(reason)); DetachNotifications notifications(initially_active_web_contents, selection_model_); @@ -454,13 +524,14 @@ std::unique_ptr TabStripModel::DetachWebContentsImpl( int index_at_time_of_removal, bool create_historical_tab, TabStripModelChange::RemoveReason reason) { - if (contents_data_.empty()) + if (empty()) { return nullptr; + } CHECK(ContainsIndex(index_at_time_of_removal)); for (auto& observer : observers_) { observer.OnTabWillBeRemoved( - contents_data_[index_at_time_of_removal]->contents(), + GetTabAtIndex(index_at_time_of_removal)->contents(), index_at_time_of_removal); } @@ -477,11 +548,32 @@ std::unique_ptr TabStripModel::DetachWebContentsImpl( std::optional next_selected_index = DetermineNewSelectedIndex(index_at_time_of_removal); - UngroupTab(index_at_time_of_removal); + const std::optional old_group = + GetTabGroupForTab(index_at_time_of_removal); + UngroupTab(index_at_time_of_removal, old_group); - std::unique_ptr old_data = - std::move(contents_data_[index_at_time_of_removal]); - contents_data_.erase(contents_data_.begin() + index_at_time_of_removal); + std::unique_ptr old_data; + if (IsContentsDataVector()) { + old_data = std::move(GetContentsDataAsVector()[index_at_time_of_removal]); + GetContentsDataAsVector().erase(GetContentsDataAsVector().begin() + + index_at_time_of_removal); + } else { + old_data = GetContentsDataAsCollection()->RemoveTabAtIndexRecursive( + index_at_time_of_removal); + // It is possible that on tab removal the group collection is left without + // any child and in that case we should remove it. + if (old_group.has_value()) { + tabs::TabGroupTabCollection* group_collection = + GetContentsDataAsCollection() + ->GetUnpinnedCollection() + ->GetTabGroupCollection(old_group.value()); + + if (group_collection && group_collection->TabCountRecursive() == 0) { + GetContentsDataAsCollection()->GetUnpinnedCollection()->CloseTabGroup( + group_collection); + } + } + } if (empty()) { selection_model_.Clear(); @@ -597,11 +689,14 @@ int TabStripModel::MoveWebContentsAt(int index, to_position = ConstrainMoveIndex(to_position, IsTabPinned(index)); - if (index == to_position) + if (index == to_position) { return to_position; + } MoveWebContentsAtImpl(index, to_position, select_after_move); - EnsureGroupContiguity(to_position); + if (IsContentsDataVector()) { + EnsureGroupContiguity(to_position); + } return to_position; } @@ -682,7 +777,7 @@ WebContents* TabStripModel::GetActiveWebContents() const { tabs::TabModel* TabStripModel::GetActiveTab() const { int index = active_index(); if (ContainsIndex(index)) { - return contents_data_[index].get(); + return GetTabAtIndex(index); } return nullptr; } @@ -694,8 +789,8 @@ WebContents* TabStripModel::GetWebContentsAt(int index) const { } int TabStripModel::GetIndexOfWebContents(const WebContents* contents) const { - for (size_t i = 0; i < contents_data_.size(); ++i) { - if (contents_data_[i]->contents() == contents) { + for (int i = 0; i < GetTabCount(); ++i) { + if (GetTabAtIndex(i)->contents() == contents) { return i; } } @@ -738,7 +833,7 @@ void TabStripModel::CloseAllTabsInGroup(const tab_groups::TabGroupId& group) { if (!group_model_) return; - delegate_->CreateHistoricalGroup(group); + delegate_->WillCloseGroup(group); gfx::Range tabs_in_group = group_model_->GetTabGroup(group)->ListTabs(); if (static_cast(tabs_in_group.length()) == count()) @@ -758,7 +853,8 @@ void TabStripModel::CloseWebContentsAt(int index, uint32_t close_types) { } bool TabStripModel::TabsAreLoading() const { - for (const auto& data : contents_data_) { + for (int i = 0; i < GetTabCount(); i++) { + const tabs::TabModel* const data = GetTabAtIndex(i); if (data->contents()->IsLoading()) { return true; } @@ -769,7 +865,7 @@ bool TabStripModel::TabsAreLoading() const { WebContents* TabStripModel::GetOpenerOfWebContentsAt(const int index) const { CHECK(ContainsIndex(index)); - return contents_data_[index]->opener(); + return GetTabAtIndex(index)->opener(); } void TabStripModel::SetOpenerOfWebContentsAt(int index, WebContents* opener) { @@ -779,7 +875,7 @@ void TabStripModel::SetOpenerOfWebContentsAt(int index, WebContents* opener) { // the opener being used after its freed. See crbug.com/698681. DCHECK(!opener || GetIndexOfWebContents(opener) != kNoTab) << "Cannot set opener to a web contents not owned by this tab strip."; - contents_data_[index]->set_opener(opener); + GetTabAtIndex(index)->set_opener(opener); } int TabStripModel::GetIndexOfLastWebContentsOpenedBy(const WebContents* opener, @@ -794,13 +890,14 @@ int TabStripModel::GetIndexOfLastWebContentsOpenedBy(const WebContents* opener, for (int i = start_index + 1; i < count(); ++i) { // Test opened by transitively, i.e. include tabs opened by tabs opened by // opener, etc. Stop when we find the first non-descendant. - if (!opener_and_descendants.count(contents_data_[i]->opener())) { + if (!opener_and_descendants.count(GetTabAtIndex(i)->opener())) { // Skip over pinned tabs as new tabs are added after pinned tabs. - if (contents_data_[i]->pinned()) + if (GetTabAtIndex(i)->pinned()) { continue; + } break; } - opener_and_descendants.insert(contents_data_[i]->contents()); + opener_and_descendants.insert(GetTabAtIndex(i)->contents()); last_index = i; } return last_index; @@ -827,11 +924,14 @@ void TabStripModel::TabNavigating(WebContents* contents, void TabStripModel::SetTabBlocked(int index, bool blocked) { CHECK(ContainsIndex(index)); - if (contents_data_[index]->blocked() == blocked) + tabs::TabModel* tab_model = GetTabAtIndex(index); + if (tab_model->blocked() == blocked) { return; - contents_data_[index]->set_blocked(blocked); - for (auto& observer : observers_) - observer.TabBlockedStateChanged(contents_data_[index]->contents(), index); + } + tab_model->set_blocked(blocked); + for (auto& observer : observers_) { + observer.TabBlockedStateChanged(tab_model->contents(), index); + } } int TabStripModel::SetTabPinned(int index, bool pinned) { @@ -842,7 +942,7 @@ int TabStripModel::SetTabPinned(int index, bool pinned) { bool TabStripModel::IsTabPinned(int index) const { CHECK(ContainsIndex(index)) << index; - return contents_data_[index]->pinned(); + return GetTabAtIndex(index)->pinned(); } bool TabStripModel::IsTabCollapsed(int index) const { @@ -860,7 +960,7 @@ bool TabStripModel::IsGroupCollapsed( bool TabStripModel::IsTabBlocked(int index) const { CHECK(ContainsIndex(index)) << index; - return contents_data_[index]->blocked(); + return GetTabAtIndex(index)->blocked(); } bool TabStripModel::IsTabClosable(int index) const { @@ -873,7 +973,7 @@ bool TabStripModel::IsTabClosable(const content::WebContents* contents) const { std::optional TabStripModel::GetTabGroupForTab( int index) const { - return ContainsIndex(index) ? contents_data_[index]->group() : std::nullopt; + return ContainsIndex(index) ? GetTabAtIndex(index)->group() : std::nullopt; } std::optional TabStripModel::GetSurroundingTabGroup( @@ -895,9 +995,10 @@ std::optional TabStripModel::GetSurroundingTabGroup( } int TabStripModel::IndexOfFirstNonPinnedTab() const { - for (size_t i = 0; i < contents_data_.size(); ++i) { - if (!IsTabPinned(static_cast(i))) - return static_cast(i); + for (int i = 0; i < GetTabCount(); ++i) { + if (!IsTabPinned(i)) { + return i; + } } // No pinned tabs. return count(); @@ -966,6 +1067,14 @@ const ui::ListSelectionModel& TabStripModel::selection_model() const { return selection_model_; } +bool TabStripModel::CanShowModalUI() const { + return !showing_modal_ui_; +} + +std::unique_ptr TabStripModel::ShowModalUI() { + return std::make_unique(this); +} + void TabStripModel::AddWebContents( std::unique_ptr contents, int index, @@ -1051,7 +1160,7 @@ void TabStripModel::AddWebContents( // of openers. A jump would be too confusing at that point. if (inherit_opener && ui::PageTransitionTypeIncludingQualifiersIs( transition, ui::PAGE_TRANSITION_TYPED)) - contents_data_[index]->set_reset_opener_on_active_tab_change(true); + GetTabAtIndex(index)->set_reset_opener_on_active_tab_change(true); // TODO(sky): figure out why this is here and not in InsertWebContentsAt. When // here we seem to get failures in startup perf tests. @@ -1118,11 +1227,15 @@ tab_groups::TabGroupId TabStripModel::AddToNewGroup( const tab_groups::TabGroupId new_group = tab_groups::TabGroupId::GenerateNew(); AddToNewGroupImpl(indices, new_group); + // TODO(crbug.com/339858272) : Consolidate all default save logic to + // TabStripModel::AddToNewGroupImpl. + delegate_->GroupAdded(new_group); return new_group; } void TabStripModel::AddToExistingGroup(const std::vector& indices, - const tab_groups::TabGroupId& group) { + const tab_groups::TabGroupId& group, + const bool add_to_end) { ReentrancyCheck reentrancy_check(&reentrancy_guard_); CHECK(SupportsTabGroups()); @@ -1133,7 +1246,7 @@ void TabStripModel::AddToExistingGroup(const std::vector& indices, CHECK(ContainsIndex(*(indices.begin()))); CHECK(ContainsIndex(*(indices.rbegin()))); - AddToExistingGroupImpl(indices, group); + AddToExistingGroupImpl(indices, group, add_to_end); } void TabStripModel::MoveTabsAndSetGroup( @@ -1168,20 +1281,27 @@ void TabStripModel::UpdateGroupForDragRevert( std::optional group_data) { ReentrancyCheck reentrancy_check(&reentrancy_guard_); - if (!group_model_) + if (!group_model_) { return; + } if (group_id.has_value()) { const bool group_exists = group_model_->ContainsTabGroup(group_id.value()); - if (!group_exists) + if (!group_exists) { group_model_->AddTabGroup(group_id.value(), group_data); - GroupTab(index, group_id.value()); + } + GroupTab(index, group_id.value(), GetTabGroupForTab(index)); + if (!group_exists) { + // TODO(crbug.com/339858272) : Consolidate all default save logic to + // TabStripModel::AddToNewGroupImpl. + delegate_->GroupAdded(group_id.value()); + } } else { - UngroupTab(index); + UngroupTab(index, GetTabGroupForTab(index)); } } -void TabStripModel::RemoveFromGroup(const std::vector& indices) { +void TabStripModel::RemoveFromGroup(const std::vector indices) { ReentrancyCheck reentrancy_check(&reentrancy_guard_); if (!group_model_) @@ -1217,9 +1337,32 @@ void TabStripModel::RemoveFromGroup(const std::vector& indices) { right_of_group.push_back(index); } } - MoveTabsAndSetGroupImpl(left_of_group, first_tab_in_group, std::nullopt); - MoveTabsAndSetGroupImpl(right_of_group, last_tab_in_group + 1, - std::nullopt); + + if (IsContentsDataVector()) { + MoveTabsAndSetGroupImpl(left_of_group, first_tab_in_group, std::nullopt); + MoveTabsAndSetGroupImpl(right_of_group, last_tab_in_group + 1, + std::nullopt); + } else { + tabs::TabGroupTabCollection* group_collection = + GetContentsDataAsCollection() + ->GetUnpinnedCollection() + ->GetTabGroupCollection(kv.first); + + std::vector tabs_left_of_group; + std::vector tabs_right_of_group; + for (int index : left_of_group) { + tabs_left_of_group.push_back(GetTabAtIndex(index)); + } + + for (int index : right_of_group) { + tabs_right_of_group.push_back(GetTabAtIndex(index)); + } + + std::reverse(tabs_right_of_group.begin(), tabs_right_of_group.end()); + RemoveTabsFromGroupCollection(tabs_left_of_group, group_collection, true); + RemoveTabsFromGroupCollection(tabs_right_of_group, group_collection, + false); + } } } @@ -1307,20 +1450,11 @@ std::u16string TabStripModel::GetTitleAt(int index) const { return TabUIHelper::FromWebContents(GetWebContentsAt(index))->GetTitle(); } -void TabStripModel::FollowSites(const std::vector& indices) { - ReentrancyCheck reentrancy_check(&reentrancy_guard_); - for (int index : indices) - delegate_->FollowSite(GetWebContentsAt(index)); -} - -void TabStripModel::UnfollowSites(const std::vector& indices) { - ReentrancyCheck reentrancy_check(&reentrancy_guard_); - for (int index : indices) - delegate_->UnfollowSite(GetWebContentsAt(index)); -} - int TabStripModel::GetTabCount() const { - return static_cast(contents_data_.size()); + return IsContentsDataVector() + ? static_cast(GetContentsDataAsVector().size()) + : static_cast( + GetContentsDataAsCollection()->TabCountRecursive()); } // Context menu functions. @@ -1404,15 +1538,13 @@ bool TabStripModel::IsContextMenuCommandEnabled( case CommandOrganizeTabs: return true; - case CommandFollowSite: - case CommandUnfollowSite: { - std::vector indices = GetIndicesForCommand(context_index); - // Since all tabs should belong to same profile, it is enough to do the - // check based on the first tab. - content::WebContents* web_contents = GetWebContentsAt(indices[0]); - Profile* profile = - Profile::FromBrowserContext(web_contents->GetBrowserContext()); - return !profile->IsIncognitoProfile(); + case CommandCommerceProductSpecifications: { + auto selected_web_contents = + GetWebContentsesByIndices(GetIndicesForCommand(context_index)); + return commerce::IsProductSpecsMultiSelectMenuEnabled( + profile_, GetWebContentsAt(context_index)) && + commerce::IsWebContentsListEligibleForProductSpecs( + selected_web_contents); } case CommandCopyURL: @@ -1484,37 +1616,25 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index, ReentrancyCheck reentrancy_check(&reentrancy_guard_); base::RecordAction(UserMetricsAction("TabContextMenu_CloseTab")); - CloseTabs(GetWebContentsesByIndices(GetIndicesForCommand(context_index)), - TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB | - TabCloseTypes::CLOSE_USER_GESTURE); + ExecuteCloseTabsByIndicesCommand(GetIndicesForCommand(context_index)); break; } case CommandCloseOtherTabs: { ReentrancyCheck reentrancy_check(&reentrancy_guard_); - const std::vector indices = - GetIndicesClosedByCommand(context_index, command_id); - - DisconnectSavedTabGroups(indices); - base::RecordAction(UserMetricsAction("TabContextMenu_CloseOtherTabs")); - CloseTabs(GetWebContentsesByIndices(indices), - TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB); + ExecuteCloseTabsByIndicesCommand( + GetIndicesClosedByCommand(context_index, command_id)); break; } case CommandCloseTabsToRight: { ReentrancyCheck reentrancy_check(&reentrancy_guard_); - const std::vector indices = - GetIndicesClosedByCommand(context_index, command_id); - - DisconnectSavedTabGroups(indices); - base::RecordAction(UserMetricsAction("TabContextMenu_CloseTabsToRight")); - CloseTabs(GetWebContentsesByIndices(indices), - TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB); + ExecuteCloseTabsByIndicesCommand( + GetIndicesClosedByCommand(context_index, command_id)); break; } @@ -1527,8 +1647,34 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index, ReentrancyCheck reentrancy_check(&reentrancy_guard_); base::RecordAction(UserMetricsAction("TabContextMenu_TogglePinned")); + std::vector indices = GetIndicesForCommand(context_index); + std::vector groups_to_delete = + GetGroupsDestroyedFromRemovingIndices(indices); + bool pin = WillContextMenuPin(context_index); + + // If there are groups that will be deleted by closing tabs from the + // context menu, confirm the group deletion first, and then perform the + // close, either through the callback provided to confirm, or directly if + // the Confirm is allowing a synchronous delete. + if (pin && !groups_to_delete.empty()) { + base::OnceCallback callback = base::BindOnce( + [](TabStripModel* model, std::vector indices, + bool pin_indices) { + model->SetTabsPinned(indices, pin_indices); + }, + base::Unretained(this), indices, pin); + + // If the delegate returns false for confirming the destroy of groups + // that means that the user needs to make a decision about the + // destruction first. prevent CloseTabs from being called. + if (!delegate_->ConfirmRemovingAllTabsFromGroups(groups_to_delete, + std::move(callback))) { + return; + } + } + SetTabsPinned(indices, pin); break; } @@ -1636,15 +1782,18 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index, break; } - case CommandFollowSite: { - base::RecordAction(UserMetricsAction("DesktopFeed.FollowSite")); - FollowSites(GetIndicesForCommand(context_index)); - break; - } - - case CommandUnfollowSite: { - base::RecordAction(UserMetricsAction("DesktopFeed.UnfollowSite")); - UnfollowSites(GetIndicesForCommand(context_index)); + case CommandCommerceProductSpecifications: { + // ProductSpecs can only be triggered on non-incognito profiles. + DCHECK(!profile_->IsIncognitoProfile()); + auto indices = GetIndicesForCommand(context_index); + auto selected_web_contents = + GetWebContentsesByIndices(GetIndicesForCommand(context_index)); + auto eligible_urls = + commerce::GetListOfProductSpecsEligibleUrls(selected_web_contents); + Browser* browser = + chrome::FindBrowserWithTab(GetWebContentsAt(context_index)); + chrome::OpenCommerceProductSpecificationsTab(browser, eligible_urls, + indices.back()); break; } @@ -1669,9 +1818,7 @@ void TabStripModel::ExecuteContextMenuCommand(int context_index, indices.push_back(i); } - CloseTabs(GetWebContentsesByIndices(indices), - TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB); - + ExecuteCloseTabsByIndicesCommand(indices); break; } @@ -1703,6 +1850,70 @@ void TabStripModel::ExecuteAddToExistingWindowCommand(int context_index, browser_index); } +std::vector +TabStripModel::GetGroupsDestroyedFromRemovingIndices( + const std::vector& indices) const { + if (!SupportsTabGroups()) { + return std::vector(); + } + + // Collect indices of tabs in each group. + std::map> group_indices_map; + for (const int index : indices) { + std::optional tab_group = GetTabGroupForTab(index); + if (!tab_group.has_value()) { + continue; + } + + if (!group_indices_map.contains(tab_group.value())) { + group_indices_map.emplace(tab_group.value(), std::vector{}); + } + + group_indices_map[tab_group.value()].emplace_back(index); + } + + // collect the groups that are going to be destoyed because all tabs are + // closing. + std::vector groups_to_delete; + for (const auto& [group, group_indices] : group_indices_map) { + if (group_model()->GetTabGroup(group)->tab_count() == + static_cast(group_indices.size())) { + groups_to_delete.emplace_back(group); + } + } + return groups_to_delete; +} + +void TabStripModel::ExecuteCloseTabsByIndicesCommand( + const std::vector& indices_to_delete) { + std::vector groups_to_delete = + GetGroupsDestroyedFromRemovingIndices(indices_to_delete); + + // If there are groups that will be deleted by closing tabs from the context + // menu, confirm the group deletion first, and then perform the close, either + // through the callback provided to confirm, or directly if the Confirm is + // allowing a synchronous delete. + if (!groups_to_delete.empty()) { + base::OnceCallback callback = + base::BindOnce(&TabStripModel::CloseTabs, base::Unretained(this), + GetWebContentsesByIndices(indices_to_delete), + TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB | + TabCloseTypes::CLOSE_USER_GESTURE); + + // If the delegate returns false for confirming the destroy of groups + // that means that the user needs to make a decision about the + // destruction first. prevent CloseTabs from being called. + if (!delegate_->ConfirmDestroyingGroups(groups_to_delete, + std::move(callback))) { + return; + } + } + + CloseTabs(GetWebContentsesByIndices(indices_to_delete), + TabCloseTypes::CLOSE_CREATE_HISTORICAL_TAB | + TabCloseTypes::CLOSE_USER_GESTURE); +} + bool TabStripModel::WillContextMenuMuteSites(int index) { return !chrome::AreAllSitesMuted(*this, GetIndicesForCommand(index)); } @@ -1769,13 +1980,15 @@ int TabStripModel::GetIndexOfNextWebContentsOpenedBy(const WebContents* opener, // Check tabs after start_index first. for (int i = start_index + 1; i < count(); ++i) { - if (contents_data_[i]->opener() == opener) + if (GetTabAtIndex(i)->opener() == opener) { return i; + } } // Then check tabs before start_index, iterating backwards. for (int i = start_index - 1; i >= 0; --i) { - if (contents_data_[i]->opener() == opener) + if (GetTabAtIndex(i)->opener() == opener) { return i; + } } return kNoTab; } @@ -1805,20 +2018,24 @@ std::optional TabStripModel::GetNextExpandedActiveTab( } void TabStripModel::ForgetAllOpeners() { - for (const auto& data : contents_data_) - data->set_opener(nullptr); + for (int i = 0; i < GetTabCount(); ++i) { + GetTabAtIndex(i)->set_opener(nullptr); + } } void TabStripModel::ForgetOpener(WebContents* contents) { const int index = GetIndexOfWebContents(contents); CHECK(ContainsIndex(index)); - contents_data_[index]->set_opener(nullptr); + GetTabAtIndex(index)->set_opener(nullptr); } void TabStripModel::WriteIntoTrace(perfetto::TracedValue context) const { auto dict = std::move(context).WriteDictionary(); dict.Add("active_index", active_index()); - dict.Add("tabs", contents_data_); + // TODO(b/335438706): Add tracing for collection infrastructure. + if (IsContentsDataVector()) { + dict.Add("tabs", GetContentsDataAsVector()); + } } /////////////////////////////////////////////////////////////////////////////// @@ -1884,7 +2101,7 @@ bool TabStripModel::IsNewTabAtEndOfTabStrip(WebContents* contents) const { } std::vector TabStripModel::GetWebContentsesByIndices( - const std::vector& indices) { + const std::vector& indices) const { std::vector items; items.reserve(indices.size()); for (int index : indices) @@ -1903,12 +2120,20 @@ int TabStripModel::InsertTabAtImpl( const bool pin = (add_types & ADD_PINNED) != 0; index = ConstrainInsertionIndex(index, pin); + // If there's already an active tab, and the new tab will become active, send + // a notification. + if (selection_model_.active().has_value() && active && !closing_all_) { + GetTabAtIndex(active_index()) + ->WillEnterBackground(base::PassKey()); + } + // Have to get the active contents before we monkey with the contents // otherwise we run into problems when we try to change the active contents // since the old contents and the new contents will be the same... WebContents* active_contents = GetActiveWebContents(); WebContents* raw_contents = tab->contents(); CHECK_EQ(this, tab->owning_model()); + tab->set_pinned(pin); if ((add_types & ADD_INHERIT_OPENER) && active_contents) { if (active) { @@ -1931,12 +2156,18 @@ int TabStripModel::InsertTabAtImpl( // Force the group value to be set since we perform contiguity checks on the // tab groups when rendered in views. this will not inform the observers of // the group change until GroupTab called after OnTabStripModelChanged. - tab->set_group(group); + // TODO(shibalik): Check if we can avoid setting a group and re-setting a + // group in GroupTab. This behavior was needed as part of b/40058241/ . + if (IsContentsDataVector()) { + tab->set_group(group); + GetContentsDataAsVector().insert(GetContentsDataAsVector().begin() + index, + std::move(tab)); + } else { + GetContentsDataAsCollection()->AddTabRecursive(std::move(tab), index, group, + pin); + } TabStripSelectionChange selection(GetActiveWebContents(), selection_model_); - - contents_data_.insert(contents_data_.begin() + index, std::move(tab)); - selection_model_.IncrementFrom(index); if (active) { @@ -1958,13 +2189,21 @@ int TabStripModel::InsertTabAtImpl( if (group_model_ && group.has_value()) { // Unset the group at the index of the inserted WebContents so that the // GroupTab functionality isn't skipped. - contents_data_[index]->set_group(std::nullopt); - GroupTab(index, group.value()); + GetTabAtIndex(index)->set_group(std::nullopt); + GroupTab(index, group.value(), GetTabGroupForTab(index)); } return index; } +tabs::TabModel* TabStripModel::GetTabAtIndex(int index) const { + if (IsContentsDataVector()) { + return GetContentsDataAsVector()[index].get(); + } else { + return GetContentsDataAsCollection()->GetTabAtIndexRecursive(index); + } +} + void TabStripModel::CloseTabs(base::span items, uint32_t close_types) { std::vector filtered_items; @@ -2023,6 +2262,16 @@ bool TabStripModel::CloseWebContentses( if (items.empty()) return true; + for (size_t i = 0; i < items.size(); ++i) { + int index = GetIndexOfWebContents(items[i]); + if (index == active_index() && !closing_all_) { + GetTabAtIndex(active_index()) + ->WillEnterBackground(base::PassKey()); + } + GetTabAtIndex(index)->WillDetach(base::PassKey(), + tabs::TabInterface::DetachReason::kDelete); + } + // We only try the fast shutdown path if the whole browser process is *not* // shutting down. Fast shutdown during browser termination is handled in // browser_shutdown::OnShutdownStarting. @@ -2082,22 +2331,6 @@ bool TabStripModel::CloseWebContentses( detached_web_contents.push_back(std::move(dwc)); } - if (!detached_web_contents.empty()) { - // ClosedTabCache will only take ownership of the last recently closed tab. - delegate_->CacheWebContents(detached_web_contents); - } - - // If the delegate takes ownership, it must reset the reason. -#if DCHECK_IS_ON() - for (auto& dwc : detached_web_contents) - if (dwc->tab && dwc->tab->contents()) { - DCHECK_EQ(TabStripModelChange::RemoveReason::kDeleted, - dwc->remove_reason); - } else { - DCHECK_EQ(TabStripModelChange::RemoveReason::kCached, dwc->remove_reason); - } -#endif - for (auto& dwc : detached_web_contents) notifications->detached_web_contents.push_back(std::move(dwc)); @@ -2107,7 +2340,7 @@ bool TabStripModel::CloseWebContentses( WebContents* TabStripModel::GetWebContentsAtImpl(int index) const { CHECK(ContainsIndex(index)) << "Failed to find: " << index << " in: " << count() << " entries."; - return contents_data_[index]->contents(); + return GetTabAtIndex(index)->contents(); } TabStripSelectionChange TabStripModel::SetSelection( @@ -2120,6 +2353,12 @@ TabStripSelectionChange TabStripModel::SetSelection( selection.new_model = new_model; selection.reason = reason; + if (selection_model_.active().has_value() && new_model.active().has_value() && + selection_model_.active().value() != new_model.active().value()) { + GetTabAtIndex(active_index()) + ->WillEnterBackground(base::PassKey()); + } + // Validate that |new_model| only selects tabs that actually exist. CHECK(empty() || new_model.active().has_value(), base::NotFatalUntil::M124); CHECK(empty() || ContainsIndex(new_model.active().value()), @@ -2171,8 +2410,9 @@ void TabStripModel::SelectRelativeTab(TabRelativeDirection direction, TabStripUserGestureDetails detail) { // This may happen during automated testing or if a user somehow buffers // many key accelerators. - if (contents_data_.empty()) + if (empty()) { return; + } const int start_index = active_index(); std::optional start_group = @@ -2216,7 +2456,7 @@ void TabStripModel::MoveTabRelative(TabRelativeDirection direction) { // actually moving the tab just change its group membership. if (group_model_ && current_group != target_group) { if (current_group.has_value()) { - UngroupTab(current_index); + UngroupTab(current_index, GetTabGroupForTab(current_index)); return; } else if (target_group.has_value()) { // If the tab is at a group boundary and the group is collapsed, treat the @@ -2229,7 +2469,8 @@ void TabStripModel::MoveTabRelative(TabRelativeDirection direction) { ? tabs_in_group.end() - 1 : tabs_in_group.start(); } else { - GroupTab(current_index, target_group.value()); + GroupTab(current_index, target_group.value(), + GetTabGroupForTab(current_index)); return; } } @@ -2243,15 +2484,59 @@ void TabStripModel::MoveWebContentsAtImpl(int index, bool select_after_move) { FixOpeners(index); - TabStripSelectionChange selection(GetActiveWebContents(), selection_model_); + CHECK_LT(index, count()); + CHECK_LT(to_position, count()); - CHECK_LT(index, static_cast(contents_data_.size())); - CHECK_LT(to_position, static_cast(contents_data_.size())); - std::unique_ptr moved_data = std::move(contents_data_[index]); - WebContents* web_contents = moved_data->contents(); - contents_data_.erase(contents_data_.begin() + index); - contents_data_.insert(contents_data_.begin() + to_position, - std::move(moved_data)); + if (IsContentsDataVector()) { + MoveWebContentsAtImplWithVector(index, to_position, select_after_move); + } else { + MoveWebContentsAtImplWithCollection(index, to_position, select_after_move); + } +} + +void TabStripModel::MoveWebContentsAtImplWithVector(int index, + int to_position, + bool select_after_move) { + CHECK(IsContentsDataVector()); + std::unique_ptr moved_data = + std::move(GetContentsDataAsVector()[index]); + WebContents* const web_contents = moved_data->contents(); + + GetContentsDataAsVector().erase(GetContentsDataAsVector().begin() + index); + GetContentsDataAsVector().insert( + GetContentsDataAsVector().begin() + to_position, std::move(moved_data)); + SendMoveNotificationForWebContents(index, to_position, select_after_move, + web_contents); +} + +void TabStripModel::MoveWebContentsAtImplWithCollection( + int index, + int to_position, + bool select_after_move) { + CHECK(!IsContentsDataVector()); + const bool is_tab_pinned = IsTabPinned(to_position); + + std::unique_ptr moved_data = + GetContentsDataAsCollection()->RemoveTabAtIndexRecursive(index); + WebContents* const web_contents = moved_data->contents(); + + GetContentsDataAsCollection()->AddTabRecursive( + std::move(moved_data), to_position, std::nullopt, is_tab_pinned); + + SendMoveNotificationForWebContents(index, to_position, select_after_move, + web_contents); +} + +void TabStripModel::SendMoveNotificationForWebContents( + int index, + int to_position, + bool select_after_move, + WebContents* web_contents) { + if (select_after_move && GetActiveWebContents() != web_contents) { + GetTabAtIndex(active_index()) + ->WillEnterBackground(base::PassKey()); + } + TabStripSelectionChange selection(GetActiveWebContents(), selection_model_); selection_model_.Move(index, to_position, 1); if (!selection_model_.IsSelected(to_position) && select_after_move) { @@ -2310,57 +2595,199 @@ void TabStripModel::MoveSelectedTabsToImpl(int index, } } -void TabStripModel::AddToNewGroupImpl(const std::vector& indices, - const tab_groups::TabGroupId& new_group) { - if (!group_model_) +void TabStripModel::AddToNewGroupImpl(const std::vector indices, + const tab_groups::TabGroupId new_group) { + if (!group_model_) { return; - - DCHECK(!base::Contains(contents_data_, new_group, &tabs::TabModel::group)); - group_model_->AddTabGroup(new_group, std::nullopt); - - // Find a destination for the first tab that's not pinned or inside another - // group. We will stack the rest of the tabs up to its right. - int destination_index = -1; - for (int i = indices[0]; i < count(); i++) { - const int destination_candidate = i + 1; - - // Grouping at the end of the tabstrip is always valid. - if (!ContainsIndex(destination_candidate)) { - destination_index = destination_candidate; - break; - } - - // Grouping in the middle of pinned tabs is never valid. - if (IsTabPinned(destination_candidate)) - continue; - - // Otherwise, grouping is valid if the destination is not in the middle of a - // different group. - std::optional destination_group = - GetTabGroupForTab(destination_candidate); - if (!destination_group.has_value() || - destination_group != GetTabGroupForTab(indices[0])) { - destination_index = destination_candidate; - break; - } } - MoveTabsAndSetGroupImpl(indices, destination_index, new_group); + DCHECK([&]() { + for (int i = 0; i < GetTabCount(); ++i) { + tabs::TabModel* tab = GetTabAtIndex(i); + if (tab->group().has_value() && tab->group().value() == new_group) { + return false; + } + } + return true; + }()); + + group_model_->AddTabGroup(new_group, std::nullopt); + + if (!IsContentsDataVector()) { + AddToNewGroupWithCollectionImpl(indices, new_group); + } else { + // Find a destination for the first tab that's not pinned or inside another + // group. We will stack the rest of the tabs up to its right. + int destination_index = -1; + for (int i = indices[0]; i < count(); i++) { + const int destination_candidate = i + 1; + + // Grouping at the end of the tabstrip is always valid. + if (!ContainsIndex(destination_candidate)) { + destination_index = destination_candidate; + break; + } + + // Grouping in the middle of pinned tabs is never valid. + if (IsTabPinned(destination_candidate)) { + continue; + } + + // Otherwise, grouping is valid if the destination is not in the middle of + // a different group. + std::optional destination_group = + GetTabGroupForTab(destination_candidate); + if (!destination_group.has_value() || + destination_group != GetTabGroupForTab(indices[0])) { + destination_index = destination_candidate; + break; + } + } + MoveTabsAndSetGroupImpl(indices, destination_index, new_group); + } // Excluding the active tab, deselect all tabs being added to the group. // See crbug/1301846 for more info. const gfx::Range tab_indices = group_model()->GetTabGroup(new_group)->ListTabs(); - for (auto index = tab_indices.start(); index < tab_indices.end(); ++index) - if (active_index() != static_cast(index) && IsTabSelected(index)) + for (auto index = tab_indices.start(); index < tab_indices.end(); ++index) { + if (active_index() != static_cast(index) && IsTabSelected(index)) { ToggleSelectionAt(index); + } + } } -void TabStripModel::AddToExistingGroupImpl( - const std::vector& indices, - const tab_groups::TabGroupId& group) { - if (!group_model_) +void TabStripModel::AddToNewGroupWithCollectionImpl( + const std::vector indices, + const tab_groups::TabGroupId new_group) { + tabs::TabGroupTabCollection* group_collection = + GetContentsDataAsCollection()->CreateNewGroupCollectionForTab( + GetTabAtIndex(indices[0]), new_group); + + std::vector tabs; + for (int index : indices) { + tabs.push_back(GetTabAtIndex(index)); + } + + AddTabsToGroupCollection(tabs, group_collection); +} + +void TabStripModel::AddToExistingGroupWithCollectionImpl( + const std::vector indices, + const tab_groups::TabGroupId group, + const bool add_to_end) { + std::vector tabs; + for (int index : indices) { + tabs.push_back(GetTabAtIndex(index)); + } + + tabs::TabGroupTabCollection* group_collection = + GetContentsDataAsCollection() + ->GetUnpinnedCollection() + ->GetTabGroupCollection(group); + + std::vector left_of_group; + std::vector right_of_group; + for (tabs::TabModel* tab : tabs) { + if (GetIndexOfTab(tab->GetHandle()) < + GetContentsDataAsCollection()->GetIndexOfTabRecursive( + group_collection->GetTabAtIndex(0))) { + left_of_group.push_back(tab); + } else if (GetIndexOfTab(tab->GetHandle()) > + GetContentsDataAsCollection()->GetIndexOfTabRecursive( + group_collection->GetTabAtIndex(0))) { + right_of_group.push_back(tab); + } + } + + if (add_to_end) { + AddTabsToGroupCollection(tabs, group_collection); + } else { + // Reverse the left of the group to maintain right final ordering after all + // the move operations. + std::reverse(left_of_group.begin(), left_of_group.end()); + AddTabsToGroupCollection(left_of_group, group_collection, true); + AddTabsToGroupCollection(right_of_group, group_collection); + } +} + +void TabStripModel::AddTabsToGroupCollection( + const std::vector tabs, + tabs::TabGroupTabCollection* group_collection, + bool start_of_group) { + for (tabs::TabModel* tab : tabs) { + size_t index = + GetContentsDataAsCollection()->GetIndexOfTabRecursive(tab).value(); + + // This mostly needs to be called for observer notification. Otherwise it is + // not needed. It results in move operation potentially being called twice. + if (tab->pinned()) { + index = SetTabPinnedImpl(index, false); + } + + const std::optional old_group = tab->group(); + + // Remove the tab from the current index. + std::unique_ptr tab_model = + GetContentsDataAsCollection()->RemoveTabAtIndexRecursive(index); + + // Insert the tab into the group based on start_of_group. + if (start_of_group) { + group_collection->AddTab(std::move(tab_model), 0); + } else { + group_collection->AppendTab(std::move(tab_model)); + } + + const size_t to_position = + GetContentsDataAsCollection()->GetIndexOfTabRecursive(tab).value(); + + if (to_position != index) { + SendMoveNotificationForWebContents(index, to_position, false, + tab->contents()); + } + + GroupTab(to_position, group_collection->GetTabGroupId(), old_group); + } +} + +void TabStripModel::RemoveTabsFromGroupCollection( + const std::vector tabs, + tabs::TabGroupTabCollection* group_collection, + bool move_to_left) { + for (tabs::TabModel* tab : tabs) { + const size_t index_of_collection_in_unpinned_container = + GetContentsDataAsCollection() + ->GetUnpinnedCollection() + ->GetIndexOfCollection(group_collection) + .value(); + const size_t index_of_tab_in_tab_strip = + GetContentsDataAsCollection()->GetIndexOfTabRecursive(tab).value(); + + std::unique_ptr tab_model = + GetContentsDataAsCollection()->RemoveTabAtIndexRecursive( + index_of_tab_in_tab_strip); + GetContentsDataAsCollection()->GetUnpinnedCollection()->AddTab( + std::move(tab_model), + index_of_collection_in_unpinned_container + (move_to_left ? 0 : 1)); + + const size_t to_position = + GetContentsDataAsCollection()->GetIndexOfTabRecursive(tab).value(); + + if (to_position != index_of_tab_in_tab_strip) { + SendMoveNotificationForWebContents(index_of_tab_in_tab_strip, to_position, + false, tab->contents()); + } + + UngroupTab(to_position, group_collection->GetTabGroupId()); + } +} + +void TabStripModel::AddToExistingGroupImpl(const std::vector& indices, + const tab_groups::TabGroupId& group, + const bool add_to_end) { + if (!group_model_) { return; + } // Do nothing if the "existing" group can't be found. This would only happen // if the existing group is closed programmatically while the user is @@ -2370,8 +2797,13 @@ void TabStripModel::AddToExistingGroupImpl( // If this happens, the browser should not crash. So here we just make it a // no-op, since we don't want to create unintended side effects in this rare // corner case. - if (!group_model_->ContainsTabGroup(group)) + if (!group_model_->ContainsTabGroup(group)) { return; + } + + if (!IsContentsDataVector()) { + return AddToExistingGroupWithCollectionImpl(indices, group, add_to_end); + } const TabGroup* group_object = group_model_->GetTabGroup(group); int first_tab_in_group = group_object->GetFirstTab().value(); @@ -2384,7 +2816,7 @@ void TabStripModel::AddToExistingGroupImpl( std::vector tabs_right_of_group; for (int index : indices) { if (index >= first_tab_in_group && index <= last_tab_in_group) { - GroupTab(index, group); + GroupTab(index, group, GetTabGroupForTab(index)); } else if (index < first_tab_in_group) { tabs_left_of_group.push_back(index); } else { @@ -2392,8 +2824,15 @@ void TabStripModel::AddToExistingGroupImpl( } } - MoveTabsAndSetGroupImpl(tabs_left_of_group, first_tab_in_group, group); - MoveTabsAndSetGroupImpl(tabs_right_of_group, last_tab_in_group + 1, group); + if (add_to_end) { + std::vector all_tabs = tabs_left_of_group; + all_tabs.insert(all_tabs.end(), tabs_right_of_group.begin(), + tabs_right_of_group.end()); + MoveTabsAndSetGroupImpl(all_tabs, last_tab_in_group + 1, group); + } else { + MoveTabsAndSetGroupImpl(tabs_left_of_group, first_tab_in_group, group); + MoveTabsAndSetGroupImpl(tabs_right_of_group, last_tab_in_group + 1, group); + } } void TabStripModel::MoveTabsAndSetGroupImpl( @@ -2431,9 +2870,12 @@ void TabStripModel::MoveAndSetGroup( int index, int new_index, std::optional new_group) { - if (!group_model_) + if (!group_model_) { return; + } + const std::optional old_group = + GetTabGroupForTab(index); if (new_group.has_value()) { // Unpin tabs when grouping -- the states should be mutually exclusive. // Here we move the tab twice to ensure the tabstrip is always in a valid @@ -2442,29 +2884,18 @@ void TabStripModel::MoveAndSetGroup( index = SetTabPinnedImpl(index, false); } - std::optional old_group = GetTabGroupForTab(index); - if (old_group.has_value()) { - // TODO (1302144): We don't maintain group contiguity in this case. If - // |index| is in the middle of |old_group|, GroupTab will notify observers - // while |old_group| is split in twain. Simply reordering the move and - // group actions won't do it; we'd need to move, ungroup, move, and then - // group. - GroupTab(index, new_group.value()); - if (index != new_index) - MoveWebContentsAtImpl(index, new_index, false); - } else { - // Move the tab now so that group contiguity is preserved. - // When grouping, this will move the tab next to |new_group|. - if (index != new_index) - MoveWebContentsAtImpl(index, new_index, false); - GroupTab(new_index, new_group.value()); + if (index != new_index) { + MoveWebContentsAtImpl(index, new_index, false); } + GroupTab(new_index, new_group.value(), old_group); + } else { // Move the tab now so that group contiguity is preserved. // When ungrouping, this will move the tab to the edge of |old_group|. - if (index != new_index) + if (index != new_index) { MoveWebContentsAtImpl(index, new_index, false); - UngroupTab(new_index); + } + UngroupTab(new_index, old_group); } } @@ -2473,110 +2904,131 @@ void TabStripModel::AddToReadLaterImpl(const std::vector& indices) { delegate_->AddToReadLater(GetWebContentsAt(index)); } -std::optional TabStripModel::UngroupTab(int index) { - if (!group_model_) +std::optional TabStripModel::UngroupTab( + int index, + const std::optional old_group) { + if (!group_model_) { return std::nullopt; + } - std::optional group = GetTabGroupForTab(index); - if (!group.has_value()) + if (!old_group.has_value()) { return std::nullopt; + } // Update the tab. - contents_data_[index]->set_group(std::nullopt); + GetTabAtIndex(index)->set_group(std::nullopt); for (auto& observer : observers_) { observer.TabGroupedStateChanged(std::nullopt, - contents_data_[index]->contents(), index); + GetTabAtIndex(index)->contents(), index); } // Update the group model. - TabGroup* tab_group = group_model_->GetTabGroup(group.value()); + TabGroup* tab_group = group_model_->GetTabGroup(old_group.value()); tab_group->RemoveTab(); - if (tab_group->IsEmpty()) - group_model_->RemoveTabGroup(group.value()); + if (tab_group->IsEmpty()) { + if (!IsContentsDataVector()) { + tabs::TabGroupTabCollection* group_collection = + GetContentsDataAsCollection() + ->GetUnpinnedCollection() + ->GetTabGroupCollection(old_group.value()); - return group; + if (group_collection->TabCountRecursive() == 0) { + GetContentsDataAsCollection()->GetUnpinnedCollection()->CloseTabGroup( + group_collection); + } + } + group_model_->RemoveTabGroup(old_group.value()); + } + + return old_group; } -void TabStripModel::GroupTab(int index, const tab_groups::TabGroupId& group) { +void TabStripModel::GroupTab( + int index, + const tab_groups::TabGroupId& group, + const std::optional old_group) { if (!group_model_) return; // Check for an old group first, so that any groups that are changed can be // notified appropriately. - std::optional old_group = GetTabGroupForTab(index); if (old_group.has_value()) { - if (old_group.value() == group) + if (old_group.value() == group) { return; - else - UngroupTab(index); + } else { + UngroupTab(index, old_group); + } } - contents_data_[index]->set_group(group); + GetTabAtIndex(index)->set_group(group); for (auto& observer : observers_) { - observer.TabGroupedStateChanged(group, contents_data_[index]->contents(), + observer.TabGroupedStateChanged(group, GetTabAtIndex(index)->contents(), index); } group_model_->GetTabGroup(group)->AddTab(); } -void TabStripModel::DisconnectSavedTabGroups( - const std::vector& indices) const { - if (!base::FeatureList::IsEnabled(features::kTabGroupsSave)) { - return; - } - - tab_groups::SavedTabGroupKeyedService* const keyed_service = - tab_groups::SavedTabGroupServiceFactory::GetForProfile(profile_); - const tab_groups::SavedTabGroupModel* const stg_model = - keyed_service->model(); - - // Count the tabs in each group in `indices`. - std::unordered_map - tabs_per_group; - for (const int index : indices) { - const std::optional group = - GetTabGroupForTab(index); - if (group.has_value() && stg_model->Contains(group.value())) { - tabs_per_group[group.value()]++; - } - } - - // Disconnect each group fully contained in `indices`. - for (const auto& [group, count] : tabs_per_group) { - const gfx::Range grouped_tabs = - group_model_->GetTabGroup(group)->ListTabs(); - if (grouped_tabs.length() == count) { - keyed_service->DisconnectLocalTabGroup(group); - } - } -} - int TabStripModel::SetTabPinnedImpl(int index, bool pinned) { CHECK(ContainsIndex(index)); - if (contents_data_[index]->pinned() == pinned) + tabs::TabModel* tab_model = GetTabAtIndex(index); + if (tab_model->pinned() == pinned) { return index; + } // Upgroup tabs if pinning -- the states should be mutually exclusive. - if (pinned && group_model_) - UngroupTab(index); + if (pinned && group_model_) { + UngroupTab(index, GetTabGroupForTab(index)); + } // The tab's position may have to change as the pinned tab state is changing. - int non_pinned_tab_index = IndexOfFirstNonPinnedTab(); - contents_data_[index]->set_pinned(pinned); - if (pinned && index != non_pinned_tab_index) { - MoveWebContentsAtImpl(index, non_pinned_tab_index, false); - index = non_pinned_tab_index; - } else if (!pinned && index + 1 != non_pinned_tab_index) { - MoveWebContentsAtImpl(index, non_pinned_tab_index - 1, false); - index = non_pinned_tab_index - 1; - } - + const int index_after_pin_state_update = + UpdatePinAndMoveWebContents(index, pinned); + CHECK_EQ(GetTabAtIndex(index_after_pin_state_update), tab_model); for (auto& observer : observers_) { - observer.TabPinnedStateChanged(this, contents_data_[index]->contents(), - index); + observer.TabPinnedStateChanged(this, tab_model->contents(), + index_after_pin_state_update); } - return index; + return index_after_pin_state_update; +} + +int TabStripModel::UpdatePinAndMoveWebContents(int index, bool pinned) { + tabs::TabModel* const tab_model = GetTabAtIndex(index); + const int non_pinned_tab_index = IndexOfFirstNonPinnedTab(); + + if (IsContentsDataVector()) { + tab_model->set_pinned(pinned); + if (pinned && index != non_pinned_tab_index) { + MoveWebContentsAtImpl(index, non_pinned_tab_index, false); + return non_pinned_tab_index; + } else if (!pinned && index + 1 != non_pinned_tab_index) { + MoveWebContentsAtImpl(index, non_pinned_tab_index - 1, false); + return non_pinned_tab_index - 1; + } + return index; + } else { + if (pinned) { + std::unique_ptr tab_to_pin = + GetContentsDataAsCollection()->RemoveTabAtIndexRecursive(index); + GetContentsDataAsCollection()->GetPinnedCollection()->AppendTab( + std::move(tab_to_pin)); + } else { + std::unique_ptr tab_to_unpin = + GetContentsDataAsCollection()->RemoveTabAtIndexRecursive(index); + GetContentsDataAsCollection()->GetUnpinnedCollection()->AddTab( + std::move(tab_to_unpin), 0); + } + + const int index_after_updating_pinned_state = + pinned ? non_pinned_tab_index : non_pinned_tab_index - 1; + + if (index_after_updating_pinned_state != index) { + SendMoveNotificationForWebContents(index, + index_after_updating_pinned_state, + false, tab_model->contents()); + } + return index_after_updating_pinned_state; + } } std::vector TabStripModel::SetTabsPinned(const std::vector& indices, @@ -2657,22 +3109,27 @@ void TabStripModel::FixOpeners(int index) { WebContents* old_contents = GetWebContentsAtImpl(index); WebContents* new_opener = GetOpenerOfWebContentsAt(index); - for (auto& data : contents_data_) { - if (data->opener() != old_contents) + for (int i = 0; i < GetTabCount(); i++) { + tabs::TabModel* tab = GetTabAtIndex(i); + if (tab->opener() != old_contents) { continue; + } // Ensure a tab isn't its own opener. - data->set_opener(new_opener == data->contents() ? nullptr : new_opener); + tab->set_opener(new_opener == tab->contents() ? nullptr : new_opener); } // Sanity check that none of the tabs' openers refer |old_contents| or // themselves. - DCHECK(!base::ranges::any_of( - contents_data_, - [old_contents](const std::unique_ptr& data) { - return data->opener() == old_contents || - data->opener() == data->contents(); - })); + DCHECK([&]() { + for (int i = 0; i < GetTabCount(); ++i) { + tabs::TabModel* tab = GetTabAtIndex(i); + if (tab->opener() == old_contents || tab->opener() == tab->contents()) { + return false; + } + } + return true; + }()); } void TabStripModel::EnsureGroupContiguity(int index) { @@ -2686,12 +3143,12 @@ void TabStripModel::EnsureGroupContiguity(int index) { if (old_group != new_left_group && old_group != new_right_group) { if (new_left_group == new_right_group && new_left_group.has_value()) { // The tab is in the middle of an existing group, so add it to that group. - GroupTab(index, new_left_group.value()); + GroupTab(index, new_left_group.value(), GetTabGroupForTab(index)); } else if (old_group.has_value() && group_model_->GetTabGroup(old_group.value())->tab_count() > 1) { // The tab is between groups and its group is non-contiguous, so clear // this tab's group. - UngroupTab(index); + UngroupTab(index, GetTabGroupForTab(index)); } } } @@ -2734,8 +3191,9 @@ void TabStripModel::OnActiveTabChanged( // Forget the opener relationship if it needs to be reset whenever the // active tab changes (see comment in TabStripModel::AddWebContents, where // the flag is set). - if (contents_data_[index]->reset_opener_on_active_tab_change()) + if (GetTabAtIndex(index)->reset_opener_on_active_tab_change()) { ForgetOpener(old_contents); + } } } DCHECK(selection.new_model.active().has_value()); @@ -2808,6 +3266,18 @@ int TabStripModel::DetermineInsertionIndex(ui::PageTransition transition, return count(); } +void TabStripModel::GroupCloseStopped(const tab_groups::TabGroupId& group) { + delegate_->GroupCloseStopped(group); + + gfx::Range tabs_in_group = group_model_->GetTabGroup(group)->ListTabs(); + std::vector ungrouping_tabs_indices; + ungrouping_tabs_indices.reserve(tabs_in_group.length()); + for (uint32_t i = tabs_in_group.start(); i < tabs_in_group.end(); i++) { + ungrouping_tabs_indices.push_back(i); + } + RemoveFromGroup(ungrouping_tabs_indices); +} + std::optional TabStripModel::DetermineNewSelectedIndex( int removing_index) const { DCHECK(ContainsIndex(removing_index)); @@ -2878,3 +3348,14 @@ std::optional TabStripModel::DetermineNewSelectedIndex( return removing_index; } + +TabStripModel::ScopedTabStripModalUIImpl::ScopedTabStripModalUIImpl( + TabStripModel* model) + : model_(model) { + CHECK(!model_->showing_modal_ui_); + model_->showing_modal_ui_ = true; +} + +TabStripModel::ScopedTabStripModalUIImpl::~ScopedTabStripModalUIImpl() { + model_->showing_modal_ui_ = false; +} diff --git a/src/chrome/browser/ui/views/frame/browser_root_view.cc b/src/chrome/browser/ui/views/frame/browser_root_view.cc index 20a8fdfa..2cdfa497 100644 --- a/src/chrome/browser/ui/views/frame/browser_root_view.cc +++ b/src/chrome/browser/ui/views/frame/browser_root_view.cc @@ -1,4 +1,4 @@ -// Copyright 2024 The Chromium Authors and Alex313031 +// Copyright 2012 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -198,7 +198,7 @@ BrowserRootView::DropInfo::~DropInfo() { BrowserRootView::BrowserRootView(BrowserView* browser_view, views::Widget* widget) : views::internal::RootView(widget), browser_view_(browser_view) { - scroll_event_changes_tab_ = ShouldScrollChangesTab(); + scroll_event_changes_tab_ = ShouldScrollChangesTab(); } BrowserRootView::~BrowserRootView() { @@ -240,7 +240,7 @@ bool BrowserRootView::CanDrop(const ui::OSExchangeData& data) { // necessary because we don't want to return true if the custom MIME type is // there but the mouse is not over the tab strip region, and we don't know the // current mouse location. - // TODO(crbug.com/1307594): This is a smoking gun code smell; + // TODO(crbug.com/40828528): This is a smoking gun code smell; // TabStripRegionView and Toolbar have different affordances, so they should // separately override the drag&drop methods. if (data.HasCustomFormat( @@ -416,7 +416,8 @@ void BrowserRootView::OnMouseExited(const ui::MouseEvent& event) { RootView::OnMouseExited(event); } -gfx::Size BrowserRootView::CalculatePreferredSize() const { +gfx::Size BrowserRootView::CalculatePreferredSize( + const views::SizeBounds& available_size) const { return browser_view_->GetRestoredBounds().size(); } @@ -474,28 +475,23 @@ void BrowserRootView::PaintChildren(const views::PaintInfo& paint_info) { cc::PaintFlags flags; flags.setColor(toolbar_top_separator_color); flags.setAntiAlias(true); - if (features::IsChromeRefresh2023()) { - const float stroke_width = scale; - // Outset the rectangle and corner radius by half the stroke width - // to draw an outer stroke. - const float stroke_outset = stroke_width / 2; - const float corner_radius = - GetLayoutConstant(TOOLBAR_CORNER_RADIUS) * scale + stroke_outset; + const float stroke_width = scale; + // Outset the rectangle and corner radius by half the stroke width + // to draw an outer stroke. + const float stroke_outset = stroke_width / 2; + const float corner_radius = + GetLayoutConstant(TOOLBAR_CORNER_RADIUS) * scale + stroke_outset; - flags.setStyle(cc::PaintFlags::kStroke_Style); - flags.setStrokeWidth(stroke_width); + flags.setStyle(cc::PaintFlags::kStroke_Style); + flags.setStrokeWidth(stroke_width); - // Only draw the top half of the rounded rect. - canvas->ClipRect(gfx::RectF(x, 0, width, bottom + corner_radius), - SkClipOp::kIntersect); + // Only draw the top half of the rounded rect. + canvas->ClipRect(gfx::RectF(x, 0, width, bottom + corner_radius), + SkClipOp::kIntersect); - gfx::RectF rect(x, bottom, width, 2 * corner_radius); - rect.Outset(stroke_outset); - canvas->DrawRoundRect(rect, corner_radius, flags); - } else { - flags.setStyle(cc::PaintFlags::kFill_Style); - canvas->DrawRect(gfx::RectF(x, bottom - scale, width, scale), flags); - } + gfx::RectF rect(x, bottom, width, 2 * corner_radius); + rect.Outset(stroke_outset); + canvas->DrawRoundRect(rect, corner_radius, flags); } } diff --git a/src/chrome/browser/ui/views/frame/browser_root_view.h b/src/chrome/browser/ui/views/frame/browser_root_view.h index 5eef4b0d..9fd1872f 100644 --- a/src/chrome/browser/ui/views/frame/browser_root_view.h +++ b/src/chrome/browser/ui/views/frame/browser_root_view.h @@ -89,7 +89,8 @@ class BrowserRootView : public views::internal::RootView { DropCallback GetDropCallback(const ui::DropTargetEvent& event) override; bool OnMouseWheel(const ui::MouseWheelEvent& event) override; void OnMouseExited(const ui::MouseEvent& event) override; - gfx::Size CalculatePreferredSize() const override; + gfx::Size CalculatePreferredSize( + const views::SizeBounds& available_size) const override; protected: // views::View: diff --git a/src/chrome/browser/ui/views/tabs/tab.cc b/src/chrome/browser/ui/views/tabs/tab.cc index f9dde83e..55201264 100644 --- a/src/chrome/browser/ui/views/tabs/tab.cc +++ b/src/chrome/browser/ui/views/tabs/tab.cc @@ -113,6 +113,10 @@ constexpr int kTabAlertIndicatorCloseButtonPaddingAdjustmentTouchUI = 8; constexpr int kTabAlertIndicatorCloseButtonPaddingAdjustment = 6; constexpr int kTabAlertIndicatorCloseButtonPaddingAdjustmentRefresh = 4; +// When the DiscardRingImprovements feature is enabled, increase the radius of +// the discard ring by this amount if there is enough space. +constexpr int kIncreasedDiscardIndicatorRadiusDp = 2; + bool g_show_hover_card_on_mouse_hover = true; // Helper functions ------------------------------------------------------------ @@ -228,17 +232,11 @@ Tab::Tab(TabSlotController* controller) // |title_| paints on top of an opaque region (the tab background) of a // non-opaque layer (the tabstrip's layer), which cannot currently be detected // by the subpixel-rendering opacity check. - // TODO(https://crbug.com/1139395): Improve the check so that this case doen't + // TODO(crbug.com/40725997): Improve the check so that this case doen't // need a manual suppression by detecting cases where the text is painted onto // onto opaque parts of a not-entirely-opaque layer. title_->SetSkipSubpixelRenderingOpacityCheck(true); - if (features::IsChromeRefresh2023() && - base::FeatureList::IsEnabled(features::kChromeRefresh2023TopChromeFont)) { - title_->SetTextContext(views::style::CONTEXT_LABEL); - title_->SetTextStyle(views::style::STYLE_BODY_4_EMPHASIS); - } - AddChildView(title_.get()); SetEventTargeter(std::make_unique(this)); @@ -337,6 +335,16 @@ void Tab::Layout(PassKey) { } else { MaybeAdjustLeftForPinnedTab(&favicon_bounds, gfx::kFaviconSize); } + + if (base::FeatureList::IsEnabled( + performance_manager::features::kDiscardRingImprovements)) { + icon_->EnlargeDiscardIndicatorRadius( + width() - 2 * tab_style()->GetBottomCornerRadius() >= + gfx::kFaviconSize + 2 * kIncreasedDiscardIndicatorRadiusDp + ? kIncreasedDiscardIndicatorRadiusDp + : 0); + } + // Add space for insets outside the favicon bounds. favicon_bounds.Inset(-icon_->GetInsets()); favicon_bounds.set_size(icon_->GetPreferredSize()); @@ -650,7 +658,7 @@ void Tab::MaybeUpdateHoverStatus(const ui::MouseEvent& event) { #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) // Move the hit test area for hovering up so that it is not overlapped by tab // hover cards when they are shown. - // TODO(crbug.com/978134): Once Linux/CrOS widget transparency is solved, + // TODO(crbug.com/41467565): Once Linux/CrOS widget transparency is solved, // remove that case. constexpr int kHoverCardOverlap = 6; if (event.location().y() >= height() - kHoverCardOverlap) { @@ -747,7 +755,8 @@ void Tab::GetAccessibleNodeData(ui::AXNodeData* node_data) { } } -gfx::Size Tab::CalculatePreferredSize() const { +gfx::Size Tab::CalculatePreferredSize( + const views::SizeBounds& available_size) const { return gfx::Size(tab_style()->GetStandardWidth(), GetLayoutConstant(TAB_HEIGHT)); } @@ -985,13 +994,14 @@ void Tab::SetTabNeedsAttention(bool attention) { SchedulePaint(); } -void Tab::SetFreezingVoteToken( - std::unique_ptr token) { - freezing_token_ = std::move(token); +void Tab::CreateFreezingVote(content::WebContents* contents) { + if (!freezing_vote_.has_value()) { + freezing_vote_.emplace(contents); + } } -void Tab::ReleaseFreezingVoteToken() { - freezing_token_.reset(); +void Tab::ReleaseFreezingVote() { + freezing_vote_.reset(); } // static @@ -1027,8 +1037,8 @@ void Tab::MaybeAdjustLeftForPinnedTab(gfx::Rect* bounds, const int pinned_width = tab_style()->GetPinnedWidth(); const int ideal_delta = width() - pinned_width; const int ideal_x = (pinned_width - visual_width) / 2; - // TODO(crbug.com/533570): This code is broken when the current width is less - // than the pinned width. + // TODO(crbug.com/40436434): This code is broken when the current width is + // less than the pinned width. bounds->set_x( bounds->x() + base::ClampRound( diff --git a/src/chrome/browser/ui/views/tabs/tab_strip.cc b/src/chrome/browser/ui/views/tabs/tab_strip.cc index e61e2490..5b9ecb81 100644 --- a/src/chrome/browser/ui/views/tabs/tab_strip.cc +++ b/src/chrome/browser/ui/views/tabs/tab_strip.cc @@ -14,11 +14,12 @@ #include #include +#include "base/check.h" +#include "base/command_line.h" #include "base/compiler_specific.h" #include "base/containers/adapters.h" #include "base/containers/contains.h" #include "base/containers/flat_map.h" -#include "base/command_line.h" #include "base/feature_list.h" #include "base/functional/bind.h" #include "base/functional/callback.h" @@ -29,6 +30,7 @@ #include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/metrics/user_metrics.h" +#include "base/not_fatal_until.h" #include "base/numerics/safe_conversions.h" #include "base/observer_list.h" #include "base/ranges/algorithm.h" @@ -169,7 +171,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext, // from us before our destructor is even called. ~TabDragContextImpl() override = default; - gfx::Size CalculatePreferredSize() const override { + gfx::Size CalculatePreferredSize( + const views::SizeBounds& available_size) const override { int max_child_x = 0; for (views::View* child : children()) { if (!views::IsViewClass(child)) { @@ -298,17 +301,21 @@ class TabStrip::TabDragContextImpl : public TabDragContext, } } - DCHECK(!dragging_views.empty()); - DCHECK(base::Contains(dragging_views, source)); + CHECK(!dragging_views.empty(), base::NotFatalUntil::M128) + << "Dragging views cannot be empty during drag initialization."; + CHECK(base::Contains(dragging_views, source), base::NotFatalUntil::M128) + << "Source must be part of dragging views."; // Delete the existing DragController before creating a new one. We do this // as creating the DragController remembers the WebContents delegates and we // need to make sure the existing DragController isn't still a delegate. drag_controller_.reset(); - DCHECK(event.type() == ui::ET_MOUSE_PRESSED || + CHECK((event.type() == ui::ET_MOUSE_PRESSED || event.type() == ui::ET_GESTURE_TAP_DOWN || - event.type() == ui::ET_GESTURE_SCROLL_BEGIN); + event.type() == ui::ET_GESTURE_SCROLL_BEGIN), + base::NotFatalUntil::M128) + << "Event type must be suitable for starting a drag."; drag_controller_ = std::make_unique(); @@ -400,8 +407,12 @@ class TabStrip::TabDragContextImpl : public TabDragContext, void OwnDragController( std::unique_ptr controller) override { - DCHECK(controller); - DCHECK(!drag_controller_); + CHECK(controller, base::NotFatalUntil::M128) + << "The provided TabDragController is null, which is not expected."; + + CHECK(!drag_controller_, base::NotFatalUntil::M128) + << "Attempting to own a new drag controller while one already exists."; + drag_controller_ = std::move(controller); if (drag_controller_set_callback_) { std::move(drag_controller_set_callback_).Run(drag_controller_.get()); @@ -505,9 +516,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext, const int first_dragged_tab_model_index = tab_strip_->GetModelIndexOf(dragged_views[group.has_value() ? 1 : 0]) .value(); - const int index = - CalculateInsertionIndex(dragged_bounds, first_dragged_tab_model_index, - num_dragged_tabs, std::move(group)); + const int index = CalculateInsertionIndex( + dragged_bounds, first_dragged_tab_model_index, num_dragged_tabs, group); const Tab* last_visible_tab = tab_strip_->GetLastVisibleTab(); int last_insertion_point = @@ -523,7 +533,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext, std::vector CalculateBoundsForDraggedViews( const std::vector>& views) override { - DCHECK(!views.empty()); + CHECK(!views.empty(), base::NotFatalUntil::M128) + << "The views vector must not be empty."; std::vector bounds; const int overlap = TabStyle::Get()->GetTabOverlap(); @@ -540,8 +551,11 @@ class TabStrip::TabDragContextImpl : public TabDragContext, void SetBoundsForDrag( const std::vector>& views, const std::vector& bounds) override { + CHECK(!views.empty() || !bounds.empty(), base::NotFatalUntil::M128) + << "Views and bounds cannot both be empty."; tab_strip_->tab_container_->CancelAnimation(); - DCHECK_EQ(views.size(), bounds.size()); + CHECK_EQ(views.size(), bounds.size(), base::NotFatalUntil::M128) + << "The sizes of views and bounds must match."; for (size_t i = 0; i < views.size(); ++i) { views[i]->SetBoundsRect(bounds[i]); } @@ -554,9 +568,9 @@ class TabStrip::TabDragContextImpl : public TabDragContext, // Reset the layout size as we've effectively laid out a different size. // This ensures a layout happens after the drag is done. tab_strip_->tab_container_->InvalidateIdealBounds(); - if (views.at(0)->group().has_value()) { + if (views[0]->group().has_value()) { tab_strip_->tab_container_->UpdateTabGroupVisuals( - views.at(0)->group().value()); + views[0]->group().value()); } } @@ -569,7 +583,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext, // No tabs should be dragging at this point. for (int i = 0; i < GetTabCount(); ++i) { - DCHECK(!GetTabAt(i)->dragging()); + CHECK(!GetTabAt(i)->dragging(), base::NotFatalUntil::M128) + << "A tab is still marked as dragging when starting a new drag."; } tab_strip_->tab_container_->CompleteAnimationAndLayout(); @@ -641,7 +656,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext, const gfx::Point& location, bool initial_drag) override { std::vector bounds = CalculateBoundsForDraggedViews(views); - DCHECK_EQ(views.size(), bounds.size()); + CHECK_EQ(views.size(), bounds.size(), base::NotFatalUntil::M128) + << "The sizes of views and bounds must match in LayoutDraggedViewsAt."; // The index of `source_view` in the TabStrip's viewmodel. std::optional source_view_model_index = GetIndexOf(source_view); @@ -825,7 +841,9 @@ class TabStrip::TabDragContextImpl : public TabDragContext, } } - CHECK_NE(min_distance_index, -1); + CHECK_NE(min_distance_index, -1) + << "No valid insertion index found for the dragged tab, which " + "indicates a potential logic error in tab placement calculations."; // When moving a tab within a tabstrip, the target index is expressed as if // the tabs are not in the tabstrip, i.e. it acts like the tabs are first @@ -997,7 +1015,8 @@ TabStrip::~TabStrip() { RemoveChildViewT(base::to_address(tab_container_)); RemoveChildViewT(base::to_address(drag_context_)); - CHECK(!IsInObserverList()); + CHECK(!IsInObserverList()) + << "TabStrip should not be in any observer lists at destruction."; } void TabStrip::SetAvailableWidthCallback( @@ -1067,7 +1086,8 @@ void TabStrip::UpdateLoadingAnimations(const base::TimeDelta& elapsed_time) { } void TabStrip::AddTabAt(int model_index, TabRendererData data) { - DCHECK(IsValidModelIndex(model_index)); + CHECK(IsValidModelIndex(model_index), base::NotFatalUntil::M128) + << "Attempted to add a tab with an invalid model index."; const bool pinned = data.pinned; Tab* tab = tab_container_->AddTab( @@ -1087,10 +1107,10 @@ void TabStrip::AddTabAt(int model_index, TabRendererData data) { observer.OnTabAdded(model_index); } - // At the start of AddTabAt() the model and tabs are out sync. Any queries to - // find a tab given a model index can go off the end of |tabs_|. As such, it - // is important that we complete the drag *after* adding the tab so that the - // model and tabstrip are in sync. + // At the start of AddTabAt() the model and tabs are out of sync. Any queries + // to find a tab given a model index can go off the end of |tabs_|. As such, + // it is important that we complete the drag *after* adding the tab so that + // the model and tabstrip are in sync. drag_context_->TabWasAdded(); Profile* profile = controller_->GetProfile(); @@ -1115,7 +1135,9 @@ void TabStrip::AddTabAt(int model_index, TabRendererData data) { void TabStrip::MoveTab(int from_model_index, int to_model_index, TabRendererData data) { - DCHECK_GT(GetTabCount(), 0); + CHECK_GT(GetTabCount(), 0, base::NotFatalUntil::M128) + << "The tab strip must contain at least one tab to perform a move " + "operation."; Tab* moving_tab = tab_at(from_model_index); moving_tab->SetData(std::move(data)); @@ -1135,7 +1157,10 @@ void TabStrip::RemoveTabAt(content::WebContents* contents, // OnTabWillBeRemoved should have ended any ongoing drags containing // `contents` already - unless the call is coming from inside the house! (i.e. // the TabDragController is doing the removing as part of reverting a drag) - DCHECK(drag_context_->CanRemoveTabIfDragging(contents)); + CHECK(drag_context_->CanRemoveTabIfDragging(contents), + base::NotFatalUntil::M128) + << "Attempted to remove a tab that could not be removed during drag."; + tab_container_->RemoveTab(model_index, was_active); UpdateHoverCard(nullptr, HoverCardUpdateType::kTabRemoved); @@ -1275,8 +1300,11 @@ bool TabStrip::ShouldDrawStrokes() const { } void TabStrip::SetSelection(const ui::ListSelectionModel& new_selection) { - DCHECK(new_selection.active().has_value()) + // This CHECK ensures there is always an active tab to maintain UI + // consistency. + CHECK(new_selection.active().has_value(), base::NotFatalUntil::M128) << "We should never transition to a state where no tab is active."; + Tab* const new_active_tab = tab_at(new_selection.active().value()); Tab* const old_active_tab = selected_tabs_.active().has_value() ? tab_at(selected_tabs_.active().value()) @@ -1355,7 +1383,7 @@ std::optional TabStrip::GetModelIndexOf(const TabSlotView* view) const { const std::optional viewmodel_index = tab_container_->GetModelIndexOf(view); - // TODO(1392523): The viewmodel (as accessed by + // TODO(crbug.com/40880410): The viewmodel (as accessed by // `tab_container_->GetModelIndexOf(Tab*)`) can be out of sync with the actual // TabStripModel when multiple tabs are closed at once. We can check // IsValidModelIndex to avoid crashes or out of bounds issues, but we can't @@ -1469,9 +1497,9 @@ bool TabStrip::IsAnimatingInTabStrip() const { void TabStrip::UpdateAnimationTarget(TabSlotView* tab_slot_view, gfx::Rect target_bounds) { - // TODO(1116121): This may need to do coordinate space transformations if the - // view hierarchy changes so `tab_container_` and `drag_context_` don't share - // spaces. + // TODO(crbug.com/40711732): This may need to do coordinate space + // transformations if the view hierarchy changes so `tab_container_` and + // `drag_context_` don't share spaces. drag_context_->UpdateAnimationTarget(tab_slot_view, target_bounds); } @@ -1638,7 +1666,7 @@ void TabStrip::ToggleTabGroupCollapsedState( // If tab count changed, all tab groups are collapsed and we have // created a new tab. We need to exit closing mode to resize the new // tab immediately. - // TODO(crbug/1384151): This should be captured along with the + // TODO(crbug.com/40878307): This should be captured along with the // ToggleTabGroup logic, so other callers to // TabStripController::ToggleTabGroupCollapsedState see the same // behavior. @@ -1700,9 +1728,11 @@ void TabStrip::MaybeStartDrag( // Check that the source is either a valid tab or a tab group header, which // are the only valid drag targets. - DCHECK(GetModelIndexOf(source).has_value() || - source->GetTabSlotViewType() == - TabSlotView::ViewType::kTabGroupHeader); + CHECK(GetModelIndexOf(source).has_value() || + source->GetTabSlotViewType() == + TabSlotView::ViewType::kTabGroupHeader, + base::NotFatalUntil::M128) + << "Drag source must be a valid tab or a tab group header."; drag_context_->MaybeStartDrag(source, event, original_selection); has_reported_tab_drag_metrics_ = false; @@ -1778,18 +1808,11 @@ void TabStrip::OnMouseEventInTab(views::View* source, void TabStrip::UpdateHoverCard(Tab* tab, HoverCardUpdateType update_type) { if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII("tab-hover-cards") == "tooltip" || - base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII("tab-hover-cards") == "none") return; - tab_container_->UpdateHoverCard(tab, update_type); -} - -bool TabStrip::ShowDomainInHoverCards() const { -#if BUILDFLAG(IS_CHROMEOS_ASH) - const auto* app_controller = GetBrowser()->app_controller(); - if (app_controller && app_controller->system_app()) { - return false; + base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII("tab-hover-cards") == "none") { + return; + } else { + tab_container_->UpdateHoverCard(tab, update_type); } -#endif - return true; } bool TabStrip::HoverCardIsShowingForTab(Tab* tab) { @@ -1896,9 +1919,8 @@ const Browser* TabStrip::GetBrowser() const { // TabStrip, views::View overrides: views::SizeBounds TabStrip::GetAvailableSize(const views::View* child) const { - // We can only reach here if SetAvailableWidthCallback() was never called, - // e.g. if tab scrolling is disabled. Defer to our parent. - DCHECK(child == base::to_address(tab_container_)); + CHECK(child == base::to_address(tab_container_), base::NotFatalUntil::M128) + << "The child view does not match the expected tab_container_ address."; return parent()->GetAvailableSize(this); } @@ -1911,11 +1933,12 @@ gfx::Size TabStrip::GetMinimumSize() const { return min_size; } -gfx::Size TabStrip::CalculatePreferredSize() const { +gfx::Size TabStrip::CalculatePreferredSize( + const views::SizeBounds& available_size) const { // `tab_container_` and `drag_context_` overlap (both share TabStrip's // origin), so we need to be able to cover the union of their bounds. - gfx::Size preferred_size = tab_container_->GetPreferredSize(); - preferred_size.SetToMax(drag_context_->GetPreferredSize()); + gfx::Size preferred_size = tab_container_->GetPreferredSize(available_size); + preferred_size.SetToMax(drag_context_->GetPreferredSize(available_size)); return preferred_size; } @@ -1939,12 +1962,14 @@ void TabStrip::Layout(PassKey) { } if (tab_container_->bounds() != GetLocalBounds()) { + UpdateHoverCard(nullptr, + TabSlotController::HoverCardUpdateType::kAnimating); tab_container_->SetBoundsRect(GetLocalBounds()); } else { // We still need to layout in this case, as the available width may have // changed, which can change layout outcomes (e.g. affecting tab // visibility). See https://crbug.com/1370459. - // TODO(crbug.com/1371301): TabContainer should observe available width + // TODO(crbug.com/40870361): TabContainer should observe available width // changes and invalidate its layout when needed. tab_container_->DeprecatedLayoutImmediately(); } @@ -2019,7 +2044,9 @@ void TabStrip::NewTabButtonPressed(const ui::Event& event) { if (ui::Clipboard::IsSupportedClipboardBuffer( ui::ClipboardBuffer::kSelection)) { ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread(); - CHECK(clipboard); + CHECK(clipboard) + << "Clipboard instance is not available, cannot proceed with " + "middle mouse button action."; std::u16string clipboard_text; clipboard->ReadText(ui::ClipboardBuffer::kSelection, /* data_dst = */ nullptr, &clipboard_text); @@ -2148,7 +2175,8 @@ void TabStrip::UpdateContrastRatioValues() { } void TabStrip::ShiftTabRelative(Tab* tab, int offset) { - DCHECK_EQ(1, std::abs(offset)); + CHECK_EQ(1, std::abs(offset), base::NotFatalUntil::M128) + << "Offset must be 1 or -1 to shift tab left or right."; const std::optional maybe_start_index = GetModelIndexOf(tab); if (!maybe_start_index.has_value()) { return; @@ -2218,7 +2246,8 @@ void TabStrip::ShiftTabRelative(Tab* tab, int offset) { void TabStrip::ShiftGroupRelative(const tab_groups::TabGroupId& group, int offset) { - DCHECK_EQ(1, std::abs(offset)); + CHECK_EQ(1, std::abs(offset), base::NotFatalUntil::M128) + << "Offset must be 1 or -1 to shift the group left or right."; gfx::Range tabs_in_group = controller_->ListTabsInGroup(group); const int start_index = tabs_in_group.start(); @@ -2238,7 +2267,8 @@ void TabStrip::ShiftGroupRelative(const tab_groups::TabGroupId& group, std::optional target_group = tab_at(index_of_skipped_over_tab)->group(); if (target_group.has_value()) { - CHECK_NE(target_group.value(), group); + CHECK_NE(target_group.value(), group) + << "The target group must be different from the current group to move."; } const int num_skipped_tabs = @@ -2275,7 +2305,8 @@ void TabStrip::TabContextMenuController::ShowContextMenuForViewImpl( ui::MenuSourceType source_type) { // We are only intended to be installed as a context-menu handler for tabs, so // this cast should be safe. - DCHECK(views::IsViewClass(source)); + CHECK(views::IsViewClass(source), base::NotFatalUntil::M128) + << "The source must be a Tab class."; Tab* const tab = static_cast(source); if (tab->closing()) { return; diff --git a/src/chrome/browser/ui/views/tabs/tab_style_views.cc b/src/chrome/browser/ui/views/tabs/tab_style_views.cc index 89e363f0..4fd60bff 100644 --- a/src/chrome/browser/ui/views/tabs/tab_style_views.cc +++ b/src/chrome/browser/ui/views/tabs/tab_style_views.cc @@ -7,11 +7,11 @@ #include #include +#include "base/command_line.h" #include "base/i18n/rtl.h" #include "base/memory/raw_ptr.h" #include "base/numerics/safe_conversions.h" #include "base/strings/strcat.h" -#include "base/command_line.h" #include "cc/paint/paint_record.h" #include "cc/paint/paint_shader.h" #include "chrome/browser/themes/theme_properties.h" @@ -997,7 +997,7 @@ void GM2TabStyleViews::PaintBackgroundHover(gfx::Canvas* canvas, TabStyle::TabSelectionState::kActive, /*hovered=*/false), hover_controller_->GetAlpha()); - // TODO(crbug/1308932): Remove FromColor and make all SkColor4f. + // TODO(crbug.com/40219248): Remove FromColor and make all SkColor4f. const SkColor4f colors[2] = { SkColor4f::FromColor(color), SkColor4f::FromColor(SkColorSetA(color, SK_AlphaTRANSPARENT))}; diff --git a/src/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc b/src/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc index ce8beb3e..e50a927d 100644 --- a/src/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc +++ b/src/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc @@ -21,6 +21,7 @@ #include "chrome/browser/ui/browser_otr_state.h" #include "chrome/browser/ui/color/chrome_color_id.h" #include "chrome/browser/ui/layout_constants.h" +#include "chrome/browser/ui/toolbar/app_menu_icon_controller.h" #include "chrome/browser/ui/toolbar/app_menu_model.h" #include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/views/chrome_layout_provider.h" @@ -146,7 +147,7 @@ void BrowserAppMenuButton::UpdateThemeBasedState() { // Outset focus ring should be present for the chip but not when only // the icon is visible. views::FocusRing::Get(this)->SetOutsetFocusRingDisabled( - IsLabelPresentAndVisible() ? false : true); + !IsLabelPresentAndVisible()); } } @@ -198,6 +199,9 @@ bool BrowserAppMenuButton::IsLabelPresentAndVisible() const { SkColor BrowserAppMenuButton::GetForegroundColor(ButtonState state) const { if (features::IsChromeRefresh2023() && IsLabelPresentAndVisible()) { const auto* const color_provider = GetColorProvider(); + if (type_and_severity_.use_primary_colors) { + return color_provider->GetColor(kColorAppMenuExpandedForegroundPrimary); + } return color_provider->GetColor(kColorAppMenuExpandedForegroundDefault); } @@ -230,30 +234,22 @@ void BrowserAppMenuButton::UpdateTextAndHighlightColor() { text = l10n_util::GetStringUTF16(message_id); #else text = l10n_util::GetStringUTF16(IDS_APP_MENU_BUTTON_UPDATE); +#endif + } else if (type_and_severity_.type == + AppMenuIconController::IconType::DEFAULT_BROWSER_PROMPT) { +#if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_CHROMEOS) + tooltip_message_id = IDS_APP_MENU_TOOLTIP_DEFAULT_PROMPT; + text = l10n_util::GetStringUTF16(IDS_APP_MENU_BUTTON_DEFAULT_PROMPT); +#else + tooltip_message_id = IDS_APPMENU_TOOLTIP; #endif } else { tooltip_message_id = IDS_APPMENU_TOOLTIP_ALERT; text = l10n_util::GetStringUTF16(IDS_APP_MENU_BUTTON_ERROR); } - std::optional color; - const auto* const color_provider = GetColorProvider(); - switch (type_and_severity_.severity) { - case AppMenuIconController::Severity::NONE: - break; - case AppMenuIconController::Severity::LOW: - color = color_provider->GetColor(kColorAppMenuHighlightSeverityLow); - break; - case AppMenuIconController::Severity::MEDIUM: - color = color_provider->GetColor(kColorAppMenuHighlightSeverityMedium); - break; - case AppMenuIconController::Severity::HIGH: - color = color_provider->GetColor(kColorAppMenuHighlightSeverityHigh); - break; - } - SetTooltipText(l10n_util::GetStringUTF16(tooltip_message_id)); - SetHighlight(text, color); + SetHighlight(text, GetHighlightColor()); } bool BrowserAppMenuButton::ShouldPaintBorder() const { @@ -275,11 +271,32 @@ void BrowserAppMenuButton::UpdateLayoutInsets() { std::optional BrowserAppMenuButton::GetHighlightTextColor() const { if (features::IsChromeRefresh2023() && IsLabelPresentAndVisible()) { const auto* const color_provider = GetColorProvider(); + if (type_and_severity_.use_primary_colors) { + return color_provider->GetColor(kColorAppMenuExpandedForegroundPrimary); + } return color_provider->GetColor(kColorAppMenuExpandedForegroundDefault); } return std::nullopt; } +std::optional BrowserAppMenuButton::GetHighlightColor() const { + const auto* const color_provider = GetColorProvider(); + if (features::IsChromeRefresh2023() && + type_and_severity_.use_primary_colors) { + return color_provider->GetColor(kColorAppMenuHighlightPrimary); + } + switch (type_and_severity_.severity) { + case AppMenuIconController::Severity::NONE: + return std::nullopt; + case AppMenuIconController::Severity::LOW: + return color_provider->GetColor(kColorAppMenuHighlightSeverityLow); + case AppMenuIconController::Severity::MEDIUM: + return color_provider->GetColor(kColorAppMenuHighlightSeverityMedium); + case AppMenuIconController::Severity::HIGH: + return color_provider->GetColor(kColorAppMenuHighlightSeverityHigh); + } +} + void BrowserAppMenuButton::OnTouchUiChanged() { UpdateColorsAndInsets(); PreferredSizeChanged(); diff --git a/src/chrome/browser/ui/views/toolbar/home_button.cc b/src/chrome/browser/ui/views/toolbar/home_button.cc index 01587d58..078839df 100644 --- a/src/chrome/browser/ui/views/toolbar/home_button.cc +++ b/src/chrome/browser/ui/views/toolbar/home_button.cc @@ -4,6 +4,8 @@ #include "chrome/browser/ui/views/toolbar/home_button.h" +#include + #include "base/command_line.h" #include "base/functional/bind.h" #include "base/functional/callback_helpers.h" @@ -82,7 +84,7 @@ void HomePageUndoBubble::Init() { l10n_util::GetStringUTF16(IDS_TOOLBAR_INFORM_SET_HOME_PAGE), undo_string}; views::StyledLabel* label = AddChildView(std::make_unique()); - label->SetText(base::JoinString(message, base::StringPiece16(u" "))); + label->SetText(base::JoinString(message, std::u16string_view(u" "))); gfx::Range undo_range(label->GetText().length() - undo_string.length(), label->GetText().length()); diff --git a/src/components/feed/core/shared_prefs/pref_names.cc b/src/components/feed/core/shared_prefs/pref_names.cc index 6ba69842..21dc66ae 100644 --- a/src/components/feed/core/shared_prefs/pref_names.cc +++ b/src/components/feed/core/shared_prefs/pref_names.cc @@ -19,7 +19,7 @@ const char kEnableSnippets[] = "ntp_snippets.enable"; const char kArticlesListVisible[] = "ntp_snippets.list_visible"; // A boolean pref set to true if swapping out NTP isn't enabled or if DSE is // Google. -// TODO(https://crbug.com/1483475): Inhibit loading feeds in native feeds code +// TODO(crbug.com/40282032): Inhibit loading feeds in native feeds code // when this pref is set to false. const char kEnableSnippetsByDse[] = "ntp_snippets_by_dse.enable"; diff --git a/src/components/history/core/browser/history_backend.cc b/src/components/history/core/browser/history_backend.cc index 751a3a85..7cf8413c 100644 --- a/src/components/history/core/browser/history_backend.cc +++ b/src/components/history/core/browser/history_backend.cc @@ -1213,6 +1213,8 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) { last_visit_id); } + delegate_->NotifyVisitedLinksAdded(request); + ScheduleCommit(); } @@ -1462,7 +1464,7 @@ std::pair HistoryBackend::AddPageVisit( visit_info.visited_link_id = visited_link_info.id; } - // TODO(crbug.com/1476511): any visit added via sync should not have a + // TODO(crbug.com/40280017): any visit added via sync should not have a // corresponding entry in the VisitedLinkDatabase. if (visit_source == VisitSource::SOURCE_SYNCED) { CHECK(visit_info.visited_link_id == kInvalidVisitedLinkID); @@ -1487,7 +1489,7 @@ std::pair HistoryBackend::AddPageVisit( return std::make_pair(url_id, visit_info.visit_id); } -// TODO(crbug.com/1475714): Determine if we want to record these URLs in the +// TODO(crbug.com/40279741): Determine if we want to record these URLs in the // VisitedLinkDatabase, and if so, plumb the correct value for top_level_site. void HistoryBackend::AddPagesWithDetails(const URLRows& urls, VisitSource visit_source) { @@ -1548,7 +1550,11 @@ void HistoryBackend::AddPagesWithDetails(const URLRows& urls, } bool HistoryBackend::IsExpiredVisitTime(const base::Time& time) const { - return time < expirer_.GetCurrentExpirationTime(); + if (base::CommandLine::ForCurrentProcess()->HasSwitch("keep-all-history")) { + return false; + } else { + return time < expirer_.GetCurrentExpirationTime(); + } } // static @@ -1690,6 +1696,17 @@ bool HistoryBackend::GetMostRecentVisitsForURL(URLID id, return false; } +QueryURLResult HistoryBackend::GetMostRecentVisitsForGurl(GURL url, + int max_visits) { + QueryURLResult result; + if (db_ && GetURL(url, &result.row) && + db_->GetMostRecentVisitsForURL(result.row.id(), max_visits, + &result.visits)) { + result.success = true; + } + return result; +} + bool HistoryBackend::GetForeignVisit(const std::string& originator_cache_guid, VisitID originator_visit_id, VisitRow* visit_row) { @@ -1826,7 +1843,7 @@ VisitID HistoryBackend::UpdateSyncedVisit( // existing row. It'll be updated below, if necessary. updated_row.segment_id = original_row.segment_id; - // TODO(crbug.com/1476511): any VisitedLinkID associated with `updated_row` + // TODO(crbug.com/40280017): any VisitedLinkID associated with `updated_row` // will be voided to avoid storing stale/incorrect VisitedLinkIDs once // elements of the VisitRow's partition key change (in this case the // referring_visit). @@ -1877,7 +1894,7 @@ bool HistoryBackend::UpdateVisitReferrerOpenerIDs(VisitID visit_id, row.referring_visit = referrer_id; row.opener_visit = opener_id; - // TODO(crbug.com/1476511): any VisitedLinkID associated with `row` + // TODO(crbug.com/40280017): any VisitedLinkID associated with `row` // will be voided to avoid storing stale/incorrect VisitedLinkIDs once // elements of the VisitRow's partition key change (in this case the // referring_visit). @@ -2103,6 +2120,13 @@ DomainsVisitedResult HistoryBackend::GetUniqueDomainsVisited( return db_->GetUniqueDomainsVisited(begin_time, end_time); } +GetAllAppIdsResult HistoryBackend::GetAllAppIds() { + if (!db_) { + return {}; + } + return db_->GetAllAppIds(); +} + HistoryLastVisitResult HistoryBackend::GetLastVisitToHost( const std::string& host, base::Time begin_time, @@ -2905,7 +2929,7 @@ void HistoryBackend::GetRedirectsFromSpecificVisit(VisitID cur_visit, visit_set.insert(cur_visit); while (db_->GetRedirectFromVisit(cur_visit, &cur_visit, &cur_url)) { if (visit_set.find(cur_visit) != visit_set.end()) { - NOTREACHED() << "Loop in visit chain, giving up"; + DUMP_WILL_BE_NOTREACHED_NORETURN() << "Loop in visit chain, giving up"; return; } visit_set.insert(cur_visit); @@ -2926,7 +2950,7 @@ void HistoryBackend::GetRedirectsToSpecificVisit(VisitID cur_visit, visit_set.insert(cur_visit); while (db_->GetRedirectToVisit(cur_visit, &cur_visit, &cur_url)) { if (visit_set.find(cur_visit) != visit_set.end()) { - NOTREACHED() << "Loop in visit chain, giving up"; + DUMP_WILL_BE_NOTREACHED_NORETURN() << "Loop in visit chain, giving up"; return; } visit_set.insert(cur_visit); @@ -2934,11 +2958,6 @@ void HistoryBackend::GetRedirectsToSpecificVisit(VisitID cur_visit, } } -void HistoryBackend::ScheduleAutocomplete( - base::OnceCallback callback) { - std::move(callback).Run(this, db_.get()); -} - void HistoryBackend::DeleteFTSIndexDatabases() { // Find files on disk matching the text databases file pattern so we can // quickly test for and delete them. @@ -3530,7 +3549,7 @@ void HistoryBackend::DatabaseErrorCallback(int error, sql::Statement* stmt) { // Don't just do the close/delete here, as we are being called by `db` and // that seems dangerous. - // TODO(https://crbug.com/854258): It is also dangerous to kill the database + // TODO(crbug.com/41395467): It is also dangerous to kill the database // by a posted task: tasks that run before KillHistoryDatabase still can try // to use the broken database. Consider protecting against other tasks using // the DB or consider changing KillHistoryDatabase() to use RazeAndClose() @@ -3594,6 +3613,11 @@ void HistoryBackend::ProcessDBTask( ProcessDBTaskImpl(); } +void HistoryBackend::RunDBTask( + base::OnceCallback callback) { + std::move(callback).Run(this, db_.get()); +} + void HistoryBackend::NotifyFaviconsChanged(const std::set& page_urls, const GURL& icon_url) { delegate_->NotifyFaviconsChanged(page_urls, icon_url); @@ -3617,7 +3641,7 @@ void HistoryBackend::NotifyURLsModified(const URLRows& changed_urls, delegate_->NotifyURLsModified(changed_urls); } -void HistoryBackend::NotifyURLsDeleted(DeletionInfo deletion_info) { +void HistoryBackend::NotifyDeletions(DeletionInfo deletion_info) { std::set origins; for (const history::URLRow& row : deletion_info.deleted_rows()) origins.insert(row.url().DeprecatedGetOriginAsURL()); @@ -3626,12 +3650,12 @@ void HistoryBackend::NotifyURLsDeleted(DeletionInfo deletion_info) { GetCountsAndLastVisitForOrigins(origins)); for (HistoryBackendObserver& observer : observers_) { - observer.OnURLsDeleted( + observer.OnHistoryDeletions( this, deletion_info.IsAllHistory(), deletion_info.is_from_expiration(), deletion_info.deleted_rows(), deletion_info.favicon_urls()); } - delegate_->NotifyURLsDeleted(std::move(deletion_info)); + delegate_->NotifyDeletions(std::move(deletion_info)); } void HistoryBackend::NotifyVisitUpdated(const VisitRow& visit, @@ -3641,10 +3665,23 @@ void HistoryBackend::NotifyVisitUpdated(const VisitRow& visit, } } -void HistoryBackend::NotifyVisitDeleted(const VisitRow& visit) { - tracker_.RemoveVisitById(visit.visit_id); - for (HistoryBackendObserver& observer : observers_) { - observer.OnVisitDeleted(visit); +void HistoryBackend::NotifyVisitsDeleted( + const std::vector& visits) { + std::vector links; + for (const DeletedVisit& visit : visits) { + tracker_.RemoveVisitById(visit.visit_row.visit_id); + for (HistoryBackendObserver& observer : observers_) { + observer.OnVisitDeleted(visit.visit_row); + } + // Determine if a VisitedLink was deleted as a result of the deleted Visit. + if (visit.deleted_visited_link.has_value()) { + links.push_back(visit.deleted_visited_link.value()); + } + } + // We want to avoid posting a new task for every VisitedLink deleted, so we + // notify the `delegate_` in a batch. + if (!links.empty()) { + delegate_->NotifyVisitedLinksDeleted(links); } } @@ -3703,7 +3740,7 @@ void HistoryBackend::DeleteAllHistory() { // Send out the notification that history is cleared. The in-memory database // will pick this up and clear itself. - NotifyURLsDeleted(DeletionInfo::ForAllHistory()); + NotifyDeletions(DeletionInfo::ForAllHistory()); } bool HistoryBackend::ClearAllFaviconHistory( diff --git a/src/components/user_education/common/feature_promo_controller.cc b/src/components/user_education/common/feature_promo_controller.cc index 3f687120..bb0378c4 100644 --- a/src/components/user_education/common/feature_promo_controller.cc +++ b/src/components/user_education/common/feature_promo_controller.cc @@ -4,6 +4,7 @@ #include "components/user_education/common/feature_promo_controller.h" +#include #include #include "base/auto_reset.h" @@ -14,12 +15,14 @@ #include "base/functional/callback_forward.h" #include "base/metrics/histogram_functions.h" #include "base/metrics/user_metrics.h" +#include "base/notimplemented.h" #include "build/build_config.h" #include "components/feature_engagement/public/feature_constants.h" #include "components/strings/grit/components_strings.h" #include "components/user_education/common/feature_promo_data.h" #include "components/user_education/common/feature_promo_lifecycle.h" #include "components/user_education/common/feature_promo_registry.h" +#include "components/user_education/common/feature_promo_result.h" #include "components/user_education/common/feature_promo_session_policy.h" #include "components/user_education/common/feature_promo_specification.h" #include "components/user_education/common/feature_promo_storage_service.h" @@ -95,9 +98,10 @@ FeaturePromoControllerCommon::~FeaturePromoControllerCommon() { } FeaturePromoResult FeaturePromoControllerCommon::CanShowPromo( - const base::Feature& iph_feature) const { - auto result = CanShowPromoCommon(iph_feature, false); - if (result && !feature_engagement_tracker_->WouldTriggerHelpUI(iph_feature)) { + const FeaturePromoParams& params) const { + auto result = CanShowPromoCommon(params, ShowSource::kNormal); + if (result && + !feature_engagement_tracker_->WouldTriggerHelpUI(*params.feature)) { result = FeaturePromoResult::kBlockedByConfig; } return result; @@ -105,19 +109,20 @@ FeaturePromoResult FeaturePromoControllerCommon::CanShowPromo( FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromo( FeaturePromoParams params) { - const char* feature_name = params.feature.get().name; - auto result = MaybeShowPromoCommon(std::move(params), /* for_demo =*/false); - auto failure = result.failure(); - if (failure.has_value()) { - RecordPromoNotShown(feature_name, failure.value()); - } - return result; + return MaybeShowPromoImpl(std::move(params), ShowSource::kNormal); } bool FeaturePromoControllerCommon::MaybeShowStartupPromo( FeaturePromoParams params) { const base::Feature* const iph_feature = ¶ms.feature.get(); + // No point in queueing a disabled feature. + if (!in_iph_demo_mode_ && !base::FeatureList::IsEnabled(*iph_feature)) { + RecordPromoNotShown(iph_feature->name, + FeaturePromoResult::kFeatureDisabled); + return false; + } + // If the promo is currently running, fail. if (GetCurrentPromoFeature() == iph_feature) { return false; @@ -134,10 +139,8 @@ bool FeaturePromoControllerCommon::MaybeShowStartupPromo( return false; } - queued_promos_.emplace_back( - iph_feature, - QueuedPromoData(std::move(params), - session_policy_->SpecificationToPromoInfo(*spec))); + queued_promos_.emplace_back(std::move(params), + session_policy_->SpecificationToPromoInfo(*spec)); // This will fire immediately if the tracker is initialized. feature_engagement_tracker_->AddOnInitializedCallback(base::BindOnce( @@ -152,38 +155,54 @@ bool FeaturePromoControllerCommon::MaybeShowStartupPromo( FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromoForDemoPage( FeaturePromoParams params) { - return MaybeShowPromoCommon(std::move(params), /* for_demo =*/true); + return MaybeShowPromoCommon(std::move(params), ShowSource::kDemo); +} + +FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromoImpl( + FeaturePromoParams params, + ShowSource source) { + const char* feature_name = params.feature.get().name; + auto result = MaybeShowPromoCommon(std::move(params), source); + auto failure = result.failure(); + if (failure.has_value()) { + RecordPromoNotShown(feature_name, failure.value()); + } + return result; } FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromoCommon( FeaturePromoParams params, - bool for_demo) { + ShowSource source) { // Perform common checks. - const FeaturePromoSpecification* spec = nullptr; + const FeaturePromoSpecification* primary_spec = nullptr; + const FeaturePromoSpecification* display_spec = nullptr; std::unique_ptr lifecycle = nullptr; ui::TrackedElement* anchor_element = nullptr; - auto result = CanShowPromoCommon(params.feature.get(), for_demo, &spec, + auto result = CanShowPromoCommon(params, source, &primary_spec, &display_spec, &lifecycle, &anchor_element); if (!result) { return result; } - CHECK(spec); + CHECK(primary_spec); + CHECK(display_spec); CHECK(lifecycle); CHECK(anchor_element); + const bool for_demo = source == ShowSource::kDemo; // If the session policy allows overriding the current promo, abort it. if (current_promo_) { EndPromo(*GetCurrentPromoFeature(), - FeaturePromoClosedReason::kOverrideForDemo); + for_demo ? FeaturePromoClosedReason::kOverrideForDemo + : FeaturePromoClosedReason::kOverrideForPrecedence); } // If the session policy allows overriding other help bubbles, close them. if (auto* const help_bubble = bubble_factory_registry_->GetHelpBubble(anchor_element->context())) { - help_bubble->Close(); + help_bubble->Close(HelpBubble::CloseReason::kProgrammaticallyClosed); } - // TODO(crbug.com/1258216): Currently this must be called before + // TODO(crbug.com/40200981): Currently this must be called before // ShouldTriggerHelpUI() below. See bug for details. const bool screen_reader_available = CheckScreenReaderPromptAvailable(for_demo || in_iph_demo_mode_); @@ -200,7 +219,7 @@ FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromoCommon( // Construct the parameters for the promotion. ShowPromoBubbleParams show_params; - show_params.spec = spec; + show_params.spec = display_spec; show_params.anchor_element = anchor_element; show_params.screen_reader_prompt_available = screen_reader_available; show_params.body_format = std::move(params.body_params); @@ -218,7 +237,7 @@ FeaturePromoResult FeaturePromoControllerCommon::MaybeShowPromoCommon( } // Update the most recent promo info. - last_promo_info_ = session_policy_->SpecificationToPromoInfo(*spec); + last_promo_info_ = session_policy_->SpecificationToPromoInfo(*primary_spec); session_policy_->NotifyPromoShown(last_promo_info_); bubble_closed_callback_ = std::move(params.close_callback); @@ -248,9 +267,10 @@ std::unique_ptr FeaturePromoControllerCommon::ShowCriticalPromo( EndPromo(*current, FeaturePromoClosedReason::kOverrideForPrecedence); } - // Snooze and tutorial are not supported for critical promos. - DCHECK_NE(FeaturePromoSpecification::PromoType::kSnooze, spec.promo_type()); - DCHECK_NE(FeaturePromoSpecification::PromoType::kTutorial, spec.promo_type()); + // Snooze, tutorial, and rotating are not supported for critical promos. + CHECK_NE(FeaturePromoSpecification::PromoType::kSnooze, spec.promo_type()); + CHECK_NE(FeaturePromoSpecification::PromoType::kTutorial, spec.promo_type()); + CHECK_NE(FeaturePromoSpecification::PromoType::kRotating, spec.promo_type()); ShowPromoBubbleParams show_params; show_params.spec = &spec; @@ -301,15 +321,15 @@ FeaturePromoControllerCommon::GetCurrentPromoSpecificationForAnchor( } bool FeaturePromoControllerCommon::HasPromoBeenDismissed( - const base::Feature& iph_feature, + const FeaturePromoParams& params, FeaturePromoClosedReason* last_close_reason) const { const FeaturePromoSpecification* const spec = - registry()->GetParamsForFeature(iph_feature); + registry()->GetParamsForFeature(*params.feature); if (!spec) { return false; } - const auto data = storage_service()->ReadPromoData(iph_feature); + const auto data = storage_service()->ReadPromoData(*params.feature); if (!data) { return false; } @@ -324,8 +344,11 @@ bool FeaturePromoControllerCommon::HasPromoBeenDismissed( case user_education::FeaturePromoSpecification::PromoSubtype:: kActionableAlert: return data->is_dismissed; - case user_education::FeaturePromoSpecification::PromoSubtype::kPerApp: - return base::Contains(data->shown_for_apps, GetAppId()); + case user_education::FeaturePromoSpecification::PromoSubtype::kKeyedNotice: + if (params.key.empty()) { + return false; + } + return base::Contains(data->shown_for_keys, params.key); } } @@ -337,7 +360,7 @@ bool FeaturePromoControllerCommon::EndPromo( auto close_reason_internal = end_promo_reason == EndFeaturePromoReason::kFeatureEngaged ? FeaturePromoClosedReason::kFeatureEngaged - : FeaturePromoClosedReason::kAbortPromo; + : FeaturePromoClosedReason::kAbortedByFeature; return EndPromo(iph_feature, close_reason_internal); } @@ -346,7 +369,7 @@ bool FeaturePromoControllerCommon::EndPromo( FeaturePromoClosedReason close_reason) { const auto it = FindQueuedPromo(iph_feature); if (it != queued_promos_.end()) { - auto& cb = it->second.params.queued_promo_callback; + auto& cb = it->params.queued_promo_callback; if (cb) { std::move(cb).Run(iph_feature, FeaturePromoResult::kCanceled); } @@ -370,7 +393,13 @@ void FeaturePromoControllerCommon::RecordPromoEnded( current_promo_->OnPromoEnded(close_reason, continue_after_close); if (!continue_after_close) { current_promo_.reset(); - MaybeShowQueuedPromo(); + // Try to show the next queued promo (if any) but only if the current promo + // was not ended by being overridden; in that case a different promo is + // already trying to show. + if (close_reason != FeaturePromoClosedReason::kOverrideForDemo && + close_reason != FeaturePromoClosedReason::kOverrideForPrecedence) { + MaybeShowQueuedPromo(); + } } } @@ -440,7 +469,7 @@ bool FeaturePromoControllerCommon::CheckScreenReaderPromptAvailable( return false; } - // TODO(crbug.com/1258216): Once we have our answer, immediately dismiss + // TODO(crbug.com/40200981): Once we have our answer, immediately dismiss // so that this doesn't interfere with actually showing the bubble. This // dismiss can be moved elsewhere once we support concurrency. feature_engagement_tracker_->Dismissed(*prompt_feature); @@ -481,26 +510,61 @@ FeaturePromoControllerCommon::GetNextQueuedPromo() { QueuedPromos::iterator result = queued_promos_.end(); for (auto it = queued_promos_.begin(); it != queued_promos_.end(); ++it) { if (result == queued_promos_.end() || - it->second.info.priority > result->second.info.priority) { + it->info.priority > result->info.priority) { result = it; } } return result; } +const FeaturePromoControllerCommon::QueuedPromoData* +FeaturePromoControllerCommon::GetNextQueuedPromo() const { + // Const cast is safe here because it does not modify the queue, and only a + // const pointer to the value found is returned. + const auto it = + const_cast(this)->GetNextQueuedPromo(); + return it != queued_promos_.end() ? &*it : nullptr; +} + void FeaturePromoControllerCommon::MaybeShowQueuedPromo() { // This should only ever be called after the tracker is initialized. CHECK(feature_engagement_tracker_->IsInitialized()); - // Fetch the next-highest-priority promo from the queue. + // If there is already a promo showing, it may be necessary to hold off trying + // to show another. + const std::optional current_promo = + (current_promo_ || critical_promo_bubble_) + ? std::make_optional(last_promo_info_) + : std::nullopt; + + // Also, if the next promo in queue cannot be shown and the current promo is + // not high-priority, any messaging priority must be released. + const bool must_release_on_failure = + !current_promo || current_promo->priority != + FeaturePromoSessionPolicy::PromoPriority::kHigh; + + // Fetch the next-highest-priority promo from the queue. If there's nothing, + // then there's nothing to do. const auto next = GetNextQueuedPromo(); if (next == queued_promos_.end()) { - messaging_priority_handle_.Release(); + if (must_release_on_failure) { + messaging_priority_handle_.Release(); + } return; } - const bool is_high_priority = next->second.info.priority == - FeaturePromoSessionPolicy::PromoPriority::kHigh; + // If there is already a promo showing and this promo would not override it, + // bail out. + if (current_promo && + !session_policy_->CanShowPromo(next->info, current_promo)) { + if (must_release_on_failure) { + messaging_priority_handle_.Release(); + } + return; + } + + const bool is_high_priority = + next->info.priority == FeaturePromoSessionPolicy::PromoPriority::kHigh; // Coordinate with the product messaging system to make sure a promo will not // attempt to be shown over a non-IPH legal notice. @@ -548,13 +612,14 @@ void FeaturePromoControllerCommon::MaybeShowQueuedPromo() { // Store the data that is needed to show the promo and then remove it from // the queue. - const base::Feature* const iph_feature = &next->second.params.feature.get(); - FeaturePromoParams params = std::move(next->second.params); + const base::Feature* const iph_feature = &next->params.feature.get(); + FeaturePromoParams params = std::move(next->params); queued_promos_.erase(next); QueuedPromoCallback callback = std::move(params.queued_promo_callback); // Try to start the promo, assuming the tracker was successfully initialized. - const FeaturePromoResult result = MaybeShowPromo(std::move(params)); + const FeaturePromoResult result = + MaybeShowPromoImpl(std::move(params), ShowSource::kQueue); if (callback) { std::move(callback).Run(*iph_feature, result); } @@ -569,9 +634,10 @@ void FeaturePromoControllerCommon::MaybeShowQueuedPromo() { // Returns whether `iph_feature` is queued to be shown. bool FeaturePromoControllerCommon::IsPromoQueued( const base::Feature& iph_feature) const { - return std::any_of( - queued_promos_.begin(), queued_promos_.end(), - [&iph_feature](const auto& pr) { return pr.first == &iph_feature; }); + return std::any_of(queued_promos_.begin(), queued_promos_.end(), + [&iph_feature](const QueuedPromoData& data) { + return &data.params.feature.get() == &iph_feature; + }); } // Returns an iterator into the queued promo list matching `iph_feature`, or @@ -579,38 +645,42 @@ bool FeaturePromoControllerCommon::IsPromoQueued( FeaturePromoControllerCommon::QueuedPromos::iterator FeaturePromoControllerCommon::FindQueuedPromo( const base::Feature& iph_feature) { - return std::find_if( - queued_promos_.begin(), queued_promos_.end(), - [&iph_feature](const auto& pr) { return pr.first == &iph_feature; }); + return std::find_if(queued_promos_.begin(), queued_promos_.end(), + [&iph_feature](const QueuedPromoData& data) { + return &data.params.feature.get() == &iph_feature; + }); } void FeaturePromoControllerCommon::FailQueuedPromos() { - for (auto& [feature, data] : queued_promos_) { + for (auto& data : queued_promos_) { auto& cb = data.params.queued_promo_callback; if (cb) { - std::move(cb).Run(*feature, FeaturePromoResult::kError); + std::move(cb).Run(*data.params.feature, FeaturePromoResult::kError); } } queued_promos_.clear(); } FeaturePromoResult FeaturePromoControllerCommon::CanShowPromoCommon( - const base::Feature& iph_feature, - bool for_demo, - const FeaturePromoSpecification** spec_out, + const FeaturePromoParams& params, + ShowSource source, + const FeaturePromoSpecification** primary_spec_out, + const FeaturePromoSpecification** display_spec_out, std::unique_ptr* lifecycle_out, ui::TrackedElement** anchor_element_out) const { + const bool for_demo = source == ShowSource::kDemo; + // Ensure that this promo isn't already queued for startup. // // Note that this check is bypassed if this is for an explicit demo, but not // in demo mode, as the IPH may be queued for startup specifically because it // is being demoed. - if (!for_demo && IsPromoQueued(iph_feature)) { + if (!for_demo && IsPromoQueued(*params.feature)) { return FeaturePromoResult::kBlockedByPromo; } const FeaturePromoSpecification* const spec = - registry()->GetParamsForFeature(iph_feature); + registry()->GetParamsForFeature(*params.feature); if (!spec) { return FeaturePromoResult::kError; } @@ -620,17 +690,16 @@ FeaturePromoResult FeaturePromoControllerCommon::CanShowPromoCommon( // Engagement tracker more times than necessary, emitting unnecessary logging // events when features are disabled. if (!for_demo && !in_iph_demo_mode_ && - !base::FeatureList::IsEnabled(iph_feature)) { + !base::FeatureList::IsEnabled(*params.feature)) { return FeaturePromoResult::kFeatureDisabled; } // Check the lifecycle, but only if not in demo mode. This is especially // important for snoozeable, app, and legal notice promos. - std::unique_ptr lifecycle; + auto lifecycle = std::make_unique( + storage_service_, params.key, &*params.feature, spec->promo_type(), + spec->promo_subtype(), spec->rotating_promos().size()); if (!for_demo && !in_iph_demo_mode_) { - lifecycle = std::make_unique( - storage_service_, GetAppId(), &iph_feature, spec->promo_type(), - spec->promo_subtype()); if (const auto result = lifecycle->CanShow(); !result) { return result; } @@ -646,11 +715,23 @@ FeaturePromoResult FeaturePromoControllerCommon::CanShowPromoCommon( // When not in demo mode, refer to the session policy to determine if the // promo can show. if (!for_demo && !in_iph_demo_mode_) { - const auto result = session_policy_->CanShowPromo( - session_policy_->SpecificationToPromoInfo(*spec), current_promo); + const auto promo_info = session_policy_->SpecificationToPromoInfo(*spec); + auto result = session_policy_->CanShowPromo(promo_info, current_promo); if (!result) { return result; } + + // If this is not from the queue, compare against queued promos as well. + if (source != ShowSource::kQueue) { + if (const auto* const queued = GetNextQueuedPromo()) { + // This is the opposite situation: only exclude this promo if the queued + // promo (which is not yet running) would cancel *this* promo. + result = session_policy_->CanShowPromo(queued->info, promo_info); + if (result) { + return FeaturePromoResult::kBlockedByPromo; + } + } + } } // Promos are blocked if some other critical user messaging is queued. @@ -659,9 +740,35 @@ FeaturePromoResult FeaturePromoControllerCommon::CanShowPromoCommon( return FeaturePromoResult::kBlockedByPromo; } + // For rotating promos, cycle forward to the next valid index. + auto* anchor_spec = spec; + if (spec->promo_type() == FeaturePromoSpecification::PromoType::kRotating) { + int current_index = lifecycle->GetPromoIndex(); + // In demos, when repeating the same repeating promo to test it, the index + // should cycle. However, the updated index is not written until the + // previous promo is ended, which happens later. In order to simulate this, + // base the starting index off the one being used by the previous promo. + if (current_promo_ && current_promo_->iph_feature() == &*params.feature) { + current_index = (current_promo_->GetPromoIndex() + 1) % + spec->rotating_promos().size(); + } + + // Find the next index in the rotation that has a valid promo. This is the + // actual index that will be used. + int index = current_index; + while (!spec->rotating_promos().at(index).has_value()) { + index = (index + 1) % spec->rotating_promos().size(); + CHECK_NE(index, current_index) + << "Wrapped around while looking for a valid rotating promo; this " + "should have been caught during promo registration."; + } + lifecycle->SetPromoIndex(index); + anchor_spec = &*spec->rotating_promos().at(index); + } + // Fetch the anchor element. For now, assume all elements are Views. ui::TrackedElement* const anchor_element = - spec->GetAnchorElement(GetAnchorContext()); + anchor_spec->GetAnchorElement(GetAnchorContext()); if (!anchor_element) { return FeaturePromoResult::kBlockedByUi; } @@ -673,20 +780,16 @@ FeaturePromoResult FeaturePromoControllerCommon::CanShowPromoCommon( // Output the lifecycle if it was requested. if (lifecycle_out) { - if (!lifecycle) { - // If in demo mode but the caller has asked for a lifecycle anyway, then - // provide one. - lifecycle = std::make_unique( - storage_service_, GetAppId(), &iph_feature, spec->promo_type(), - spec->promo_subtype()); - } *lifecycle_out = std::move(lifecycle); } // If the caller has asked for the specification or anchor element, then // provide them. - if (spec_out) { - *spec_out = spec; + if (primary_spec_out) { + *primary_spec_out = spec; + } + if (display_spec_out) { + *display_spec_out = anchor_spec; } if (anchor_element_out) { *anchor_element_out = anchor_element; @@ -740,6 +843,20 @@ std::unique_ptr FeaturePromoControllerCommon::ShowPromoBubbleImpl( } switch (spec.promo_type()) { + case FeaturePromoSpecification::PromoType::kToast: { + // Rotating toast promos require a "got it" button. + if (current_promo_ && + current_promo_->promo_type() == + FeaturePromoSpecification::PromoType::kRotating) { + bubble_params.buttons = CreateRotatingToastButtons(*spec.feature()); + // If no hint is set, promos with buttons take focus. However, toasts do + // not take focus by default. So if the hint isn't already set, set the + // promo not to take focus. + bubble_params.focus_on_show_hint = + bubble_params.focus_on_show_hint.value_or(false); + } + break; + } case FeaturePromoSpecification::PromoType::kSnooze: CHECK(spec.feature()); bubble_params.buttons = @@ -762,9 +879,10 @@ std::unique_ptr FeaturePromoControllerCommon::ShowPromoBubbleImpl( spec.custom_action_dismiss_string_id()); break; case FeaturePromoSpecification::PromoType::kUnspecified: - case FeaturePromoSpecification::PromoType::kToast: case FeaturePromoSpecification::PromoType::kLegacy: break; + case FeaturePromoSpecification::PromoType::kRotating: + NOTREACHED_NORETURN() << "Not implemented; should never reach this code."; } bool had_screen_reader_promo = false; @@ -779,9 +897,7 @@ std::unique_ptr FeaturePromoControllerCommon::ShowPromoBubbleImpl( auto help_bubble = bubble_factory_registry_->CreateHelpBubble( params.anchor_element, std::move(bubble_params)); if (help_bubble) { - // Record that the focus help message was actually read to the user. See the - // note in MaybeShowPromoImpl(). - // TODO(crbug.com/1258216): Rewrite this when we have the ability for FE + // TODO(crbug.com/40200981): Rewrite this when we have the ability for FE // promos to ignore other active promos. if (had_screen_reader_promo) { feature_engagement_tracker_->NotifyEvent( @@ -806,7 +922,9 @@ void FeaturePromoControllerCommon::FinishContinuedPromo( } } -void FeaturePromoControllerCommon::OnHelpBubbleClosed(HelpBubble* bubble) { +void FeaturePromoControllerCommon::OnHelpBubbleClosed( + HelpBubble* bubble, + HelpBubble::CloseReason reason) { // Since we're in the middle of processing callbacks we can't reset our // subscription but since it's a weak pointer (internally) and since we should // should only get called here once, it's not a big deal if we don't reset @@ -814,7 +932,7 @@ void FeaturePromoControllerCommon::OnHelpBubbleClosed(HelpBubble* bubble) { if (bubble == critical_promo_bubble_) { critical_promo_bubble_ = nullptr; } else if (bubble == promo_bubble()) { - if (current_promo_->OnPromoBubbleClosed()) { + if (current_promo_->OnPromoBubbleClosed(reason)) { current_promo_.reset(); } } @@ -919,6 +1037,14 @@ void FeaturePromoControllerCommon::OnTutorialAborted( } } +std::vector +FeaturePromoControllerCommon::CreateRotatingToastButtons( + const base::Feature& feature) { + // For now, use the same "got it" button as a snooze IPH that has run out of + // snoozes. + return CreateSnoozeButtons(feature, /*can_snooze=*/false); +} + std::vector FeaturePromoControllerCommon::CreateSnoozeButtons(const base::Feature& feature, bool can_snooze) { @@ -1095,10 +1221,29 @@ FeaturePromoControllerCommon::BlockActiveWindowCheckForTesting() { true); } -FeaturePromoParams::FeaturePromoParams(const base::Feature& iph_feature) - : feature(iph_feature) {} +FeaturePromoParams::FeaturePromoParams(const base::Feature& iph_feature, + const std::string& promo_key) + : feature(iph_feature), key(promo_key) {} FeaturePromoParams::FeaturePromoParams(FeaturePromoParams&& other) noexcept = default; FeaturePromoParams::~FeaturePromoParams() = default; +std::ostream& operator<<(std::ostream& os, FeaturePromoStatus status) { + switch (status) { + case FeaturePromoStatus::kBubbleShowing: + os << "kBubbleShowing"; + break; + case FeaturePromoStatus::kContinued: + os << "kContinued"; + break; + case FeaturePromoStatus::kNotRunning: + os << "kNotRunning"; + break; + case FeaturePromoStatus::kQueuedForStartup: + os << "kQueuedForStartup"; + break; + } + return os; +} + } // namespace user_education diff --git a/src/components/vector_icons/BUILD.gn b/src/components/vector_icons/BUILD.gn index 63d4f0df..65ad7a5a 100644 --- a/src/components/vector_icons/BUILD.gn +++ b/src/components/vector_icons/BUILD.gn @@ -12,6 +12,7 @@ aggregate_vector_icons("components_vector_icons") { sources = [ "${branding_path_component}/product.icon", + "${branding_path_component}/product_refresh.icon", "account_circle.icon", "account_circle_chrome_refresh.icon", "account_circle_off_chrome_refresh.icon", @@ -44,6 +45,7 @@ aggregate_vector_icons("components_vector_icons") { "cardboard.icon", "caret_down.icon", "caret_up.icon", + "cast.icon", "celebration.icon", "certificate.icon", "certificate_chrome_refresh.icon", @@ -70,7 +72,6 @@ aggregate_vector_icons("components_vector_icons") { "database_off.icon", "description.icon", "devices.icon", - "devices_chrome_refresh.icon", "devices_off.icon", "devices_off_chrome_refresh.icon", "document_scanner.icon", @@ -102,6 +103,7 @@ aggregate_vector_icons("components_vector_icons") { "font_download.icon", "font_download_chrome_refresh.icon", "font_download_off_chrome_refresh.icon", + "forward_10.icon", "forward_arrow.icon", "forward_arrow_chrome_refresh.icon", "google_color.icon", @@ -121,6 +123,8 @@ aggregate_vector_icons("components_vector_icons") { "info_refresh.icon", "insert_drive_file_outline.icon", "keyboard.icon", + "keyboard_lock.icon", + "keyboard_lock_off.icon", "launch.icon", "launch_chrome_refresh.icon", "launch_off_chrome_refresh.icon", @@ -178,6 +182,8 @@ aggregate_vector_icons("components_vector_icons") { "picture_in_picture_alt.icon", "play_arrow.icon", "play_arrow_chrome_refresh.icon", + "pointer_lock.icon", + "pointer_lock_off.icon", "printer.icon", "privacy_sandbox.icon", "protected_content.icon", @@ -188,6 +194,7 @@ aggregate_vector_icons("components_vector_icons") { "reload.icon", "reload_chrome_refresh.icon", "replay.icon", + "replay_10.icon", "save_cloud.icon", "screen_share.icon", "search.icon", @@ -207,6 +214,8 @@ aggregate_vector_icons("components_vector_icons") { "settings_outline.icon", "shopping_bag.icon", "shopping_bag_refresh.icon", + "skip_next.icon", + "skip_previous.icon", "sms.icon", "storage_access.icon", "storage_access_off.icon", @@ -225,6 +234,7 @@ aggregate_vector_icons("components_vector_icons") { "usb.icon", "usb_chrome_refresh.icon", "usb_off_chrome_refresh.icon", + "video_library.icon", "videocam.icon", "videocam_chrome_refresh.icon", "videocam_off.icon", @@ -267,6 +277,7 @@ aggregate_vector_icons("components_vector_icons") { "google_chrome/google_lens_full_logo.icon", "google_chrome/google_lens_full_logo_dark.icon", "google_chrome/google_lens_logo.icon", + "google_chrome/google_lens_monochrome_logo.icon", "google_chrome/google_password_manager.icon", "google_chrome/google_pay_logo.icon", "google_chrome/google_search_companion_logo.icon", diff --git a/src/content/common/gpu_pre_sandbox_hook_linux.cc b/src/content/common/gpu_pre_sandbox_hook_linux.cc index 9e59fd49..59a08ed0 100644 --- a/src/content/common/gpu_pre_sandbox_hook_linux.cc +++ b/src/content/common/gpu_pre_sandbox_hook_linux.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -84,6 +85,10 @@ inline bool UseV4L2Codec( #endif } +#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) +static const char kMaliConfPath[] = "/etc/mali_platform.conf"; +#endif + #if BUILDFLAG(IS_CHROMEOS) && defined(__aarch64__) static const char kLibGlesPath[] = "/usr/lib64/libGLESv2.so.2"; static const char kLibEglPath[] = "/usr/lib64/libEGL.so.1"; @@ -195,6 +200,13 @@ void AddArmMaliGpuPermissions(std::vector* permissions) { permissions->push_back(BrokerFilePermission::ReadWrite(kMali0Path)); +#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) + // Files needed for protected DMA allocations. + static const char kDmaHeapPath[] = "/dev/dma_heap/restricted_mtk_cma"; + permissions->push_back(BrokerFilePermission::ReadWrite(kDmaHeapPath)); + permissions->push_back(BrokerFilePermission::ReadOnly(kMaliConfPath)); +#endif + // Non-privileged render nodes for format enumeration. // https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#render-nodes base::FileEnumerator enumerator( @@ -250,7 +262,7 @@ void AddAmdGpuPermissions(std::vector* permissions) { "/usr/lib64/dri/r300_dri.so", "/usr/lib64/dri/r600_dri.so", "/usr/lib64/dri/radeonsi_dri.so", - // GPU Log Warning Workaround + // GPU Log Warnings Workaround "/usr/share/vulkan/icd.d", "/usr/share/vulkan/icd.d/radeon_icd.x86_64.json" "/etc/vulkan/icd.d", @@ -430,7 +442,7 @@ void AddVulkanICDPermissions(std::vector* permissions) { static const char* const kReadOnlyICDList[] = { "intel_icd.x86_64.json", "nvidia_icd.json", "radeon_icd.x86_64.json", - "mali_icd.json"}; + "mali_icd.json", "freedreno_icd.aarch64.json"}; for (std::string prefix : kReadOnlyICDPrefixes) { permissions->push_back(BrokerFilePermission::ReadOnly(prefix)); @@ -531,6 +543,16 @@ std::vector FilePermissionsForGpu( } void LoadArmGpuLibraries() { +#if BUILDFLAG(IS_CHROMEOS) && defined(ARCH_CPU_ARM_FAMILY) + // This environmental variable needs to be set before we load libMali if we + // want to instantiate protected Vulkan device queues. + static const char kMaliConfVar[] = "MALI_PLATFORM_CONFIG"; + // Note this function will only fail if we run out of memory entirely, in + // which case we would have much bigger problems, so we don't bother to check + // the return value. + setenv(kMaliConfVar, kMaliConfPath, 1); +#endif + // Preload the Mali library. if (UseChromecastSandboxAllowlist()) { for (const char* path : kAllowedChromecastPaths) { @@ -619,6 +641,7 @@ void LoadVulkanLibraries() { dlopen("libvulkan_radeon.so", dlopen_flag); dlopen("libvulkan_intel.so", dlopen_flag); dlopen("libGLX_nvidia.so.0", dlopen_flag); + dlopen("libvulkan_freedreno.so", dlopen_flag); } void LoadChromecastV4L2Libraries() { diff --git a/src/ui/base/ui_base_features.cc b/src/ui/base/ui_base_features.cc index a97f5d4c..73f737f9 100644 --- a/src/ui/base/ui_base_features.cc +++ b/src/ui/base/ui_base_features.cc @@ -53,6 +53,12 @@ BASE_FEATURE(kApplyNativeOcclusionToCompositor, #endif ); +// If enabled, native window occlusion tracking will always be used, even if +// CHROME_HEADLESS is set. +BASE_FEATURE(kAlwaysTrackNativeWindowOcclusionForTest, + "AlwaysTrackNativeWindowOcclusionForTest", + base::FEATURE_DISABLED_BY_DEFAULT); + // Field trial param name for `kApplyNativeOcclusionToCompositor`. const base::FeatureParam kApplyNativeOcclusionToCompositorType{ &kApplyNativeOcclusionToCompositor, "type", @@ -119,9 +125,9 @@ BASE_FEATURE(kSupportF11AndF12KeyShortcuts, base::FEATURE_ENABLED_BY_DEFAULT); bool AreF11AndF12ShortcutsEnabled() { - // TODO(crbug/1264581): Remove this once kDeviceI18nShortcutsEnabled policy is - // deprecated. This policy allows managed users to still be able to use - // deprecated legacy shortcuts which some enterprise customers rely on. + // TODO(crbug.com/40203434): Remove this once kDeviceI18nShortcutsEnabled + // policy is deprecated. This policy allows managed users to still be able to + // use deprecated legacy shortcuts which some enterprise customers rely on. if (::ui::ShortcutMappingPrefDelegate::IsInitialized()) { ::ui::ShortcutMappingPrefDelegate* instance = ::ui::ShortcutMappingPrefDelegate::GetInstance(); @@ -135,6 +141,17 @@ bool AreF11AndF12ShortcutsEnabled() { } #endif // BUILDFLAG(IS_CHROMEOS_ASH) +#if BUILDFLAG(IS_OZONE) +BASE_FEATURE(kOzoneBubblesUsePlatformWidgets, + "OzoneBubblesUsePlatformWidgets", +#if BUILDFLAG(IS_CHROMEOS_LACROS) + base::FEATURE_ENABLED_BY_DEFAULT +#else + base::FEATURE_DISABLED_BY_DEFAULT +#endif +); +#endif // BUILDFLAG(IS_OZONE) + // Update of the virtual keyboard settings UI as described in // https://crbug.com/876901. BASE_FEATURE(kInputMethodSettingsUiUpdate, @@ -177,7 +194,7 @@ BASE_FEATURE(kSystemCaptionStyle, // When enabled, the feature will query the OS for a default cursor size, // to be used in determining the concrete object size of a custom cursor in // blink. Currently enabled by default on Windows only. -// TODO(crbug.com/1333523) - Implement for other platforms. +// TODO(crbug.com/40845719) - Implement for other platforms. BASE_FEATURE(kSystemCursorSizeSupported, "SystemCursorSizeSupported", #if BUILDFLAG(IS_WIN) @@ -214,7 +231,7 @@ bool IsUiGpuRasterizationEnabled() { // Enables scrolling with layers under ui using the ui::Compositor. BASE_FEATURE(kUiCompositorScrollWithLayers, "UiCompositorScrollWithLayers", -// TODO(https://crbug.com/615948): Use composited scrolling on all platforms. +// TODO(crbug.com/40471184): Use composited scrolling on all platforms. #if BUILDFLAG(IS_APPLE) base::FEATURE_ENABLED_BY_DEFAULT #else @@ -226,7 +243,7 @@ BASE_FEATURE(kUiCompositorScrollWithLayers, // native apps on Windows. BASE_FEATURE(kExperimentalFlingAnimation, "ExperimentalFlingAnimation", -// TODO(crbug.com/1052397): Revisit the macro expression once build flag switch +// TODO(crbug.com/40118868): Revisit the macro expression once build flag switch // of lacros-chrome is complete. #if BUILDFLAG(IS_WIN) || \ (BUILDFLAG(IS_LINUX) && !BUILDFLAG(IS_CHROMEOS_ASH) && \ @@ -255,11 +272,6 @@ BASE_FEATURE(kFocusFollowsCursor, base::FEATURE_DISABLED_BY_DEFAULT); #if BUILDFLAG(IS_WIN) -// Enables InputPane API for controlling on screen keyboard. -BASE_FEATURE(kInputPaneOnScreenKeyboard, - "InputPaneOnScreenKeyboard", - base::FEATURE_ENABLED_BY_DEFAULT); - // Enables using WM_POINTER instead of WM_TOUCH for touch events. BASE_FEATURE(kPointerEventsForTouch, "PointerEventsForTouch", @@ -279,8 +291,8 @@ BASE_FEATURE(kImprovedKeyboardShortcuts, bool IsImprovedKeyboardShortcutsEnabled() { #if BUILDFLAG(IS_CHROMEOS_ASH) - // TODO(crbug/1264581): Remove this once kDeviceI18nShortcutsEnabled policy is - // deprecated. + // TODO(crbug.com/40203434): Remove this once kDeviceI18nShortcutsEnabled + // policy is deprecated. if (::ui::ShortcutMappingPrefDelegate::IsInitialized()) { ::ui::ShortcutMappingPrefDelegate* instance = ::ui::ShortcutMappingPrefDelegate::GetInstance(); @@ -367,12 +379,6 @@ bool IsNotificationGesturesUpdateEnabled() { return base::FeatureList::IsEnabled(kNotificationGesturesUpdate); } -#if BUILDFLAG(IS_CHROMEOS_ASH) -BASE_FEATURE(kHandwritingGesture, - "HandwritingGesture", - base::FEATURE_ENABLED_BY_DEFAULT); -#endif - BASE_FEATURE(kSynchronousPageFlipTesting, "SynchronousPageFlipTesting", base::FEATURE_ENABLED_BY_DEFAULT); @@ -461,10 +467,13 @@ bool IsVariableRefreshRateEnabled() { } // Special default case for devices with |kVariableRefreshRateDefaultEnabled| - // set. Requires |kVariableRefreshRateAvailable| to also be set. + // set. Requires |kVariableRefreshRateAvailable| to also be set. We also check + // if the FeatureList exists as it can be null during the ASSERT_DEATH + // handling. // TODO(b/310666603): Remove after VRR is enabled-by-default for all hardware. - if (!base::FeatureList::GetInstance()->IsFeatureOverridden( - kEnableVariableRefreshRate.name) && + if (!(base::FeatureList::GetInstance() && + base::FeatureList::GetInstance()->IsFeatureOverridden( + kEnableVariableRefreshRate.name)) && base::FeatureList::IsEnabled(kVariableRefreshRateDefaultEnabled) && base::FeatureList::IsEnabled(kVariableRefreshRateAvailable)) { return true; @@ -493,20 +502,6 @@ bool IsLacrosColorManagementEnabled() { return base::FeatureList::IsEnabled(kLacrosColorManagement); } -BASE_FEATURE(kCustomizeChromeSidePanel, - "CustomizeChromeSidePanel", - base::FEATURE_DISABLED_BY_DEFAULT); - -BASE_FEATURE(kCustomizeChromeSidePanelNoChromeRefresh2023, - "CustomizeChromeSidePanelNoChromeRefresh2023", - base::FEATURE_ENABLED_BY_DEFAULT); - -bool CustomizeChromeSupportsChromeRefresh2023() { - return base::FeatureList::IsEnabled(kCustomizeChromeSidePanel) && - !base::FeatureList::IsEnabled( - kCustomizeChromeSidePanelNoChromeRefresh2023); -} - BASE_FEATURE(kChromeRefresh2023, "ChromeRefresh2023", base::FEATURE_DISABLED_BY_DEFAULT); @@ -515,122 +510,20 @@ BASE_FEATURE(kChromeRefreshSecondary2023, "ChromeRefreshSecondary2023", base::FEATURE_DISABLED_BY_DEFAULT); -BASE_FEATURE(kChromeRefresh2023NTB, - "ChromeRefresh2023NTB", - base::FEATURE_DISABLED_BY_DEFAULT); - -const char kChromeRefresh2023NTBVariationKey[] = "Variation"; - -constexpr base::FeatureParam::Option - ChromeRefresh2023NTBVariationOption[] = { - {ChromeRefresh2023NTBVariation::kGM2Full, "GM2Full"}, - {ChromeRefresh2023NTBVariation::kGM3OldIconNoBackground, - "GM3OldIconNoBackground"}, - {ChromeRefresh2023NTBVariation::kGM3OldIconWithBackground, - "GM3OldIconWithBackground"}, - {ChromeRefresh2023NTBVariation::kGM3NewIconNoBackground, - "GM3NewIconNoBackground"}, - {ChromeRefresh2023NTBVariation::kGM3NewIconWithBackground, - "GM3NewIconWithBackground"}, - {ChromeRefresh2023NTBVariation::kNoChoice, "No Choice"}}; - -const base::FeatureParam - kChromeRefresh2023NTBValue(&kChromeRefresh2023NTB, - kChromeRefresh2023NTBVariationKey, - ChromeRefresh2023NTBVariation::kNoChoice, - &ChromeRefresh2023NTBVariationOption); - -ChromeRefresh2023NTBVariation GetChromeRefresh2023NTB() { - ChromeRefresh2023NTBVariation option = kChromeRefresh2023NTBValue.Get(); - if (option == ChromeRefresh2023NTBVariation::kNoChoice) { - if (!IsChromeRefresh2023()) { - return ChromeRefresh2023NTBVariation::kGM2Full; - } else { - return ChromeRefresh2023NTBVariation::kGM3NewIconNoBackground; - } - } - - return option; -} - -BASE_FEATURE(kChromeRefresh2023TopChromeFont, - "ChromeRefresh2023TopChromeFont", - base::FEATURE_DISABLED_BY_DEFAULT); - bool IsChromeRefresh2023() { - if (!CustomizeChromeSupportsChromeRefresh2023()) { - // Bail before checking any other feature flags so that associated studies - // don't get activated. - return false; - } return base::FeatureList::IsEnabled(kChromeRefresh2023) || base::FeatureList::IsEnabled(kChromeRefreshSecondary2023); } -BASE_FEATURE(kChromeWebuiRefresh2023, - "ChromeWebuiRefresh2023", - base::FEATURE_DISABLED_BY_DEFAULT); - bool IsChromeWebuiRefresh2023() { - if (!CustomizeChromeSupportsChromeRefresh2023()) { - // Bail before checking any other feature flags so that associated studies - // don't get activated. - return false; - } return IsChromeRefresh2023() && - (base::FeatureList::IsEnabled(kChromeWebuiRefresh2023) || - base::FeatureList::IsEnabled(kChromeRefreshSecondary2023)); -} - -constexpr base::FeatureParam::Option - kChromeRefresh2023LevelOption[] = {{ChromeRefresh2023Level::kLevel1, "1"}, - {ChromeRefresh2023Level::kLevel2, "2"}}; - -const base::FeatureParam kChromeRefresh2023Level( - &kChromeRefresh2023, - "level", - ChromeRefresh2023Level::kLevel2, - &kChromeRefresh2023LevelOption); - -ChromeRefresh2023Level GetChromeRefresh2023LevelUncached() { - if (!CustomizeChromeSupportsChromeRefresh2023()) { - // Bail before checking any other feature flags so that associated studies - // don't get activated. - return ChromeRefresh2023Level::kDisabled; - } - // For simplicity, the secondary field trial to enable chrome refresh will - // also enable the omnibox refresh. - if (base::FeatureList::IsEnabled(kChromeRefreshSecondary2023)) { - return ChromeRefresh2023Level::kLevel2; - } - - return IsChromeRefresh2023() ? kChromeRefresh2023Level.Get() - : ChromeRefresh2023Level::kDisabled; -} - -ChromeRefresh2023Level GetChromeRefresh2023Level() { - // Cached due to frequent calls for performance optimization. - // Please update `GetChromeRefresh2023LevelUncached()` for any changes. - static const ChromeRefresh2023Level level = - GetChromeRefresh2023LevelUncached(); - return level; + base::FeatureList::IsEnabled(kChromeRefreshSecondary2023); } BASE_FEATURE(kBubbleMetricsApi, "BubbleMetricsApi", base::FEATURE_DISABLED_BY_DEFAULT); -#if BUILDFLAG(IS_MAC) -// When enabled, images will be written to the system clipboard as both a TIFF -// and a PNG (as opposed to just a TIFF). This requires encoding the sanitized -// bitmap to a PNG on the UI thread on copy, which may cause jank. This matches -// the behavior of other platforms. -// TODO(https://crbug.com/1443646): Remove this flag eventually. -BASE_FEATURE(kMacClipboardWriteImageWithPng, - "MacClipboardWriteImageWithPng", - base::FEATURE_ENABLED_BY_DEFAULT); -#endif // BUILDFLAG(IS_MAC) - #if BUILDFLAG(IS_APPLE) // Font Smoothing was enabled by default prior to introducing this feature. // We want to experiment with disabling it to align with CR2023 designs. @@ -645,4 +538,7 @@ BASE_FEATURE(kUseGammaContrastRegistrySettings, base::FEATURE_ENABLED_BY_DEFAULT); #endif // BUILDFLAG(IS_WIN) +BASE_FEATURE(kBubbleFrameViewTitleIsHeading, + "BubbleFrameViewTitleIsHeading", + base::FEATURE_ENABLED_BY_DEFAULT); } // namespace features