[pull] master from chromium:master #7
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
-Adds app menu item for AutoDark. -Shows app menu item when both flag and global setting are enabled. -Menu item adds site setting exceptions to block sites from auto-darkening (or removes exceptions for sites that are currently blocked). Note: Uses placeholder string for app menu item and temp value for ContentSettingType. Screenshot: https://screenshot.googleplex.com/3gp8H3r5kV4KEE7.png Bug: 1248537 Change-Id: Icf04ee922923b0c0298b12e7c21e2dd65f0c758e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3140103 Reviewed-by: Wenyu Fu <wenyufu@chromium.org> Reviewed-by: Filip Gorski <fgorski@chromium.org> Reviewed-by: Christian Dullweber <dullweber@chromium.org> Reviewed-by: Gang Wu <gangwu@chromium.org> Commit-Queue: Neil Coronado <nemco@google.com> Cr-Commit-Position: refs/heads/main@{#922769}
From some of the DwCs, we found that the target & main frame SiteInstances are possibly in a different BrowsingInstance from the problematic proxy. This CL adds more crash keys to figure out what's going on, including exposing/tracking some more debugging-only values/ functions related to BFCache. Bug: 1243541 Change-Id: I63739a7751e177685468471605b7a2fcc078f9fc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168552 Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Alexander Timin <altimin@chromium.org> Cr-Commit-Position: refs/heads/main@{#922770}
If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/fuchsia-aemu-chromium-autoroll Please CC chrome-fuchsia-gardener@grotations.appspotmail.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Tbr: chrome-fuchsia-gardener@grotations.appspotmail.com Disable-Retries: true Change-Id: I123f49c85e4beb5bea5c0088841a133aa9c43055 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169976 Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922771}
https://android.googlesource.com/platform/external/perfetto.git/+log/a9118d769009..62ae508a04c8 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/perfetto-trace-processor-mac-chromium Please CC perfetto-bugs@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Tbr: perfetto-bugs@google.com Change-Id: Ice8f4d2d92d96cbafd8156b554559bc61fcbd333 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169827 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922772}
Polymer dom-repeat is not optimized with subproperties. This fixes the rendering issue with updating the app notification list. Bug: 1217174 Change-Id: Ida887b1908799c837d3c4dd2ccf04090f4e790ef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169823 Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org> Reviewed-by: Regan Hsu <hsuregan@chromium.org> Cr-Commit-Position: refs/heads/main@{#922773}
Roll Chrome Win64 PGO profile from chrome-win64-main-1631908776-0e1e348c8f0e5753c304f4ea2cdd53a5d4c2b56f.profdata to chrome-win64-main-1631927567-3c747e586d45ac5cc1b05f19cd6697b12697ad10.profdata If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/pgo-win64-chromium Please CC pgo-profile-sheriffs@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium main branch: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chrome.try:win64-chrome Tbr: pgo-profile-sheriffs@google.com Change-Id: I70fd0402f213a28f5d0f99f483f926cd3d68fe30 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169440 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922774}
This CL performs minimal-change moves of code from /chrome/browser/chromeos/extensions/quick_unlock_private to /chrome/browser/extensions/api/quick_unlock_private Motivation: Lacros will need chrome.quickUnlockPrivate to support the "Confirm your password" (or other quick unlock means) when trying to view / copy / edit passwords on chrome://settings/passwords. Bug: 1227546 Change-Id: I4fca253814a5b438d6a319763a4b019aab2f91ad Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3162419 Commit-Queue: Samuel Huang <huangs@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Reviewed-by: Alexander Alekseev <alemate@chromium.org> Cr-Commit-Position: refs/heads/main@{#922775}
Roll Chrome Win32 PGO profile from chrome-win32-main-1631908776-9bd1978d6895c4bfae3ac788ed55aff5a5c41ef4.profdata to chrome-win32-main-1631927567-5194c38cb36f15e24bbd691333c915fbb87828ec.profdata If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/pgo-win32-chromium Please CC pgo-profile-sheriffs@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium main branch: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chrome.try:win-chrome Tbr: pgo-profile-sheriffs@google.com Change-Id: Ic4dc1b8e5a4db10548ad689550735e3293435405 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170097 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922776}
Before (dark): http://shortn/_TWoyQj5Wra After (dark): http://shortn/_7fuc9EpeRV Before (light): http://shortn/_bv8tWZByKf After (light): http://shortn/_uTp2ysv6tZ Bug: 1250875 Change-Id: I878c7c534738be193cbb6b4a76310c3738eb6d5c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169967 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Commit-Queue: David Black <dmblack@google.com> Cr-Commit-Position: refs/heads/main@{#922777}
https://pdfium.googlesource.com/pdfium.git/+log/b8dca8956a07..007c5e55b2e1 2021-09-17 tsepez@chromium.org Remove unused methods CPWL_Wnd::GetFocused() and SetBackgroundColor() 2021-09-17 tsepez@chromium.org Remove unreachable return in CPWL_MsgControl::SetFocus(). 2021-09-17 tsepez@chromium.org Make CPWL_EditImpl::DrawEdit() a non-static method. 2021-09-17 tsepez@chromium.org Remove STL includes for files without std:: symbols 2021-09-17 vmiklos@collabora.co.uk fdrm: fix MSVC 2019 build If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/pdfium-autoroll Please CC pdfium-deps-rolls@chromium.org on the revert to ensure that a human is aware of the problem. To file a bug in PDFium: https://bugs.chromium.org/p/pdfium/issues/entry To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Bug: None Tbr: pdfium-deps-rolls@chromium.org Change-Id: I4f5abeb47cf5d7a61c437a5fcaceb02f4c9cce56 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169864 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922778}
Roll Chrome Mac PGO profile from chrome-mac-main-1631895234-51ef5b12740a944d441055d0c58b5dfcebfd2221.profdata to chrome-mac-main-1631923111-63c5602e99f5eda6e8619ce55f50053381e74764.profdata If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/pgo-mac-chromium Please CC pgo-profile-sheriffs@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium main branch: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chrome.try:mac-chrome Tbr: pgo-profile-sheriffs@google.com Change-Id: I1099648baa95a5e171550b71c8e07d036caa024b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170002 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922779}
If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/androidx-chromium Please CC mheikal@google.com,clank-library-failures@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Tbr: mheikal@google.com,clank-library-failures@google.com Change-Id: I36daa5531b339bb7e9e34c424b9f83be23585f6d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169241 Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922780}
Change-Id: Iac2e61c8f30c906e813d1f5c6a3a4b0e229a610c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169139 Auto-Submit: Ben Mason <benmason@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922781}
https://chromium.googlesource.com/devtools/devtools-frontend.git/+log/e4132180df00..0073a92aab7c 2021-09-18 devtools-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com Update DevTools DEPS. If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/devtools-frontend-chromium Please CC devtools-waterfall-sheriff-onduty@grotations.appspotmail.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Bug: None Tbr: devtools-waterfall-sheriff-onduty@grotations.appspotmail.com Change-Id: Ic7d3b8aaabee03b65c36c5f746cb185bab8885cc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169439 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922782}
https://chromium.googlesource.com/angle/angle.git/+log/89c555f16430..458389f249b0 2021-09-17 syoussefi@chromium.org Vulkan: Support Linux dma-bufs 2021-09-17 timvp@google.com Destroy all unreferenced Contexts resources during terminate() 2021-09-17 rafael.cintron@microsoft.com Fix memory leak in StreamProducerD3DTexture.validateD3DTexture 2021-09-17 jmadill@chromium.org Move JsonSerializer from libANGLE to common. 2021-09-17 jmadill@chromium.org Fix libpng warning. If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/angle-chromium-autoroll Please CC jonahr@google.com on the revert to ensure that a human is aware of the problem. To file a bug in ANGLE: https://bugs.chromium.org/p/angleproject/issues/entry To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win-asan;luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:linux-swangle-try-x64;luci.chromium.try:win-swangle-try-x86 Bug: chromium:1190892 Tbr: jonahr@google.com Test: Test: EGLContextSharingTestNoFixture.EglTerminateMultipleTimes Change-Id: I7d0e798c96f3b0081b8b361c3ec41388f2b112a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170096 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922783}
https://chrome-internal.googlesource.com/chrome/src-internal.git/+log/f05b4b7eea4e..cc48041bd648 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://skia-autoroll.corp.goog/r/src-internal-chromium-autoroll Please CC thomasanderson@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome Bug: chromium:1214825,chromium:1225326,chromium:1248224,chromium:1249980 Tbr: thomasanderson@google.com Change-Id: I95df6a0208dcec88a368d50d96390939f6630a48 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169939 Commit-Queue: chromium-internal-autoroll <chromium-internal-autoroll@skia-corp.google.com.iam.gserviceaccount.com> Bot-Commit: chromium-internal-autoroll <chromium-internal-autoroll@skia-corp.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922784}
BUG=b/199532589 TEST=None Change-Id: Id79d70de889ab74365f6f5ada1e31d468aee8f20 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169610 Reviewed-by: Muhammad Hasan Khan <mhasank@chromium.org> Commit-Queue: Yao Li <yaohuali@google.com> Cr-Commit-Position: refs/heads/main@{#922785}
A few browser tests are passing in the --force-enable-metrics-reporting flag. This flag makes Chromium builds behave like Google Chrome and send data to servers. This CL updates existing tests to no longer use the switch. Bug: 1166393 Change-Id: Ib902f06760e8a74a0c54c684a30be7ca2ac81480 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168713 Commit-Queue: Robert Ogden <robertogden@chromium.org> Reviewed-by: Robert Ogden <robertogden@chromium.org> Cr-Commit-Position: refs/heads/main@{#922786}
- Creates a new boolean property |isMultiPageScan_| that is used to mark a multi-page scan - This is now separate from the boolean property |multiPageScanChecked| which only marks if the checkbox is checked - This fixes an issue where switching the file type from PDF to PNG/JPG hides the checked checkbox but a multi-page scan would still incorrectly start. Bug: 1210271 Change-Id: Idcf6871dfd02b2c09f5eb8fb816cc21ef45be37e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169039 Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org> Commit-Queue: Gavin Williams <gavinwill@chromium.org> Cr-Commit-Position: refs/heads/main@{#922787}
Automatic update for 2021-09-18 UTC Change-Id: Iaf1db505671cafbc5ab5af718e392251ba4a5203 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170391 Commit-Queue: CT Log list updates bot <mdb.chrome-pki-metadata@google.com> Bot-Commit: CT Log list updates bot <mdb.chrome-pki-metadata@google.com> Cr-Commit-Position: refs/heads/main@{#922788}
If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/fuchsia-sdk-chromium-autoroll Please CC chrome-fuchsia-gardener@grotations.appspotmail.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chromium.try:fuchsia-arm64-cast;luci.chromium.try:fuchsia-deterministic-dbg;luci.chromium.try:fuchsia-x64-cast Tbr: chrome-fuchsia-gardener@grotations.appspotmail.com Disable-Retries: true Change-Id: I6f141ada22c1de978dd7edcb06b4268af4d8a0de Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170100 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922789}
This works by adding aria attributes in the html, such that: * When a questionnaire is added, a message is read aloud to notify the user. * When the "Send" button is hovered, the message is read aloud again to remind the user in case they didn't hear the first time. Bug: 1241169 Change-Id: I9e4b84c3cb2bd54d9a9447883bc5cecb9806549f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3166184 Commit-Queue: Sonny Sasaka <sonnysasaka@chromium.org> Reviewed-by: Akihiro Ota <akihiroota@chromium.org> Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org> Cr-Commit-Position: refs/heads/main@{#922790}
- Show "Hold to maximize" widget on snap-top phantom window and hide the widget once the snap-top phantom window turns into maximize phantom window. - NIT: Use |kDwellTime| for landscape maximize instead of |kDwellLongTime| even if vertical snap feature is enabled. Bug: 1236294 Test: Manual Change-Id: I8d60587bd33cbed9ecba7c81733db3a888b75cc9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3161677 Commit-Queue: Cattalyya Nuengsigkapian <cattalyya@chromium.org> Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/heads/main@{#922791}
https://dawn.googlesource.com/dawn.git/+log/a6394ddc07c6..39046f36d7f3 2021-09-17 kainino@chromium.org Tweak JavaScript formatting of compressed formats 2021-09-17 kainino@chromium.org Add new items for Emscripten/webgpu-headers 2021-09-17 kainino@chromium.org Generate multiple variants of webgpu.h header 2021-09-17 tangm@microsoft.com D3D12: Move transform/FirstIndexOffset outside of TranslateToHLSL 2021-09-17 kainino@chromium.org Stub out depth16unorm texture format 2021-09-17 enga@chromium.org Add an option for the Adapter to use tiered limits 2021-09-17 cwallez@chromium.org Remove readonly storage textures If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/dawn-chromium-autoroll Please CC shrekshao@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Dawn: https://bugs.chromium.org/p/dawn/issues/entry To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Cq-Include-Trybots: luci.chromium.try:dawn-linux-x64-deps-rel;luci.chromium.try:dawn-mac-x64-deps-rel;luci.chromium.try:dawn-win10-x64-deps-rel;luci.chromium.try:dawn-win10-x86-deps-rel Bug: None Tbr: shrekshao@google.com Change-Id: I543574b937cdbe5cc87c5ad1b055e50d8be76770 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169876 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922792}
The test is only flaking on arm64 debug builds Bug: 1249304 Change-Id: I6ea92051409100a3156e9e628406af896a1223c3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169950 Auto-Submit: Chong Gu <chonggu@google.com> Reviewed-by: Victor Costan <pwnall@chromium.org> Commit-Queue: Victor Costan <pwnall@chromium.org> Cr-Commit-Position: refs/heads/main@{#922793}
This patch handles null values for callbacks of PlatformServiceBridge. Callback argument for methods like `queryMetricsSetting` or `querySafeBrowsingUserConsent` are Boolean object value. It could be null and may cause NE if it is casted to primitive boolean value directly. So use `Boolean.TRUE.equals()` for this to avoid subtle null values. Bug: 1025431 Change-Id: If6f782be0e2a60402477b004b273d539592b968a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3159766 Reviewed-by: Hazem Ashmawy <hazems@chromium.org> Reviewed-by: Nate Fischer <ntfschr@chromium.org> Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com> Cr-Commit-Position: refs/heads/main@{#922794}
This CL extends the test coverage of FirstRunIntegrationTest by allowing tests with all promos enabled or disabled (today, only the enabled cases for Data Saver and Sync promos are covered), and more tests for onBackPressed during FRE. This refactoring is part of a larger goal to skip FRE promos disabled by cloud policy when policies are received before a promo is shown. A follow-up CL will change FirstRunActivity to react to changes in preferences due to cloud policies being set after FR started, and integration tests will need to cover more cases as well. Because of that, I decided to send this CL separately to make review easier. Other than the additional tests, this CL also mock settings for Sign-in/Sync enabled and Data Saver enabled in FirstRunIntegrationTest, so we can test all the possible combinations. Tests for the new sign-in flow on FRE will be added later. Please notice that we preferred not to use parametrized tests because not all tests on this file make sense to be run with multiple configurations. Bug: 1203435 Change-Id: If2423e288f9f87e145aa99afece8ee5be99c7de2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081379 Commit-Queue: Jan Krcal <jkrcal@chromium.org> Auto-Submit: Fabio Tirelo <ftirelo@chromium.org> Reviewed-by: Jan Krcal <jkrcal@chromium.org> Reviewed-by: Sky Malice <skym@chromium.org> Reviewed-by: Wenyu Fu <wenyufu@chromium.org> Cr-Commit-Position: refs/heads/main@{#922795}
https://android.googlesource.com/platform/external/perfetto.git/+log/a9118d769009..62ae508a04c8 If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/perfetto-trace-processor-win-chromium Please CC perfetto-bugs@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md Tbr: perfetto-bugs@google.com Change-Id: I016e993e920fa9d07f6c3be577443f2cf31437f7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169826 Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#922796}
This CL fixes the issue that the policy cellular shill configuration got removed when user logs out. This CL contains two changes: 1. Remove the NetworkTechnology property as the identifying property for cellular when comparing two Shill properties. 2. Append the Iccid and Eid from old shill entry to avoid creating cellular service with empty Iccid and Eid when device got boot up. Bug: 1231305 Change-Id: Ie8d495bb0b88c27477b6c943cfd52197ba228878 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3166181 Commit-Queue: Jason Zhang <jiajunz@google.com> Reviewed-by: Azeem Arshad <azeemarshad@chromium.org> Cr-Commit-Position: refs/heads/main@{#922797}
Move and rename will only be supported for FileSystemFileHandles. See https://crbug.com/1250534 for rationale. Bug: 1140805, 1250534, 1250536 Change-Id: Ied6d5d69d8a0a1e05b4e1625cb54cfbe5bdb695c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3167575 Commit-Queue: Austin Sullivan <asully@chromium.org> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/main@{#922798}
pull bot
pushed a commit
that referenced
this pull request
Jun 30, 2023
This reverts commit 32257e5. Reason for revert: bot failures, e.g.: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-lacros-asan-lsan-rel/5477/test-results?sortby=&groupby= [ RUN ] IndexedDBLevelDBCodingTest.DecodeSortableWithCorruption FATAL content_unittests[168632:168632]: [indexed_db_leveldb_coding.cc(697)] Check failed: !key_arrays.empty(). #0 0x557f3bbe5f35 in backtrace /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4411:13 #1 0x557f4fce4862 in base::debug::CollectStackTrace(void**, unsigned long) ./../../base/debug/stack_trace_posix.cc:1001:7 #2 0x557f4fc9f313 in StackTrace ./../../base/debug/stack_trace.cc:221:12 #3 0x557f4fc9f313 in base::debug::StackTrace::StackTrace() ./../../base/debug/stack_trace.cc:218:28 #4 0x557f4f9eebd3 in logging::LogMessage::~LogMessage() ./../../base/logging.cc:730:29 #5 0x557f4f9a3df5 in ~DCheckLogMessage ./../../base/check.cc:91:3 #6 0x557f4f9a3df5 in logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() ./../../base/check.cc:87:32 #7 0x557f4f9a39a0 in logging::NotReachedError::~NotReachedError() ./../../base/check.cc:268:3 #8 0x557f4d49baaf in content::DecodeSortableIDBKey(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, blink::IndexedDBKey*) ./../../content/browser/indexed_db/indexed_db_leveldb_coding.cc:697:9 #9 0x557f3d3e2cef in content::(anonymous namespace)::IndexedDBLevelDBCodingTest_DecodeSortableWithCorruption_Test::TestBody() Original change's description: > IndexedDB: add deserialization code for sortable IDB keys > > Bug: 1409618 > Change-Id: I43fe51e77c1da43e707430cacaf8debdeacb7a65 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4651525 > Commit-Queue: Evan Stade <estade@chromium.org> > Reviewed-by: Austin Sullivan <asully@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1164358} Bug: 1409618 Change-Id: I5cdf7f83b6d60bcad20208cc22caca86a848d400 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4655982 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Frank Liberato <liberato@google.com> Commit-Queue: Frank Liberato <liberato@chromium.org> Auto-Submit: Frank Liberato <liberato@chromium.org> Cr-Commit-Position: refs/heads/main@{#1164434}
pull bot
pushed a commit
that referenced
this pull request
Aug 4, 2023
This reverts commit 7e52434. Reason for revert: the new test is failing, and is likely causing other tests to fail as well. See: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/37593/test-results?sortby=&groupby= Sample failure: [ RUN ] TimeOfDayTest.ReturnsNullTimeWhenLocalTimeFails ==324697==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5632f6dd4ee3 in base::Time::Exploded::HasValidValues() const ./../../base/time/time.cc:302:23 #1 0x5632f61d489d in ash::TimeOfDay::ToTimeToday() const ./../../ash/system/time/time_of_day.cc:54:12 #2 0x5632ea366c82 in ash::(anonymous namespace)::TimeOfDayTest_ReturnsNullTimeWhenLocalTimeFails_Test::TestBody() ./../../ash/system/time/time_of_day_unittest.cc:98:3 #3 0x5632f496278c in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 #4 0x5632f496278c in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2670:5 #5 0x5632f49650b3 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2849:11 #6 0x5632f4967144 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:3008:30 #7 0x5632f4994c10 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5866:44 #8 0x5632f4993ce2 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 #9 0x5632f4993ce2 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5440:10 #10 0x5632f6f60f30 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2284:73 #11 0x5632f6f60f30 in base::TestSuite::Run() ./../../base/test/test_suite.cc:461:16 #12 0x5632f6f6c137 in Run ./../../base/functional/callback.h:152:12 #13 0x5632f6f6c137 in RunTestSuite ./../../base/test/launcher/unit_test_launcher.cc:179:38 #14 0x5632f6f6c137 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, unsigned long, bool, base::RepeatingCallback<void ()>, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:240:10 #15 0x5632f6f6b824 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>, unsigned long) ./../../base/test/launcher/unit_test_launcher.cc:288:10 #16 0x5632f68395ae in main ./../../ash/test/ash_unittests.cc:29:10 #17 0x7f1f985cd082 in __libc_start_main ??:0:0 #18 0x5632e637b029 in _start ??:0:0 Uninitialized value was created by an allocation of 'now' in the stack frame #0 0x5632f61d4706 in ash::TimeOfDay::ToTimeToday() const ./../../ash/system/time/time_of_day.cc:48:3 Other tests seem to be failing for the same reasons, e.g. ScheduledFeatureTest.HandlesLocalTimeFailuresSunsetToSunrise Original change's description: > Explicitly check when local time conversion fails. > > base::Time::LocalExplode() can fail. The API says to check > base::Time::Exploded::HasValidValues() to detect this. > > This functionally is probably the same as before because we > were passing the exploded output to base::Time::FromLocalExploded() > immediately after, and presumably, that would fail if the exploded > input had invalid values. But it's clearer to the reader if this > is explicitly checked first. > > A unit test has also been added to TimeOfDay. > > Bug: b:294436942 > Change-Id: I751da660df4c89a9c545d465dfb5361dfb2acc64 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4750820 > Reviewed-by: Jiaming Cheng <jiamingc@chromium.org> > Commit-Queue: Eric Sum <esum@google.com> > Cr-Commit-Position: refs/heads/main@{#1179335} Bug: b:294436942 Change-Id: Iee92b34c51d4b202b6018dd1c62ddd4c4f76eaf6 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4748880 Commit-Queue: David Roger <droger@chromium.org> Owners-Override: David Roger <droger@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#1179492}
pull bot
pushed a commit
that referenced
this pull request
Aug 4, 2023
… LogMessage." This reverts commit 07a88f3. Reason for revert: breaks MSAN bot https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/37602/test-results [ RUN ] CStringBuilderTestPA.Char ==313264==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x563432178a89 in partition_alloc::internal::base::strings::CStringBuilder::c_str() ./../../base/allocator/partition_allocator/partition_alloc_base/strings/cstring_builder.cc:145:7 #1 0x563431d9652c in partition_alloc::internal::base::strings::CStringBuilderTestPA_Char_Test::TestBody() ./../../base/allocator/partition_allocator/partition_alloc_base/strings/cstring_builder_pa_unittest.cc:33:3 #2 0x5634322101ba in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 #3 0x5634322101ba in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2670:5 #4 0x563432212ae1 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2849:11 #5 0x563432214b72 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:3008:30 #6 0x563432242552 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5866:44 #7 0x563432241624 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 #8 0x563432241624 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5440:10 #9 0x5634329698b0 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2284:73 #10 0x5634329698b0 in base::TestSuite::Run() ./../../base/test/test_suite.cc:461:16 #11 0x5634329cc377 in Run ./../../base/functional/callback.h:152:12 #12 0x5634329cc377 in RunTestSuite ./../../base/test/launcher/unit_test_launcher.cc:179:38 #13 0x5634329cc377 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, unsigned long, bool, base::RepeatingCallback<void ()>, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:240:10 #14 0x5634329cba64 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>, unsigned long) ./../../base/test/launcher/unit_test_launcher.cc:288:10 #15 0x5634328f3de4 in main ./../../base/test/run_all_unittests.cc:70:10 #16 0x7f308e293082 in __libc_start_main ??:0:0 #17 0x56342e6a1349 in _start ??:0:0 Uninitialized value was created by an allocation of 'builder' in the stack frame #0 0x563431d96365 in partition_alloc::internal::base::strings::CStringBuilderTestPA_Char_Test::TestBody() ./../../base/allocator/partition_allocator/partition_alloc_base/strings/cstring_builder_pa_unittest.cc:31:3 SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/base_unittests+0x43f5a89) (BuildId: 039152aa25034492) Exiting Original change's description: > [PA] Add CStringBuilder to replace std::ostringstream used by LogMessage. > > Since std::ostringstream allocates and deallocates memory from heap, c.f. https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:buildtools/third_party/libc++/trunk/src/ios.cpp > > std::ostringstream is not available inside memory allocation. Instead > add CStringBuilder (not resize, fixed buffer size) for LogMessage. > > Change-Id: I8051978487acc5fc9b976d6085909b43f81d9d0d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4744311 > Reviewed-by: Yuki Shiino <yukishiino@chromium.org> > Commit-Queue: Takashi Sakamoto <tasak@google.com> > Cr-Commit-Position: refs/heads/main@{#1179481} Change-Id: Idb29e3252d9fe67955c0ae25ab46640ebfdae336 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4748189 Auto-Submit: David Roger <droger@chromium.org> Owners-Override: David Roger <droger@chromium.org> Commit-Queue: David Roger <droger@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#1179539}
pull bot
pushed a commit
that referenced
this pull request
Aug 9, 2023
This reverts commit 9888813. Reason for revert: Roll forward with msan fix. Original change's description: > Revert "Explicitly check when local time conversion fails." > > This reverts commit 7e52434. > > Reason for revert: the new test is failing, and is likely causing other tests to fail as well. > > See: > https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/37593/test-results?sortby=&groupby= > > Sample failure: > > [ RUN ] TimeOfDayTest.ReturnsNullTimeWhenLocalTimeFails > ==324697==WARNING: MemorySanitizer: use-of-uninitialized-value > #0 0x5632f6dd4ee3 in base::Time::Exploded::HasValidValues() const ./../../base/time/time.cc:302:23 > #1 0x5632f61d489d in ash::TimeOfDay::ToTimeToday() const ./../../ash/system/time/time_of_day.cc:54:12 > #2 0x5632ea366c82 in ash::(anonymous namespace)::TimeOfDayTest_ReturnsNullTimeWhenLocalTimeFails_Test::TestBody() ./../../ash/system/time/time_of_day_unittest.cc:98:3 > #3 0x5632f496278c in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 > #4 0x5632f496278c in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2670:5 > #5 0x5632f49650b3 in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2849:11 > #6 0x5632f4967144 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:3008:30 > #7 0x5632f4994c10 in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5866:44 > #8 0x5632f4993ce2 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10 > #9 0x5632f4993ce2 in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5440:10 > #10 0x5632f6f60f30 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2284:73 > #11 0x5632f6f60f30 in base::TestSuite::Run() ./../../base/test/test_suite.cc:461:16 > #12 0x5632f6f6c137 in Run ./../../base/functional/callback.h:152:12 > #13 0x5632f6f6c137 in RunTestSuite ./../../base/test/launcher/unit_test_launcher.cc:179:38 > #14 0x5632f6f6c137 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, unsigned long, bool, base::RepeatingCallback<void ()>, base::OnceCallback<void ()>) ./../../base/test/launcher/unit_test_launcher.cc:240:10 > #15 0x5632f6f6b824 in base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>, unsigned long) ./../../base/test/launcher/unit_test_launcher.cc:288:10 > #16 0x5632f68395ae in main ./../../ash/test/ash_unittests.cc:29:10 > #17 0x7f1f985cd082 in __libc_start_main ??:0:0 > #18 0x5632e637b029 in _start ??:0:0 > > Uninitialized value was created by an allocation of 'now' in the stack frame > #0 0x5632f61d4706 in ash::TimeOfDay::ToTimeToday() const ./../../ash/system/time/time_of_day.cc:48:3 > > > Other tests seem to be failing for the same reasons, e.g. > ScheduledFeatureTest.HandlesLocalTimeFailuresSunsetToSunrise > > Original change's description: > > Explicitly check when local time conversion fails. > > > > base::Time::LocalExplode() can fail. The API says to check > > base::Time::Exploded::HasValidValues() to detect this. > > > > This functionally is probably the same as before because we > > were passing the exploded output to base::Time::FromLocalExploded() > > immediately after, and presumably, that would fail if the exploded > > input had invalid values. But it's clearer to the reader if this > > is explicitly checked first. > > > > A unit test has also been added to TimeOfDay. > > > > Bug: b:294436942 > > Change-Id: I751da660df4c89a9c545d465dfb5361dfb2acc64 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4750820 > > Reviewed-by: Jiaming Cheng <jiamingc@chromium.org> > > Commit-Queue: Eric Sum <esum@google.com> > > Cr-Commit-Position: refs/heads/main@{#1179335} > > Bug: b:294436942 > Change-Id: Iee92b34c51d4b202b6018dd1c62ddd4c4f76eaf6 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4748880 > Commit-Queue: David Roger <droger@chromium.org> > Owners-Override: David Roger <droger@chromium.org> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Cr-Commit-Position: refs/heads/main@{#1179492} Bug: b:294436942 Change-Id: I81aca119976bca9c4992f20064b3923146c5ab4d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4752645 Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Jiaming Cheng <jiamingc@chromium.org> Commit-Queue: Eric Sum <esum@google.com> Cr-Commit-Position: refs/heads/main@{#1181006}
pull bot
pushed a commit
that referenced
this pull request
Aug 31, 2023
https://crrev.com/c/2492281 introduced a commented out DCHECK() in `AgentSchedulingGroupHost::GetProcess()` ensuring that method is never called when its LifecycleState is in the `kRenderProcessHostDestroyed` state. It's commented out with the disclaimer above it, saying it cannot be enabled yet: "since RenderViewHostImpl outlives its associated AgentSchedulingGroupHost, and the [RenderViewHostImpl] dtor queries the associated RenderProcessHost to remove itself from the PerProcessRenderViewHostSet and RemoveObserver() itself." However, this isn't exactly correct. The RVHI doesn't appear to actually outlive its RPHI/ASGH; that would cause a different kind of crash/error altogether. The flow I believe the comment intends to describe can be seen in various browser test failures that occur when the DCHECK is enabled, that look like: ``` #6 0x7f9375403d6d logging::CheckError::~CheckError() #7 0x7f93609062e9 content::AgentSchedulingGroupHost::GetProcess() #8 0x7f9360e507dd content::RenderViewHostImpl::GetProcess() #9 0x7f9360e4dec6 content::RenderViewHostImpl::~RenderViewHostImpl() #10 0x7f9360e4e339 content::RenderViewHostImpl::~RenderViewHostImpl() ``` What happens is `RenderViewHostImpl::dtor()` is called, which calls: 1. `GetAgentSchedulingGroup().RemoveRoute()` [1] a. ASGH accomplishes this by going through its RenderProcessHostImpl b. If this is the last route in the process, then `RenderProcessHostImpl::Cleanup()` is invoked, which first alerts all observers of the `RenderProcessHostDestroyed()` event, and then immediately schedules its destruction in an asynchronous task 2. `GetProcess().GetID()` and `GetProcess()->RemoveObserver()` a. This fails because by this point, `AgentSchedulingGroup`, which is-a `RenderProcessHostObserver`, has already been notified of the `RenderProcessHostDestroyed` event, despite the RPHI still being alive, although its destruction has been scheduled. There are a few options to deal with this: 1. Move all invocations of methods that can trigger the `RenderProcessHostDestroyed()` event — like `RemoveRoute()` — to *after* any co-located calls to `GetProcess()`. This is really a hack, and is very fragile. Even if implemented, it's probably not actually possible to get this correct, since there are many calls to `GetProcess()` from other classes, that happen far away from calls to things like `RemoveRoute()`. That's what we see in [2], where I tried to implement this. And even still, if this were completely possible, it's very undesirable because then you have to keep in mind the allowed ordering of some random method invocations that might arbitrarily change over time across classes. BAD! 2. Another solution is to weaken the DCHECK(), as to ensure that we never call `GetProcess()` when `state_` is destroyed *unless* the RPHI also has its `deleting_soon_` bool set to true, indicating that destruction has been scheduled. We let this case slide because there's nothing wrong with getting the RPHI at this point. In unit tests, `MockRenderProcessHost` doesn't have an `deleting_soon_` bool, so we'd have to have a special carve-out for this case, where we just treat it as OK and pass the DCHECK() since we know we're in a unit test. This solution isn't bad, but unfortunately it's not possible to implement since there is no way to detect whether `ASGH::process_` is a `RenderProcessHostImpl` or a `MockRenderProcessHost`, so we can't distinguish between the "production" and the "test" scenario. 3. A better solution would probably be to just remove the DCHECK() since it's not doing anything useful. It's commented out, and even if it were enabled, it wouldn't necessarily be correct. There's nothing inherently wrong with consulting an RPHI in the gap between `Cleanup()` and `::dtor()`, which is what the DCHECK prevents. The DCHECK isn't protecting against accessing an already-actually- destroyed RPHI anyways; that's what the `base::SafeRef` around `AgentSchedulingGroupHost::process_` does! Given the fact that this DCHECK() was probably misunderstood in the first place — mistaking the RPHDestroyed() event for *actual* destruction — and was put into place before the base::SafeRef protection was implemented, which actually does something here, I firmly believe this is the best solution forward. This CL removes the DCHECK(). [1]: https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_view_host_impl.cc;l=383-388;drc=31154961f51fd2c1116e37fbc7e2f0ea6099414e [2]: https://chromium-review.googlesource.com/c/chromium/src/+/4226953/4?checksPatchset=4&tab=checks Bug: 1111231 Change-Id: Icf40f0f0bc0d40642a5fb2b64a664fc204d7d906 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4226953 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Commit-Queue: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/main@{#1190316}
pull bot
pushed a commit
that referenced
this pull request
Sep 1, 2023
…ation
At least on Linux X11, views::test::TestDesktopScreenOzone::Create() may
try to retrieve a localized string for "Built-in display" after [1].
But screen creation happens early for interactive UI tests since [2], at
least before ChromeFeatureListCreator::ConvertFlagsToSwitches() which
assumes ui::ResourceBundle is not initialized. This is causing the crash
below for all interactive UI tests on Linux X11, which this CL fixes by
temporarily initializing ui::ResourceBundle during screen creation.
See crbug.com/1457129 for details.
[24841:24841:FATAL:resource_bundle.cc(357)] Check failed: g_shared_instance_ != nullptr.
#0 0x5585747ceee2 base::debug::CollectStackTrace()
#1 0x5585747b6213 base::debug::StackTrace::StackTrace()
#2 0x5585746ab79d logging::LogMessage::~LogMessage()
#3 0x5585746ac2ee logging::LogMessage::~LogMessage()
#4 0x558574692437 logging::CheckError::~CheckError()
#5 0x558574bd6075 ui::ResourceBundle::GetSharedInstance()
#6 0x558574bc0a18 l10n_util::GetStringUTF8()
#7 0x5585758fddc1 ui::BuildDisplaysFromXRandRInfo()
#8 0x5585758fb539 ui::XDisplayManager::FetchDisplayList()
#9 0x5585758fb449 ui::XDisplayManager::Init()
#10 0x55856dd0e34c ui::X11ScreenOzone::Init()
#11 0x55856d09ed3d views::test::TestDesktopScreenOzone::Create()
#12 0x558572c7ec70 InProcessBrowserTest::SetScreenInstance()
#13 0x558572c7e44f InProcessBrowserTest::SetUp()
#14 0x55856d24c5f2 testing::Test::Run()
#15 0x55856d24d865 testing::TestInfo::Run()
#16 0x55856d24e337 testing::TestSuite::Run()
#17 0x55856d25cf07 testing::internal::UnitTestImpl::RunAllTests()
#18 0x55856d25c9ff testing::UnitTest::Run()
#19 0x5585746730ec base::TestSuite::Run()
#20 0x558572c5eddb ChromeTestSuiteRunner::RunTestSuiteInternal()
#21 0x55856cc79be4 InteractiveUITestSuiteRunner::RunTestSuite()
#22 0x558572c5f069 ChromeTestLauncherDelegate::RunTestSuite()
#23 0x558573d7b9ab content::LaunchTestsInternal()
#24 0x558572c5f325 LaunchChromeTests()
Bug: 1457129
[1] crrev.com/1010705
[2] crrev.com/1011300
Change-Id: I173b41d4369fa26332a37b0f89e14669b6b227a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4632804
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1191366}
pull bot
pushed a commit
that referenced
this pull request
Sep 2, 2023
…reen creation" This reverts commit 5931e82. Reason for revert: Suspect causing multiple test failed on Linux Tests (dbg)(1) first failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests%20(dbg)(1)/115562/overview Original change's description: > interactive_ui_tests: use temporary ui::ResourceBundle for screen creation > > At least on Linux X11, views::test::TestDesktopScreenOzone::Create() may > try to retrieve a localized string for "Built-in display" after [1]. > But screen creation happens early for interactive UI tests since [2], at > least before ChromeFeatureListCreator::ConvertFlagsToSwitches() which > assumes ui::ResourceBundle is not initialized. This is causing the crash > below for all interactive UI tests on Linux X11, which this CL fixes by > temporarily initializing ui::ResourceBundle during screen creation. > See crbug.com/1457129 for details. > > [24841:24841:FATAL:resource_bundle.cc(357)] Check failed: g_shared_instance_ != nullptr. > #0 0x5585747ceee2 base::debug::CollectStackTrace() > #1 0x5585747b6213 base::debug::StackTrace::StackTrace() > #2 0x5585746ab79d logging::LogMessage::~LogMessage() > #3 0x5585746ac2ee logging::LogMessage::~LogMessage() > #4 0x558574692437 logging::CheckError::~CheckError() > #5 0x558574bd6075 ui::ResourceBundle::GetSharedInstance() > #6 0x558574bc0a18 l10n_util::GetStringUTF8() > #7 0x5585758fddc1 ui::BuildDisplaysFromXRandRInfo() > #8 0x5585758fb539 ui::XDisplayManager::FetchDisplayList() > #9 0x5585758fb449 ui::XDisplayManager::Init() > #10 0x55856dd0e34c ui::X11ScreenOzone::Init() > #11 0x55856d09ed3d views::test::TestDesktopScreenOzone::Create() > #12 0x558572c7ec70 InProcessBrowserTest::SetScreenInstance() > #13 0x558572c7e44f InProcessBrowserTest::SetUp() > #14 0x55856d24c5f2 testing::Test::Run() > #15 0x55856d24d865 testing::TestInfo::Run() > #16 0x55856d24e337 testing::TestSuite::Run() > #17 0x55856d25cf07 testing::internal::UnitTestImpl::RunAllTests() > #18 0x55856d25c9ff testing::UnitTest::Run() > #19 0x5585746730ec base::TestSuite::Run() > #20 0x558572c5eddb ChromeTestSuiteRunner::RunTestSuiteInternal() > #21 0x55856cc79be4 InteractiveUITestSuiteRunner::RunTestSuite() > #22 0x558572c5f069 ChromeTestLauncherDelegate::RunTestSuite() > #23 0x558573d7b9ab content::LaunchTestsInternal() > #24 0x558572c5f325 LaunchChromeTests() > > Bug: 1457129 > > [1] crrev.com/1010705 > [2] crrev.com/1011300 > > Change-Id: I173b41d4369fa26332a37b0f89e14669b6b227a1 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4632804 > Commit-Queue: Frédéric Wang <fwang@igalia.com> > Reviewed-by: Scott Violet <sky@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1191366} Bug: 1457129, 1478318 Change-Id: Ib5adf3d8d8fb0b80789b6351f90cade2d6a6b682 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4833241 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Owen Min <zmin@chromium.org> Reviewed-by: Peter Williamson <petewil@chromium.org> Owners-Override: Owen Min <zmin@chromium.org> Auto-Submit: Owen Min <zmin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1191537}
pull bot
pushed a commit
that referenced
this pull request
Sep 5, 2023
@AliMariam reported the dangling pointer detector found a new dangling pointer when running tests on linux Workstation. The error is: ``` The memory was freed at: #3 allocator_shim::internal::PartitionFree() #4 bluez::BluezDBusThreadManager::~BluezDBusThreadManager() #5 bluez::BluezDBusThreadManager::Shutdown() #6 ChromeBrowserMainPartsLinux::PostDestroyThreads() #7 content::BrowserMainLoop::ShutdownThreadsAndCleanUp() #8 content::BrowserMainRunnerImpl::Shutdown() #9 content::BrowserMain() #10 content::RunBrowserProcessMain() #11 content::ContentMainRunnerImpl::RunBrowser() #12 content::ContentMainRunnerImpl::Run() #13 content::RunContentProcess() #14 content::ContentMain() #15 ChromeMain The dangling raw_ptr was released at: #3 base::internal::RawPtrBackupRefImpl<>::ReleaseInternal() #4 dbus::ObjectManager::~ObjectManager() #5 std::__Cr::__tuple_impl<>::~__tuple_impl() #6 base::internal::BindState<>::Destroy() #7 base::[...]::LazilyDeallocatedDeque<>::Ring::~Ring() #8 base::[...]::TaskQueueImpl::UnregisterTaskQueue() #9 base::[...]::SequenceManagerImpl::UnregisterTaskQueueImpl() #10 base::sequence_manager::TaskQueue::ShutdownTaskQueue() #11 content::BrowserTaskQueues::~BrowserTaskQueues() #12 content::BrowserUIThreadScheduler::~BrowserUIThreadScheduler() #13 content::BrowserTaskExecutor::[...]::~UIThreadExecutor() #14 content::BrowserTaskExecutor::[...]::~UIThreadExecutor() #15 content::BrowserTaskExecutor::Shutdown() #16 content::ContentMainRunnerImpl::Shutdown() #17 content::RunContentProcess() #18 content::ContentMain() #19 ChromeMain ``` Diagnostic: - `bluez::BluezDBusThreadManager` owns a `dbus::Bus` as `system_bus`. - `dbus::Bus` owns: - The set of `dbus::ObjectManager` as `object_manager_table_`. - The DBus task runner as `dbus_task_runner_`. - The `dbus::ObjectManager` references `dbus::Bus` via `bus_`. So far so good, the ownership is clear. The problem happens when calling `dbus::Bus::RemoveObjectManager`. Indeed this moves the ObjectManager out of `dbus::Bus` toward a callback to a new thread. This still works transitively, because the dbus::Bus owns the thread. The problem happens after a second transfer back to the original thread. Indeed, there is a race condition possible: Behavior without problems: ----------------------------------- ┌─────────────┐ ┌───────────┐ │Origin thread│ │DBus thread│ └──────┬──────┘ └─────┬─────┘ RemoveObjectManager() │ │────────────────────────────────>│ │ RemoveObjectManagerInternal() │<────────────────────────────────│ RemoveObjectManagerInternalHelper() │ ~ObjectManager() │ │ ┌─────┴─────┐ Shutdown DBus Thread ─────────────>│DBus thread│ Shutdown DBus Thread <─────────────│DBus thread│ │ └───────────┘ ~Bus ┌──────┴──────┐ │Origin thread│ └─────────────┘ Behavior with problems: ---------------------------------------- ┌─────────────┐ ┌───────────┐ │Origin thread│ │DBus thread│ └──────┬──────┘ └─────┬─────┘ RemoveObjectManager() │ │────────────────────────────────>│ │ RemoveObjectManagerInternal() │ ┌────────────│ │ │ ┌─────┴─────┐ Shutdown DBus Thread ─────────────>│DBus thread│ Shutdown DBus Thread <─────────────│DBus thread│ │ │ └───────────┘ ~Bus() │ │ │ │<───────────────────┘ RemoveObjectManagerInternalHelper() ~ObjectManager() ┌──────┴──────┐ │Origin thread│ └─────────────┘ ----------------------------------------------------------------- In the second case: ~Bus() is called before ~ObjectManager(). The fix is a use `ObjectManager::Cleanup()` to cleanup the raw_ptr while the object is still transitively owned by the object it referenced. Bug: chromium:1478759 Fixed: chromium:1478759 Change-Id: I4ac04d449ab8a7b860256c490f8ac878c1c5c7c5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4839496 Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/main@{#1192343}
pull bot
pushed a commit
that referenced
this pull request
Sep 13, 2023
This reverts commit e2c2265. Reason for revert: failing on: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/34119/test-results?sortby=&groupby= 2023-09-12T23:08:42.747041Z FATAL browser_tests[325873:325873]: [scalable_iph_browser_test_base.cc(114)] Check failed: !mock_delegate_. Mocks have already been set up. [...] #6 0x55e25ba19703 ash::ScalableIphBrowserTestBase::SetUpMocks() #7 0x55e25ba19661 ash::ScalableIphBrowserTestBase::SetUpOnMainThread() #8 0x55e2699e883b content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() [...] Original change's description: > Add phone hub onboarding eligible condition > > - This CL adds x_CustomConditionPhoneHubOnboardingEligible. The > condition is true if feature status of Phone Hub is either > kEligiblePhoneButNotSetUp or kDisabled. > - This adds a dependency from ScalableIph to PhoneHubManager. > PhoneHubManager is expected to be initialized at a specific timing. > This CL makes sure that ScalableIph is initialized after > PhoneHubManager. > - This CL fixes an issue with our OOBE flow test in our browser tests. > - To fix the browser test, we had to change behavior of > LoggedInUserMixin. Previously LoggedInUserMixin has configured > LoginManagerMixin to skip post login screens by default. This CL > makes it optional. > - familiy_user_device_metrics_browsertest.cc was depending on the > default behavior. This CL updates the test. > > Bug: b:295369951 > Test: browser_tests --gtest_filter=*ScalableIph* > Change-Id: I361169d5ca7d531ed26f16cd94b3247be6edb94d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4845174 > Reviewed-by: Alexander Alekseev <alemate@chromium.org> > Reviewed-by: Toni Barzic <tbarzic@chromium.org> > Reviewed-by: Courtney Wong <courtneywong@chromium.org> > Reviewed-by: Tao Wu <wutao@chromium.org> > Commit-Queue: Yuki Awano <yawano@google.com> > Cr-Commit-Position: refs/heads/main@{#1195612} Bug: b:295369951 Change-Id: I7bde487bc9268817bb1468c7db30d470e42de941 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4860192 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Frank Liberato <liberato@chromium.org> Commit-Queue: Frank Liberato <liberato@chromium.org> Owners-Override: Frank Liberato <liberato@google.com> Cr-Commit-Position: refs/heads/main@{#1195718}
pull bot
pushed a commit
that referenced
this pull request
Sep 14, 2023
This reverts commit 596cfbd. Reason for revert: Reland with a fix. Original change's description: > Revert "Add phone hub onboarding eligible condition" > > This reverts commit e2c2265. > > Reason for revert: failing on: > > https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-dbg/34119/test-results?sortby=&groupby= > > 2023-09-12T23:08:42.747041Z FATAL browser_tests[325873:325873]: [scalable_iph_browser_test_base.cc(114)] Check failed: !mock_delegate_. Mocks have already been set up. > [...] > #6 0x55e25ba19703 ash::ScalableIphBrowserTestBase::SetUpMocks() > #7 0x55e25ba19661 ash::ScalableIphBrowserTestBase::SetUpOnMainThread() > #8 0x55e2699e883b content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() > [...] > > Original change's description: > > Add phone hub onboarding eligible condition > > > > - This CL adds x_CustomConditionPhoneHubOnboardingEligible. The > > condition is true if feature status of Phone Hub is either > > kEligiblePhoneButNotSetUp or kDisabled. > > - This adds a dependency from ScalableIph to PhoneHubManager. > > PhoneHubManager is expected to be initialized at a specific timing. > > This CL makes sure that ScalableIph is initialized after > > PhoneHubManager. > > - This CL fixes an issue with our OOBE flow test in our browser tests. > > - To fix the browser test, we had to change behavior of > > LoggedInUserMixin. Previously LoggedInUserMixin has configured > > LoginManagerMixin to skip post login screens by default. This CL > > makes it optional. > > - familiy_user_device_metrics_browsertest.cc was depending on the > > default behavior. This CL updates the test. > > > > Bug: b:295369951 > > Test: browser_tests --gtest_filter=*ScalableIph* > > Change-Id: I361169d5ca7d531ed26f16cd94b3247be6edb94d > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4845174 > > Reviewed-by: Alexander Alekseev <alemate@chromium.org> > > Reviewed-by: Toni Barzic <tbarzic@chromium.org> > > Reviewed-by: Courtney Wong <courtneywong@chromium.org> > > Reviewed-by: Tao Wu <wutao@chromium.org> > > Commit-Queue: Yuki Awano <yawano@google.com> > > Cr-Commit-Position: refs/heads/main@{#1195612} > > Bug: b:295369951 > Change-Id: I7bde487bc9268817bb1468c7db30d470e42de941 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4860192 > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Auto-Submit: Frank Liberato <liberato@chromium.org> > Commit-Queue: Frank Liberato <liberato@chromium.org> > Owners-Override: Frank Liberato <liberato@google.com> > Cr-Commit-Position: refs/heads/main@{#1195718} Bug: b:295369951 Change-Id: Icd257f7b427877f34fdf2e017b4d4cb503089428 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4861031 Commit-Queue: Yuki Awano <yawano@google.com> Reviewed-by: Courtney Wong <courtneywong@chromium.org> Reviewed-by: Alexander Alekseev <alemate@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1196352}
pull bot
pushed a commit
that referenced
this pull request
Oct 4, 2023
…Item`" This reverts commit 08e77e6. Reason for revert: Builder: Linux ChromiumOS MSan Tests https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests First failing build: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/38777/overview Sample stack trace: ==235916==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x56206b82114e in ash::OverviewGrid::FitWindowRectsInBounds() ./../../ash/wm/overview/overview_grid.cc:2609:9 #1 0x56206b803b04 in ash::OverviewGrid::GetWindowRects() ./../../ash/wm/overview/overview_grid.cc:2441:24 #2 0x56206b806fce in ash::OverviewGrid::PositionWindows() ./../../ash/wm/overview/overview_grid.cc:689:13 #3 0x56206b80c92a in ash::OverviewGrid::AddItem() ./../../ash/wm/overview/overview_grid.cc:835:5 #4 0x56206b865c27 in ash::OverviewSession::AddItem() ./../../ash/wm/overview/overview_session.cc:565:9 #5 0x56206b80fb65 in ash::OverviewGrid::AddDropTargetForDraggingFromThisGrid(ash::OverviewItemBase*) ./../../ash/wm/overview/overview_grid.cc:949:22 #6 0x56206b8817d0 in ash::OverviewWindowDragController::StartNormalDragMode(gfx::PointF const&) ./../../ash/wm/overview/overview_window_drag_controller.cc:352:18 #7 0x56206b8811a3 in ash::OverviewWindowDragController::Drag(gfx::PointF const&) ./../../ash/wm/overview/overview_window_drag_controller.cc:281:7 Original change's description: > snap-group: Correct drop target bounds for the `OverviewGroupItem` > > The bounds for the drop target should match the item which the drop > target is a placeholder for. It should also be applicable for group > item. This cl does the following things to make it work: > 1. Update the logic in > `OverviewGrid::CalculateWidthAndMaybeSetUnclippedBounds()` where > the width for the drop target widget is calculated; > 2. Update `IsDropTargetWindow()` to `IsDropTargetItem()` in order to > take the overview group item into account; > 3. Update the insets value in `OverviewGroupItem` anonymous space, to > only have insets in the middle. > > Fixed: b/301490403 > Demo: http://b/301490403#comment2 > Test: Added unit tests + manual > Change-Id: I67e84bdf795833e9b92bbca39098013485953dd7 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4906318 > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> > Commit-Queue: Michele Fan <michelefan@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1204855} Bug: 1489085 Change-Id: I282a746c4c4fe7ff58f679c6339970c59ae47e68 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4904793 Owners-Override: Giovanni Ortuno Urquidi <ortuno@chromium.org> Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Giovanni Ortuno Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1205009}
pull bot
pushed a commit
that referenced
this pull request
Dec 6, 2023
Fixes crashes from ChromeOS LKGM and uprev: 2023-12-05T01:55:19.934369Z FATAL chrome[3861:1]: [v8_initializer.cc(784)] V8 error: Cannot create a handle without a HandleScope (v8::HandleScope::CreateHandle()). #0 0x58368e9b9372 base::debug::CollectStackTrace() #1 0x58368e99e243 base::debug::StackTrace::StackTrace() #2 0x583687fa98c8 logging::LogMessage::~LogMessage() #3 0x58369371ae51 blink::ReportV8FatalError() #4 0x5836898c46b8 v8::Utils::ReportApiFailure() #5 0x583697d32136 v8::internal::HandleScope::Extend() #6 0x583697944118 v8::HandleScope::CreateHandle() #7 0x5836946bd75c blink::SoftNavigationHeuristics::SetCurrentTimeAsStartTime() #8 0x5836946be3d0 blink::SoftNavigationEventScope::~SoftNavigationEventScope() #9 0x583697fb0441 blink::EventDispatcher::Dispatch() #10 0x583693d5380c blink::MouseEvent::DispatchEvent() Bug: 1507981 Change-Id: I4106e037bc08c81ede1f2d497b9f993991fb9e47 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5087067 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Cr-Commit-Position: refs/heads/main@{#1233189}
pull bot
pushed a commit
that referenced
this pull request
Dec 12, 2023
When the kAppServiceStorage flag is enabled, AppService blocks the app publishers (ExtensionAppsChromeOs) creating until it's ready, after reading from the AppStorage file. This affects some tests,e.g. All/CookieDeprecationLabelAllowedTest.IsClientEligibleChecked*. Received signal 11 SEGV_MAPERR 0000000000e8 ... #6 0x562c609a8f2f base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker() #7 0x562c604a31d4 PrefNotifierImpl::RemovePrefObserver() #8 0x562c604a0d24 PrefChangeRegistrar::RemoveAll() #9 0x562c604a0ba0 PrefChangeRegistrar::~PrefChangeRegistrar() #10 0x562c55e555bc apps::ExtensionAppsChromeOs::~ExtensionAppsChromeOs() #11 0x562c55e55bca apps::ExtensionAppsChromeOs::~ExtensionAppsChromeOs() #12 0x562c55e76e2c apps::PublisherHost::~PublisherHost() #14 0x562c55e0b367 apps::AppServiceProxyAsh::~AppServiceProxyAsh() #15 0x562c55e0b696 apps::AppServiceProxyAsh::~AppServiceProxyAsh() ... #19 0x562c5f53b6ac TestingProfile::~TestingProfile() #20 0x562c5f53bb36 TestingProfile::~TestingProfile() Because the local state `local_state_` is destroyed before TestingProfile. Modify the test code, to define `local_state_` first, to let TestingProfile be destroyed first, which is also aligned with the production code. BUG=1385932, b:307623506 Change-Id: I4d7a7d0493ddb6b6f779a7b38344a8f3e05c341b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5106950 Reviewed-by: Abe Boujane <boujane@google.com> Commit-Queue: Nancy Wang <nancylingwang@chromium.org> Cr-Commit-Position: refs/heads/main@{#1236035}
pull bot
pushed a commit
that referenced
this pull request
Dec 12, 2023
This reverts commit b573ec1. Reason for revert: file read on main thread causes crashes https://ci.chromium.org/ui/p/chrome/builders/ci/chromeos-reven-chrome/9260/overview 2023-12-11T23:44:32.631073Z FATAL chrome[4644:4644]: [thread_restrictions.cc(58)] Check failed: !tls_blocking_disallowed. Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the ThreadPool, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking (see its documentation for best practices). tls_blocking_disallowed 1 set by #0 0x580a56a26b42 base::debug::CollectStackTrace() #1 0x580a56a0d4d3 base::debug::StackTrace::StackTrace() #2 0x580a569e746f base::DisallowUnresponsiveTasks() #3 0x580a5391e8ef content::BrowserMainLoop::PreMainMessageLoopRun() #4 0x580a5062f27f base::OnceCallback<>::Run() #5 0x580a540ca43b content::StartupTaskRunner::RunAllTasksNow() #6 0x580a5391e206 content::BrowserMainLoop::CreateStartupTasks() #7 0x580a5392166e content::BrowserMainRunnerImpl::Initialize() #8 0x580a5391bad6 content::BrowserMain() #9 0x580a560952b6 content::RunBrowserProcessMain() #10 0x580a56097a80 content::ContentMainRunnerImpl::RunBrowser() #11 0x580a5609715e content::ContentMainRunnerImpl::Run() #12 0x580a56093df5 content::RunContentProcess() #13 0x580a56093f42 content::ContentMain() #14 0x580a5010c79d ChromeMain #15 0x7d10446316c6 __libc_start_call_main #16 0x7d1044631782 __libc_start_main_alias_2 #17 0x580a5010c531 _start #0 0x580a56a26b42 base::debug::CollectStackTrace() #1 0x580a56a0d4d3 base::debug::StackTrace::StackTrace() #2 0x580a4ffa72c8 logging::LogMessage::~LogMessage() #3 0x580a56923860 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() #4 0x580a4ffa6d67 logging::CheckError::~CheckError() #5 0x580a569e680b base::internal::AssertBlockingAllowed() #6 0x580a50037d3b base::ScopedBlockingCall::ScopedBlockingCall() #7 0x580a5fffd0fb base::File::Stat() #8 0x580a56a21ae3 base::GetFileInfo() #9 0x580a5a144486 first_run::GetFirstRunSentinelCreationTime() #10 0x580a55d04f18 ash::ReportControllerInitializer::OwnershipStatusChanged() #11 0x580a55d3a722 ash::DeviceSettingsService::NotifyOwnershipStatusChanged() #12 0x580a55d39e59 ash::DeviceSettingsService::HandleCompletedOperation() #13 0x580a55d3a0c8 ash::DeviceSettingsService::HandleCompletedAsyncOperation() #14 0x580a538a96d8 base::internal::FunctorTraits<>::Invoke<>() #15 0x580a55d3a8c2 base::internal::Invoker<>::RunOnce() Original change's description: > Create ReportInitializer that checks for browser preconditions > > Introduce ReportControllerInitializer class in chrome/browser/ash, > functioning as a DeviceSettingsService::Observer. The class waits for > the OwnershipStatusChanged callback with ownership status indicating > device ownership, checks for the presence of the .oobe_completed file, > and waits for CrosSettings::PrepareTrustedValues to signal TRUSTED > status. > > Only after these preconditions are met does the class proceed > to initialize the ReportController. > > BUG=chromium:1504019 > > Change-Id: I0bd93c2eb9f83669ea23395d59b292a4cc858bcb > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5047743 > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> > Commit-Queue: Hirthanan Subenderan <hirthanan@google.com> > Reviewed-by: Pavol Marko <pmarko@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1235879} Bug: chromium:1504019 Change-Id: If41cc3e2fa0330afa6cb5aeb84f05efd1e614669 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5113660 Auto-Submit: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Hirthanan Subenderan <hirthanan@google.com> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/main@{#1236048}
pull bot
pushed a commit
that referenced
this pull request
Dec 12, 2023
When the kAppServiceStorage flag is enabled, AppService blocks the app publishers creating until it's ready, after reading from the AppStorage file. This affects some tests,e.g. TetherServiceTest.*. Received signal 11 SEGV_MAPERR 0000000000e8 ... base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker() #7 0x563b848f01d4 PrefNotifierImpl::RemovePrefObserver() #8 0x563b848edd24 PrefChangeRegistrar::RemoveAll() #9 0x563b848edba0 PrefChangeRegistrar::~PrefChangeRegistrar() #10 0x563b7a2a25bc apps::ExtensionAppsChromeOs::~ExtensionAppsChromeOs() #11 0x563b7a2a2bca apps::ExtensionAppsChromeOs::~ExtensionAppsChromeOs() #14 0x563b7a258367 apps::AppServiceProxyAsh::~AppServiceProxyAsh() #15 0x563b7a258696 apps::AppServiceProxyAsh::~AppServiceProxyAsh() ... #19 0x563b839886ac TestingProfile::~TestingProfile() #20 0x563b83988b36 TestingProfile::~TestingProfile() Because `local_pref_service_` is destroyed before TestingProfile. Modify the test code, to define TestingProfile after `local_pref_service_` to destroy TestingProfile first. Remove `scoped_user_manager_`, because TestingProfile can create FakeChromeUserManager in its construct function[1]. [1] https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/base/testing_profile.cc;l=267 BUG=1385932, b:307623506 Change-Id: Id13cff8e7dc283bd6aa80700956058b82afb2f0f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5102370 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Nancy Wang <nancylingwang@chromium.org> Cr-Commit-Position: refs/heads/main@{#1236053}
pull bot
pushed a commit
that referenced
this pull request
Dec 14, 2023
When the kAppServiceStorage flag is enabled, AppService blocks the app publishers creating until it's ready, after reading from the AppStorage file. This affects some tests,e.g. ArcProvisionNotificationServiceOobeTest.*. Received signal 11 SEGV_MAPERR 000000000000 ... #5 0x562602f60ed8 base::ScopedObservationTraits<>::AddObserver() #6 0x562602f5c5a5 base::ScopedObservation<>::Observe() #7 0x562602f576b4 apps::ExtensionAppsChromeOs::Initialize() #8 0x562602fa335b apps::PublisherHost::Initialize() #9 0x562602fa31ac apps::PublisherHost::PublisherHost() #10 0x562602e81ce9 std::__Cr::make_unique<>() #11 0x562602e7a3ba apps::AppServiceProxyAsh::OnAppsReady() Because the test has been in the Teardown phase when the publish is created as the AppService is ready async, and NotificationDisplayService could not exists. So modify publishers to check NotificationDisplayService when observe it. [1] https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/base/testing_profile.cc;l=267 BUG=1385932, b:307623506 Change-Id: I372dc8eb02e0d29fb76704378a40a43c580ef2bd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5109253 Reviewed-by: Alan Cutter <alancutter@chromium.org> Commit-Queue: Nancy Wang <nancylingwang@chromium.org> Cr-Commit-Position: refs/heads/main@{#1236821}
pull bot
pushed a commit
that referenced
this pull request
Jan 19, 2024
...triggers entry doom and thus ends up running the operation queue in the wrong spot of creation (and entry op) completion handler. This can among other things result in this madness: #6 disk_cache::SimpleEntryImpl::CloseInternal() #7 disk_cache::SimpleEntryImpl::RunNextOperationIfNeeded() #8 SimpleEntryImpl::ScopedOperationRunner::~ScopedOperationRunner() #9 disk_cache::SimpleEntryImpl::WriteDataInternal() #10 disk_cache::SimpleEntryImpl::RunNextOperationIfNeeded() #11 SimpleEntryImpl::ScopedOperationRunner::~ScopedOperationRunner() #12 disk_cache::SimpleEntryImpl::WriteDataInternal() #13 disk_cache::SimpleEntryImpl::RunNextOperationIfNeeded() #14 disk_cache::SimpleEntryImpl::DoomEntry() #15 disk_cache::SimpleBackendImpl::DoomEntryFromHash() #16 disk_cache::SimpleBackendImpl::DoomEntries() #17 disk_cache::SimpleIndex::StartEvictionIfNeeded() #18 disk_cache::SimpleIndex::UpdateEntrySize() #19 disk_cache::SimpleEntryImpl::UpdateDataFromEntryStat() #20 disk_cache::SimpleEntryImpl::CreationOperationComplete() (namespace elided twice to avoid wrapping). ... which means we end up at the in_results = nullptr line near the bottom of CreationOperationComplete with null `synchronous_entry_`(!) (and a dangling in_results->sync_entry, where one would expect the two to be aliases). I *think* we won't actually deliver a callback from this state since we likely needed to be in optimistic path to got thus far, but I am not certain. Similarly, when this sort of thing happens from within read/write ops, it could potentially cause callbacks to be delivered in wrong order if the queued op ends up being a stream 0 operation, which can be executed without a round trip to a worker thread. Change-Id: Iac8058f0d18225677e361c6cdddf92d28fb4833f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5054619 Reviewed-by: Adam Rice <ricea@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Maks Orlovich <morlovich@chromium.org> Cr-Commit-Position: refs/heads/main@{#1248862}
pull bot
pushed a commit
that referenced
this pull request
Feb 23, 2024
Sometimes, we hit a limit where the field trial allocator is full in tests. For example, see failures in http://shortn/_YGDSsu6Sjs. Sample: #5 0x557208a9b41e logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() #6 0x557208a9add6 logging::CheckError::~CheckError() #7 0x557208ac98f1 base::FieldTrialList::ClearParamsFromSharedMemoryForTesting() #8 0x557208c56906 base::test::ScopedFeatureList::Reset() #9 0x557208c567f4 base::test::ScopedFeatureList::~ScopedFeatureList() #10 0x5571fdeaf4ae ClientHintsBrowserTest::~ClientHintsBrowserTest() #11 0x5571fdeace3c ClientHintsBrowserTest_ClientHintsAlps_Test::~ClientHintsBrowserTest_ClientHintsAlps_Test() Seems like when ScopedFeatureList is reset, it also wants to reset the params. It does so by completely invalidating everything in the allocator, and duplicating them (but without params). This CL does so that it does not invalidate and then duplicate an entry in the allocator if it has no params anyway. Bug: b/271447491 Change-Id: I834e6ef162ca7f790b5aa2581d6e50d2f27221b3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5314262 Commit-Queue: Luc Nguyen <lucnguyen@google.com> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Cr-Commit-Position: refs/heads/main@{#1264332}
pull bot
pushed a commit
that referenced
this pull request
Mar 14, 2024
DevToolsProtocolTest_RelatedWebsiteSets.GetRelatedWebsiteSets is flaky on multiple builders. *Builders are listed in bug comment #7. Change-Id: I2563f40974e978c22062e64578e8f09a330fda73 Bug: 329530175 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5370871 Reviewed-by: Shuran Huang <shuuran@chromium.org> Commit-Queue: Paul Adedeji <pauladedeji@google.com> Owners-Override: Paul Adedeji <pauladedeji@google.com> Cr-Commit-Position: refs/heads/main@{#1272972}
pull bot
pushed a commit
that referenced
this pull request
Mar 26, 2024
This reverts commit 6f447d5. Reason for revert: Crashes on ChromeOS as soon as I touch the device's touchpad 2024-03-26T00:58:16.588106Z FATAL chrome[5433:5496]: [sequence_checker.cc(21)] Check failed: checker.CalledOnValidSequence(&bound_at). #0 0x56f06aa34da2 base::debug::CollectStackTrace() #1 0x56f06aa19ba2 base::debug::StackTrace::StackTrace() #2 0x56f06a934073 logging::LogMessage::Flush() #3 0x56f063de6fcd logging::LogMessage::~LogMessage() #4 0x56f06a92053c logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() #5 0x56f06a9205ae logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() #6 0x56f063dc2ba6 logging::CheckError::~CheckError() #7 0x56f063de9d22 base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker() #8 0x56f063de4706 base::ObserverList<>::begin() #9 0x56f06bfa75eb ui::CursorController::SetCursorLocation() #10 0x56f0646d9be5 ui::DrmCursor::SetCursorLocationLocked() #11 0x56f0646da6bf ui::DrmCursor::MoveCursor() #12 0x56f06bfd76e6 ui::GestureInterpreterLibevdevCros::OnGestureMove() Original change's description: > Mark WeakLinkNode sequence checking expensive > > This makes for-each-node sequence checking expensive (which seems > redundant). As a compromise we do non-"expensive" sequence checking in > ObserverList::begin(), which should provide the same level of protection > unless iterators are passed between sequences, which would be one heck > of a thing to try to do. > > This accounts for about 60% of sequence checking in a profile I did way > back. I have not profiled to see how much sequence checking remains with > the sequence checking moved to begin() nor do I know the average > ObserverList size. Let's try it out. > > In the same profile (though I don't remember what I profiled) sequence > checking accounted for 1.2% of cycles. Hopefully this explains some of > the performance gap between a DCHECK and regular Canary build. > > Bug: 40241607 > Change-Id: Id80d3363771e05e6f38c1432ae66b4c352acf8b8 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5319909 > Reviewed-by: François Degros <fdegros@chromium.org> > Commit-Queue: Peter Boström <pbos@chromium.org> > Reviewed-by: danakj <danakj@chromium.org> > Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> > Reviewed-by: Colin Blundell <blundell@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1277148} Bug: 40241607 Change-Id: I8e11a7d796ba10a38453dff336f46e3aba04ab97 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5394849 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: François Degros <fdegros@chromium.org> Commit-Queue: François Degros <fdegros@chromium.org> Cr-Commit-Position: refs/heads/main@{#1278133}
pull bot
pushed a commit
that referenced
this pull request
Apr 2, 2024
While investigating a black stripe that appears on the right border of a lacros window while it is been resized horizontally from the opposite border (left), it was figured that depending on the values of the window `origin`, `size` and `scale factor`, the black line was appearing or hidden. A continuous interactive window resize is the perfect scenario to replicate the bug intermittently, and give users the impression of a flashing black line. As an easy way to illustrate the issue, lets assume a lacros window with the following bounds in DIPS, `330,0 596x664`, and a device scale factor of `1.62574` - these values can replicate the problem of the vertical stripe appearing even without triggering a window resize. For instance, this bounds in DIPS becomes `536,0 970x1080` in pixels using DesktopWindowTreeHostPlatform::ConvertRectToPixels(). 1.1) During the window creation, the `ui::Compositor` instance gets its scale factor and size set in pixels. It happens when `Compositor::SetScaleAndSize()` is called. This is the stracktrace: ```` #1 0x592718b9d713 base::debug::StackTrace::StackTrace() #2 0x5927205f4655 ui::Compositor::SetScaleAndSize() #3 0x5927205eac3e aura::WindowTreeHost::UpdateCompositorScaleAndSize() #4 0x5927205ec714 aura::WindowTreeHost::OnHostResizedInPixels() #5 0x592721d2dc2e aura::WindowTreeHostPlatform::OnBoundsChanged() #6 0x592721d2cc40 views::DesktopWindowTreeHostLacros::OnBoundsChanged() #7 0x592721d2e1b9 aura::WindowTreeHostPlatform::OnStateUpdate() #8 0x592719b83731 ui::WaylandWindow::MaybeApplyLatestStateRequest() #9 0x592719b83074 ui::WaylandWindow::RequestState() #10 0x592719b7e189 ui::WaylandWindow::SetWindowScale() #11 0x592719b7d985 ui::WaylandWindow::UpdateWindowScale() #12 0x592719b789ad ui::WaylandToplevelWindow::UpdateWindowScale() #13 0x592719b77f15 ui::WaylandToplevelWindow::Show() #14 0x592721d3139d views::DesktopWindowTreeHostPlatform::Show() #15 0x592721d18b24 views::DesktopNativeWidgetAura::Show() #16 0x592721cd0a7c views::Widget::Show() #17 0x592724d06ffb BrowserView::Show() (...) ```` In practice, `aura::WindowTreeHostPlatform::OnBoundsChanged()` (frame #4) calls out to `WaylandWindow::GetBoundsInPixels()`, that translates `330,0 596x664` in DIPS to `536,0 970x1080` in pixels - see the method below. Ultimately, only the size is set to the ui::Compositor instance, ie `970x1080`. ```` void WindowTreeHostPlatform::OnBoundsChanged(const BoundsChange& change) { (...) float current_scale = compositor()->device_scale_factor(); float new_scale = ui::GetScaleFactorForNativeView(window()); auto weak_ref = GetWeakPtr(); auto new_size = GetBoundsInPixels().size(); <------ ````` 1.2) Meanwhile, `cc::Layer` instances for the window are also being created. During the layer tree creation, the layers' bounds are all set in DIPs. For instance, the root layer is created with an origin of `0,0` and bounds `596x664`, eg: ```` 2024-03-11T18:42:45.387311Z WARNING chrome[29500:29500]: [layer.cc(390)] #0 0x59271e998ce2 base::debug::CollectStackTrace() #1 0x592718b9d713 base::debug::StackTrace::StackTrace() #2 0x592725cc29d7 cc::Layer::SetBounds() #3 0x592725e2ee96 ui::Layer::SetBoundsFromAnimation() #4 0x5927265c0177 ui::LayerAnimator::SetBounds() #5 0x5927205d8fe0 aura::Window::SetBoundsInternal() #6 0x5927205d8f67 aura::Window::SetBounds() #7 0x5927205ea78e aura::WindowTreeHost::UpdateRootWindowSize() #8 0x5927205ea6df aura::WindowTreeHost::InitHost() #9 0x592721d30b36 views::DesktopWindowTreeHostPlatform::Init() #10 0x592721d16336 views::DesktopNativeWidgetAura::InitNativeWidget() #11 0x592725142c44 DesktopBrowserFrameAura::InitNativeWidget() #12 0x592721ccdd41 views::Widget::Init() #13 0x592724cf68af BrowserFrame::InitBrowserFrame() #14 0x592724db9f30 BrowserWindow::CreateBrowserWindow() #15 0x592724951ce0 Browser::Browser() #16 0x5927249511c6 Browser::Create() (...) ```` The layers' bounds get translated at places like `draw_property_utils.cc` `ComputeLocalRectInTargetSpace()`, with MathUtils::MapXXX() functions. In practice, `596x664` translates to `968.942x1079.49`, and finally gets rounded to `969x1080`. Hence, we have a root ui::Compositor size of 970x1080 and a root cc:Layer bounds of 969x1080. Using the visual debugger tool attached to lacros, one can see that all window-wide tiles's width are 969 [1]. OTOH, with the visual debugger tool attached to ash/chrome, we can see that the width the the root surface (lacros) is 970 [2]. This 1px different creates a "punch role" effect in the lacros window, and user sees whatever is underneath it rendered [3]. In the case of this bug, what the user sees is a trailing 1px wide line part of the so called "resize shadow". The user sees it through this unintentionally "punch hole" explained above. This CL changes the way PlatformWindowDelegate::State::size_px variable gets set in WaylandWindow::RequestState(), translating only its size from DIPs to pixels. This way we match how this variable is used in ui::Compositor level and cc::Layers et al (tiles, quads, overlayers, etc). As a way to illustrate the problem, see video [4]. It forcibly paints the 1px lacros root layer transparent buffer red with viz_debugger, so one can clearly see it "flashing" due to the rounding error being fixed here. [1] https://issues.chromium.org/u/0/action/issues/40876438/attachments/54873934 [2] https://issues.chromium.org/u/0/action/issues/40876438/attachments/54878944 [3] https://issues.chromium.org/u/0/action/issues/40876438/attachments/54873933 [4] https://issues.chromium.org/action/issues/40876438/attachments/54936907 Bug: 40876438 Change-Id: Id36476d41e7a2c90f8a44337731a4cfad93e6a13 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5384752 Reviewed-by: Maksim Sisov <msisov@igalia.com> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com> Cr-Commit-Position: refs/heads/main@{#1281135}
pull bot
pushed a commit
that referenced
this pull request
Apr 2, 2024
This reverts commit fe5ffc2. Reason for revert: Reland with CursorController changed to use std::vector too. Original change's description: > Revert "Mark WeakLinkNode sequence checking expensive" > > This reverts commit 6f447d5. > > Reason for revert: Crashes on ChromeOS as soon as I touch the > device's touchpad > > 2024-03-26T00:58:16.588106Z FATAL chrome[5433:5496]: [sequence_checker.cc(21)] Check failed: checker.CalledOnValidSequence(&bound_at). > #0 0x56f06aa34da2 base::debug::CollectStackTrace() > #1 0x56f06aa19ba2 base::debug::StackTrace::StackTrace() > #2 0x56f06a934073 logging::LogMessage::Flush() > #3 0x56f063de6fcd logging::LogMessage::~LogMessage() > #4 0x56f06a92053c logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() > #5 0x56f06a9205ae logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() > #6 0x56f063dc2ba6 logging::CheckError::~CheckError() > #7 0x56f063de9d22 base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker() > #8 0x56f063de4706 base::ObserverList<>::begin() > #9 0x56f06bfa75eb ui::CursorController::SetCursorLocation() > #10 0x56f0646d9be5 ui::DrmCursor::SetCursorLocationLocked() > #11 0x56f0646da6bf ui::DrmCursor::MoveCursor() > #12 0x56f06bfd76e6 ui::GestureInterpreterLibevdevCros::OnGestureMove() > > > Original change's description: > > Mark WeakLinkNode sequence checking expensive > > > > This makes for-each-node sequence checking expensive (which seems > > redundant). As a compromise we do non-"expensive" sequence checking in > > ObserverList::begin(), which should provide the same level of protection > > unless iterators are passed between sequences, which would be one heck > > of a thing to try to do. > > > > This accounts for about 60% of sequence checking in a profile I did way > > back. I have not profiled to see how much sequence checking remains with > > the sequence checking moved to begin() nor do I know the average > > ObserverList size. Let's try it out. > > > > In the same profile (though I don't remember what I profiled) sequence > > checking accounted for 1.2% of cycles. Hopefully this explains some of > > the performance gap between a DCHECK and regular Canary build. > > > > Bug: 40241607 > > Change-Id: Id80d3363771e05e6f38c1432ae66b4c352acf8b8 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5319909 > > Reviewed-by: François Degros <fdegros@chromium.org> > > Commit-Queue: Peter Boström <pbos@chromium.org> > > Reviewed-by: danakj <danakj@chromium.org> > > Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> > > Reviewed-by: Colin Blundell <blundell@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1277148} > > Bug: 40241607 > Change-Id: I8e11a7d796ba10a38453dff336f46e3aba04ab97 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5394849 > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Owners-Override: François Degros <fdegros@chromium.org> > Commit-Queue: François Degros <fdegros@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1278133} Bug: 40241607 Change-Id: I7aedf2194f8cc17a56ffa79c1824e33a7072c1f1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5406313 Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Reviewed-by: Michael Spang <spang@chromium.org> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org> Reviewed-by: François Degros <fdegros@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/main@{#1281466}
pull bot
pushed a commit
that referenced
this pull request
Apr 11, 2024
This is part of the work to get cast on starboard building out of chromium. See go/moving-cwr-to-chromium for more information on the high-level goal. This is the crash that occurs without this CL: FATAL:event_factory_evdev.cc(247)] Check failed: user_input_task_runner_. #0 0x5643c4c23ed2 base::debug::CollectStackTrace() [../../base/debug/stack_trace_posix.cc:1039:7] #1 0x5643c4c0dc62 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:229:20] #2 0x5643c4b15369 logging::LogMessage::Flush() [../../base/logging.cc:703:29] #3 0x5643c4b1524d logging::LogMessage::~LogMessage() [../../base/logging.cc:694:3] #4 0x5643c4afb6bf logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:166:3] #5 0x5643c4afb71e logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:161:32] #6 0x5643c4afb173 logging::CheckError::~CheckError() [../../third_party/libc++/src/include/__memory/unique_ptr.h:67:5] #7 0x5643c611fca5 ui::EventFactoryEvdev::Init() [../../ui/events/ozone/evdev/event_factory_evdev.cc:247:3] #8 0x5643c5f275f7 ui::PlatformEventSource::AddPlatformEventDispatcher() [../../ui/events/platform/platform_event_source.cc:54:3] #9 0x5643c11edb81 ui::(anonymous namespace)::OzonePlatformCast::CreatePlatformWindow() [../../third_party/libc++/src/include/__memory/unique_ptr.h:620:30] #10 0x5643c67cf017 aura::WindowTreeHostPlatform::CreatePlatformWindow() [../../ui/aura/window_tree_host_platform.cc:222:44] #11 0x5643c67cef76 aura::WindowTreeHostPlatform::CreateAndSetPlatformWindow() [../../ui/aura/window_tree_host_platform.cc:93:22] #12 0x5643c67cef2a aura::WindowTreeHostPlatform::WindowTreeHostPlatform() [../../ui/aura/window_tree_host_platform.cc:71:3] #13 0x5643c24eb8d1 chromecast::CastWindowTreeHostAura::CastWindowTreeHostAura() [../../chromecast/graphics/cast_window_tree_host_aura.cc:17:7] #14 0x5643c24e937e chromecast::CastWindowManagerAura::Setup() [../../third_party/libc++/src/include/__memory/unique_ptr.h:620:30] #15 0x5643c24a324d chromecast::shell::CastBrowserMainParts::PreMainMessageLoopRun() [../../chromecast/browser/cast_browser_main_parts.cc:627:20] Bug: b/333571227 Change-Id: I7fd21eec0708282fc14d1b6dd578032ba06187c9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5439606 Reviewed-by: Luke Halliwell <halliwell@chromium.org> Reviewed-by: Yuchen Liu <yucliu@chromium.org> Commit-Queue: Antonio Rivera <antoniori@google.com> Cr-Commit-Position: refs/heads/main@{#1285202}
pull bot
pushed a commit
that referenced
this pull request
Apr 20, 2024
This reverts commit c07cbfe. Reason for revert: relanding with tests and fixes Original change's description: > Revert "Get WrapperTypeInfo via ScriptWrappable" > > This reverts commit 81b6e3d. > > Reason for revert: Breaks AutotestPrivateApiTest.AutotestPrivate on linux-chromeos-rel > > https://ci.chromium.org/ui/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel > Sample failure: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-chromeos-rel/75905/overview > > I think this is the relevant part of the stack trace: > > ../../content/public/test/no_renderer_crashes_assertion.cc:102: Failure > Failed > Unexpected termination of a renderer process; status: 3, exit_code: 139 > Stack trace: > #0 0x563e8b77c0da content::NoRendererCrashesAssertion::RenderProcessExited() > #1 0x563e8967d8c4 content::RenderProcessHostImpl::ProcessDied() > #2 0x563e8967d72c content::RenderProcessHostImpl::OnChannelError() > #3 0x563e8ae3ecb5 base::TaskAnnotator::RunTaskImpl() > #4 0x563e8ae583dd base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl() > #5 0x563e8ae57e60 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() > #6 0x563e8ae58845 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() > #7 0x563e8aec2fef base::MessagePumpEpoll::Run() > #8 0x563e8ae58bb2 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run() > #9 0x563e8ae1e7fd base::RunLoop::Run() > #10 0x563e91a0c6c9 extensions::ResultCatcher::GetNextResult() > #11 0x563e8ad7fcf6 extensions::ExtensionApiTest::RunExtensionTest() > #12 0x563e8ad7f999 extensions::ExtensionApiTest::RunExtensionTest() > #13 0x563e854402ed extensions::AutotestPrivateApiTest::RunAutotestPrivateExtensionTest() > > Original change's description: > > Get WrapperTypeInfo via ScriptWrappable > > > > as opposed to using a dedicated internal field for that. > > > > Bug: 328117814 > > Change-Id: I01f9aff3ad8a41fafbd2655d23f076a0f76fdc57 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5455405 > > Reviewed-by: Nate Chapin <japhet@chromium.org> > > Commit-Queue: Andrey Kosyakov <caseq@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1288405} > > Bug: 328117814 > Change-Id: Id0ad5b6bcab7a99cf31d551df00928708dd93465 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5459075 > Reviewed-by: Jiacheng Guo <gjc@google.com> > Auto-Submit: Timothy Loh <timloh@chromium.org> > Commit-Queue: Jiacheng Guo <gjc@google.com> > Owners-Override: Timothy Loh <timloh@chromium.org> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Cr-Commit-Position: refs/heads/main@{#1288546} Bug: 328117814 Change-Id: Icc69d23b24b71adc9332e3a01c91f336506c035f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466905 Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Cr-Commit-Position: refs/heads/main@{#1290020}
pull bot
pushed a commit
that referenced
this pull request
Apr 20, 2024
Example builder failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests%20(dbg)(1)/119860/overview. crash log for renderer (pid <unknown>): STDOUT: <empty> STDERR: [840814:1:0419/092311.735189:FATAL:ax_object.cc(3129)] Check failed: !IsDetached(). STDERR: #0 0x7f57ed36492c base::debug::CollectStackTrace() [../../base/debug/stack_trace_posix.cc:1039:7] STDERR: #1 0x7f57ed316545 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:236:20] STDERR: #2 0x7f57ed3164d5 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:231:28] STDERR: #3 0x7f57ed0349bf logging::LogMessage::Flush() [../../base/logging.cc:710:29] STDERR: #4 0x7f57ed0348e7 logging::LogMessage::~LogMessage() [../../base/logging.cc:698:3] STDERR: #5 0x7f57ecfdbfc5 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:166:3] STDERR: #6 0x7f57ecfdbfe9 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:161:32] STDERR: #7 0x7f57ecfdcdcc std::__Cr::default_delete<>::operator()() [../../third_party/libc++/src/include/__memory/unique_ptr.h:67:5] STDERR: #8 0x7f57ecfdc32a std::__Cr::unique_ptr<>::reset() [../../third_party/libc++/src/include/__memory/unique_ptr.h:278:7] STDERR: #9 0x7f57ecfdb90d logging::CheckError::~CheckError() [../../base/check.cc:349:16] STDERR: #10 0x7f57bb3ae68b blink::AXObject::IsIncludedInTree() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:3129:3] STDERR: #11 0x7f57bb3a43a4 blink::AXObject::ToString() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:8108:30] STDERR: #12 0x7f57bb3a2a52 blink::AXObject::Detach() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:782:35] STDERR: #13 0x7f57bb3556e6 blink::AXNodeObject::Detach() [../../third_party/blink/renderer/modules/accessibility/ax_node_object.cc:2377:13] STDERR: #14 0x7f57bb3ec319 blink::AXObjectCacheImpl::Remove() .... Bug: 40933623 Change-Id: Ic447ade12398f7666c4f6f4def61c2e3723feb43 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466669 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: Liviu Tinta <liviutinta@chromium.org> Auto-Submit: Liviu Tinta <liviutinta@chromium.org> Owners-Override: Liviu Tinta <liviutinta@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#1290248}
pull bot
pushed a commit
that referenced
this pull request
Apr 22, 2024
This reverts commit 6578c4f. Reason for revert: Offending DCHECK was fixed and removed with https://crrev.com/c/5464203. Tests can probably be re-enabled. Original change's description: > [Gardener] Disable tests failing !IsDetached check > > Example builder failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20Tests%20(dbg)(1)/119860/overview. > > crash log for renderer (pid <unknown>): > STDOUT: <empty> > STDERR: [840814:1:0419/092311.735189:FATAL:ax_object.cc(3129)] Check failed: !IsDetached(). > STDERR: #0 0x7f57ed36492c base::debug::CollectStackTrace() [../../base/debug/stack_trace_posix.cc:1039:7] > STDERR: #1 0x7f57ed316545 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:236:20] > STDERR: #2 0x7f57ed3164d5 base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:231:28] > STDERR: #3 0x7f57ed0349bf logging::LogMessage::Flush() [../../base/logging.cc:710:29] > STDERR: #4 0x7f57ed0348e7 logging::LogMessage::~LogMessage() [../../base/logging.cc:698:3] > STDERR: #5 0x7f57ecfdbfc5 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:166:3] > STDERR: #6 0x7f57ecfdbfe9 logging::(anonymous namespace)::DCheckLogMessage::~DCheckLogMessage() [../../base/check.cc:161:32] > STDERR: #7 0x7f57ecfdcdcc std::__Cr::default_delete<>::operator()() [../../third_party/libc++/src/include/__memory/unique_ptr.h:67:5] > STDERR: #8 0x7f57ecfdc32a std::__Cr::unique_ptr<>::reset() [../../third_party/libc++/src/include/__memory/unique_ptr.h:278:7] > STDERR: #9 0x7f57ecfdb90d logging::CheckError::~CheckError() [../../base/check.cc:349:16] > STDERR: #10 0x7f57bb3ae68b blink::AXObject::IsIncludedInTree() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:3129:3] > STDERR: #11 0x7f57bb3a43a4 blink::AXObject::ToString() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:8108:30] > STDERR: #12 0x7f57bb3a2a52 blink::AXObject::Detach() [../../third_party/blink/renderer/modules/accessibility/ax_object.cc:782:35] > STDERR: #13 0x7f57bb3556e6 blink::AXNodeObject::Detach() [../../third_party/blink/renderer/modules/accessibility/ax_node_object.cc:2377:13] > STDERR: #14 0x7f57bb3ec319 blink::AXObjectCacheImpl::Remove() > .... > > Bug: 40933623 > Change-Id: Ic447ade12398f7666c4f6f4def61c2e3723feb43 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466669 > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Reviewed-by: Liviu Tinta <liviutinta@chromium.org> > Auto-Submit: Liviu Tinta <liviutinta@chromium.org> > Owners-Override: Liviu Tinta <liviutinta@chromium.org> > Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Cr-Commit-Position: refs/heads/main@{#1290248} Bug: 40933623 Change-Id: Ib77c888c8c8a1209a18ddd93f7c07d17c50a04a5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5465688 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Simon Zünd <szuend@chromium.org> Cr-Commit-Position: refs/heads/main@{#1290555}
pull bot
pushed a commit
that referenced
this pull request
May 2, 2024
This reverts commit 5b98671. Reason for revert: browser_test failures starting at https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome/41245/blamelist The failures all have stack: 2024-05-01T22:20:26.913799Z ERROR browser_tests[68320:68386]: [object_proxy.cc(576)] Failed to call method: org.chromium.debugd.GetPerfOutputV2: object_path= /org/chromium/debugd: org.freedesktop.DBus.Error.ServiceUnknown: The name org.chromium.debugd was not provided by any .service files #0 0x55f5d4f55172 base::debug::CollectStackTrace() [../../base/debug/stack_trace_posix.cc:1043:7] #1 0x55f5d4f4139e base::debug::StackTrace::StackTrace() [../../base/debug/stack_trace.cc:241:20] #2 0x55f5d57d3a8a content::(anonymous namespace)::SignalHandler() [../../content/public/test/browser_test_base.cc:177:24] #3 0x7f9d71a42520 (/usr/lib/x86_64-linux-gnu/libc.so.6+0x4251f) #4 0x55f5db0e82e7 ash::FwupdDownloadClientImpl::GetURLLoaderFactory() [../../chrome/browser/ui/ash/fwupd_download_client_impl.cc:21:44] #5 0x55f5d833381c ash::FirmwareUpdateManager::DownloadLvfsMirrorFile() [../../chromeos/ash/components/fwupd/firmware_update_manager.cc:1051:35] #6 0x55f5d83350c1 base::internal::DecayedFunctorTraits<>::Invoke<>() [../../base/functional/bind_internal.h:738:12] #7 0x55f5d8334f93 base::internal::Invoker<>::RunOnce() [../../base/functional/bind_internal.h:954:5] Original change's description: > fwupd: Add refresh metadata logic > > Download the jcat file from chromeos local mirror and store it in cache. > Copy the file to a gz file and unzip it. Read the contents to get the > latest firmware file name. Download that file from chromeos local mirror > and call UpdateMetadata on those 2 files. > > BUG=b:328637488 > TEST=autoninja chrome; manually check from logs > chromeos_unittests --gtest_filter=FwupdClientTest.* && > FirmwareUpdateManager* > > Change-Id: I5673ad909963c4436a580746db9b94dd1c1a4f1b > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5444929 > Commit-Queue: Rishabh Agrawal <rishabhagr@chromium.org> > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Adenilson Cavalcanti <cavalcantii@chromium.org> > Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org> > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1295074} Bug: b:328637488 Change-Id: Ib753c1db9048f1acf693606ff7058d9820942f3b No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5508951 Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Joel Hockey <joelhockey@chromium.org> Owners-Override: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/main@{#1295258}
pull bot
pushed a commit
that referenced
this pull request
May 15, 2024
Since @page border box layout objects aren't in the the layout tree, any code that wants to walk up the tree to find the containing block will be in for a surprise. This would happen if percentage-based @page padding was used [1]. Recomputing padding during painting when we have already done it during layout is rather pointless anyway. Read it out directly from the fragment. [1] #1 blink::LayoutBox::ContainingBlockLogicalWidthForContent() #2 blink::LayoutBoxModelObject::ComputedCSSPadding() #3 blink::LayoutBoxModelObject::PaddingTop() #4 blink::LayoutBoxModelObject::PaddingOutsets() #5 blink::BoxPainterBase::PaintFillLayer() #6 blink::BoxPainterBase::PaintFillLayers() #7 blink::BoxFragmentPainter::PaintBackground() Bug: 40286153 Change-Id: I1e6e92c2ce1d81aab2673ec9a877eac455534102 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5526469 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1300711}
pull bot
pushed a commit
that referenced
this pull request
May 21, 2024
Attempting to initialize the GPU triggers MSan failures in GL with eager
checks enabled in MSan:
Uninitialized bytes in strlen at offset 0 inside [0x70200013dec0, 30)
==589==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fffeeba96de in glLabelObjectEXT (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x4f6de)
#1 0x7fffeeba9b10 in glLabelObjectEXT (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x4fb10)
#2 0x7fffeeb879a3 (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x2d9a3)
#3 0x7fffeeb894c0 (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x2f4c0)
#4 0x7fffeeb8e400 (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x34400)
#5 0x7fffeeb8f384 (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x35384)
#6 0x7fffeeb8bd9a (/lib/x86_64-linux-gnu/libGLX_mesa.so.0+0x31d9a)
#7 0x7fffecceb484 in queryExtensionsString third_party/angle/src/libANGLE/renderer/gl/glx/FunctionsGLX.cpp:354:12
#8 0x7fffecceb484 in rx::FunctionsGLX::initialize(_XDisplay*, int, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>>*) third_party/angle/src/libANGLE/renderer/gl/glx/FunctionsGLX.cpp:223:30
#9 0x7fffeccdada2 in rx::DisplayGLX::initialize(egl::Display*) third_party/angle/src/libANGLE/renderer/gl/glx/DisplayGLX.cpp:114:15
#10 0x7fffec70f649 in egl::Display::initialize() third_party/angle/src/libANGLE/Display.cpp:1082:36
#11 0x7fffec0bed17 in egl::Initialize(egl::Thread*, egl::Display*, int*, int*) third_party/angle/src/libGLESv2/egl_stubs.cpp:514:5
#12 0x7fffec0c6319 in EGL_Initialize third_party/angle/src/libGLESv2/entry_points_egl_autogen.cpp:571:27
#13 0x7fffefc4000d in eglInitialize third_party/angle/src/libEGL/libEGL_autogen.cpp:177:12
#14 0x5555868ec92a in gl::GLDisplayEGL::InitializeDisplay(bool, std::__Cr::vector<gl::DisplayType, std::__Cr::allocator<gl::DisplayType>>, gl::EGLDisplayPlatform, gl::GLDisplayEGL*) ui/gl/gl_display.cc:769:10
#15 0x5555868eb310 in gl::GLDisplayEGL::Initialize(bool, std::__Cr::vector<gl::DisplayType, std::__Cr::allocator<gl::DisplayType>>, gl::EGLDisplayPlatform) ui/gl/gl_display.cc:660:8
#16 0x55556c6f1957 in ui::GLOzoneEGL::InitializeGLOneOffPlatform(bool, std::__Cr::vector<gl::DisplayType, std::__Cr::allocator<gl::DisplayType>>, gl::GpuPreference) ui/ozone/common/gl_ozone_egl.cc:25:17
#17 0x55558b1a8999 in gl::init::InitializeGLOneOffPlatform(gl::GpuPreference) ui/gl/init/gl_initializer_ozone.cc:27:26
#18 0x55558b1a6978 in gl::init::InitializeGLOneOffPlatformImplementation(bool, bool, bool, gl::GpuPreference) ui/gl/init/gl_factory.cc:211:24
#19 0x55558b1a6208 in gl::init::(anonymous namespace)::InitializeGLOneOffPlatformHelper(bool, gl::GpuPreference) ui/gl/init/gl_factory.cc:135:10
#20 0x55558b1a662d in gl::init::InitializeGLNoExtensionsOneOff(bool, gl::GpuPreference) ui/gl/init/gl_factory.cc:166:10
#21 0x55558b227379 in gpu::GpuInit::InitializeAndStartSandbox(base::CommandLine*, gpu::GpuPreferences const&) gpu/ipc/service/gpu_init.cc:495:18
#22 0x55559ab41ce4 in content::GpuMain(content::MainFunctionParams) content/gpu/gpu_main.cc:358:39
#23 0x55557b5888da in content::RunZygote(content::ContentMainDelegate*) content/app/content_main_runner_impl.cc:685:14
#24 0x55557b58abad in content::RunOtherNamedProcessTypeMain(std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, content::MainFunctionParams, content::ContentMainDelegate*) content/app/content_main_runner_impl.cc:789:12
#25 0x55557b58e4fe in content::ContentMainRunnerImpl::Run() content/app/content_main_runner_impl.cc:1156:10
#26 0x55557b585aaf in content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) content/app/content_main.cc:332:36
#27 0x55557b58644a in content::ContentMain(content::ContentMainParams) content/app/content_main.cc:345:10
#28 0x555580df9c37 in content::LaunchTestsInternal(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:385:12
#29 0x555580dfab58 in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:504:10
#30 0x555582b8afec in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:392:10
#31 0x55557d422a0e in main chrome/test/base/browser_tests_main.cc:60:10
#32 0x7ffff48456c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#33 0x7ffff4845784 in __libc_start_main csu/../csu/libc-start.c:360:3
#34 0x55555e647029 in _start (/usr/local/google/home/dcheng/src/chrome/src/out/msan/browser_tests+0x90f3029) (BuildId: 6cbc295b16ac1107)
Uninitialized value was created by a heap allocation
#0 0x55555e675a22 in malloc /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/msan/msan_interceptors.cpp:1021:3
#1 0x7ffff489b087 in __vasprintf_internal libio/vasprintf.c:116:16
#2 0x4ae8d349882b18ff (<unknown module>)
Bug: 40240570
Change-Id: If59faa297e214b7219ebb4c5c9572b44a33ee070
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5548826
Reviewed-by: Elad Alon <eladalon@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1303583}
pull bot
pushed a commit
that referenced
this pull request
Jun 5, 2024
This CL is trivial (contextualized by its downstream counterpart). Some PA unit tests will `free()` an object and subsequently use the address as a slot start. PA's standard mechanism for asserting that an address is a slot start is incompatible with this, and so the test should be performed with an explicitly unchecked variant. Sample backtrace from LUD unit tests: ``` (lldb) bt * thread #1, name = 'base_unittests', stop reason = signal SIGTRAP * frame #0: 0x0000555559e206b9 base_unittests`partition_alloc::internal::logging::LogMessage::~LogMessage()::$_0::operator()(this=0x00007fffffffbaa7) const at log_message.cc:128:5 frame #1: 0x0000555559e20682 base_unittests`partition_alloc::internal::logging::LogMessage::~LogMessage(this=0x00007fffffffbc28) at log_message.cc:128:5 frame #2: 0x0000555559e1fdf6 base_unittests`partition_alloc::internal::logging::CheckError::~CheckError(this=0x00007fffffffbc28) at check.cc:74:18 frame #3: 0x0000555557311045 base_unittests`partition_alloc::internal::logging::check_error::DCheck::~DCheck(this=0x00007fffffffbc28) at check.h:120:49 frame #4: 0x0000555557313786 base_unittests`partition_alloc::internal::IsReservationStart(address=67723054809088) at reservation_offset_table.h:265:3 frame #5: 0x0000555557313524 base_unittests`partition_alloc::internal::PartitionPageMetadata::FromAddr(address=67723054825472) at partition_page.h:485:3 frame #6: 0x0000555557312fd6 base_unittests`partition_alloc::internal::SlotSpanMetadata::FromAddr(address=67723054825472) at partition_page.h:534:25 frame #7: 0x00005555573157c0 base_unittests`partition_alloc::internal::SlotStart<true>::CheckIsSlotStart(this=0x00007fffffffc8b0) at partition_page.h:852:32 frame #8: 0x0000555557315780 base_unittests`partition_alloc::internal::SlotStart<true>::SlotStart(this=0x00007fffffffc8b0, untagged_slot_start=67723054825472) at partition_page.h:846:7 frame #9: 0x0000555557316708 base_unittests`partition_alloc::PartitionRoot::ObjectToSlotStart(this=0x00002b6c0005c000, object=0x00003d9800a04000) const at partition_root.h:832:12 frame #10: 0x0000555559e2cb74 base_unittests`partition_alloc::internal::LightweightQuarantineBranch::IsQuarantinedForTesting(this=0x00002b6c000146d8, object=0x00003d9800a04000) at lightweight_quarantine.cc:108:48 frame #11: 0x0000555558c26758 base_unittests`partition_alloc::PartitionAllocLightweightQuarantineTest_TooLargeAllocation_Test::TestBody(this=0x00002b6c00014680) at lightweight_quarantine_unittest.cc:129:3 ``` Bug: 337078970 Change-Id: I08592544b79be323a724b139f642105316cfdf30 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5584938 Reviewed-by: Keishi Hattori <keishi@chromium.org> Commit-Queue: Kalvin Lee <kdlee@chromium.org> Cr-Commit-Position: refs/heads/main@{#1309767}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )