Reinstate old tab-loading animation for M72 |
||||||
Issue descriptionThe new tab-loading animation isn't ready, so we need to unlaunch it.
,
Dec 6
Verified in Canary.
,
Dec 6
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1cbde78f21bba5d565b93ac1236e9ff00ed7ff24 commit 1cbde78f21bba5d565b93ac1236e9ff00ed7ff24 Author: Peter Boström <pbos@chromium.org> Date: Thu Dec 06 18:09:10 2018 Disable the new tab-loading animation Makes sure that a lot of animation-related code is bypassed when the new-tab-animation flag is off. This should hopefully fix a couple of performance regressions that have not yet been root caused so that they don't go out with M72. Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 Reviewed-on: https://chromium-review.googlesource.com/c/1364212 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614199}(cherry picked from commit 355b8185716292a8f145c77168ee89148ecabf31) Reviewed-on: https://chromium-review.googlesource.com/c/1365983 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#118} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/1cbde78f21bba5d565b93ac1236e9ff00ed7ff24/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/1cbde78f21bba5d565b93ac1236e9ff00ed7ff24/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/1cbde78f21bba5d565b93ac1236e9ff00ed7ff24/chrome/common/chrome_features.cc
,
Dec 6
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/742b8857c4dd0ab4550f7b2c1f200ce5be406b9c commit 742b8857c4dd0ab4550f7b2c1f200ce5be406b9c Author: Jeffrey Yasskin <jyasskin@chromium.org> Date: Thu Dec 06 19:13:17 2018 Revert "Disable the new tab-loading animation" This reverts commit 355b8185716292a8f145c77168ee89148ecabf31. Reason for revert: Made several tests flaky: https://crbug.com/912543 Original change's description: > Disable the new tab-loading animation > > Makes sure that a lot of animation-related code is bypassed when the > new-tab-animation flag is off. This should hopefully fix a couple of > performance regressions that have not yet been root caused so that they > don't go out with M72. > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 > Reviewed-on: https://chromium-review.googlesource.com/c/1364212 > Reviewed-by: Sidney San Martín <sdy@chromium.org> > Commit-Queue: Peter Boström <pbos@chromium.org> > Cr-Commit-Position: refs/heads/master@{#614199} TBR=pbos@chromium.org,sdy@chromium.org Change-Id: Ib4c022a255ad085c1716d3559a7f84dcb61c2785 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Reviewed-on: https://chromium-review.googlesource.com/c/1366359 Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org> Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org> Cr-Commit-Position: refs/heads/master@{#614440} [modify] https://crrev.com/742b8857c4dd0ab4550f7b2c1f200ce5be406b9c/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/742b8857c4dd0ab4550f7b2c1f200ce5be406b9c/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/742b8857c4dd0ab4550f7b2c1f200ce5be406b9c/chrome/common/chrome_features.cc
,
Dec 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ad12d44124812f22484eb4d123e4a3b4aaac93b9 commit ad12d44124812f22484eb4d123e4a3b4aaac93b9 Author: Peter Boström <pbos@chromium.org> Date: Tue Dec 11 18:02:22 2018 Reland "Disable the new tab-loading animation" This reverts commit 742b8857c4dd0ab4550f7b2c1f200ce5be406b9c. Reason for revert: Flaky tests should be fixed in r615470. Bug: chromium:912543 , chromium:913135 , chromium:913784 Original change's description: > Revert "Disable the new tab-loading animation" > > This reverts commit 355b8185716292a8f145c77168ee89148ecabf31. > > Reason for revert: Made several tests flaky: https://crbug.com/912543 > > Original change's description: > > Disable the new tab-loading animation > > > > Makes sure that a lot of animation-related code is bypassed when the > > new-tab-animation flag is off. This should hopefully fix a couple of > > performance regressions that have not yet been root caused so that they > > don't go out with M72. > > > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 > > Reviewed-on: https://chromium-review.googlesource.com/c/1364212 > > Reviewed-by: Sidney San Martín <sdy@chromium.org> > > Commit-Queue: Peter Boström <pbos@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#614199} > > TBR=pbos@chromium.org,sdy@chromium.org > > Change-Id: Ib4c022a255ad085c1716d3559a7f84dcb61c2785 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > Reviewed-on: https://chromium-review.googlesource.com/c/1366359 > Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org> > Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org> > Cr-Commit-Position: refs/heads/master@{#614440} TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Change-Id: I3981455a00f743fae535de4626179dcceab9b2c4 Reviewed-on: https://chromium-review.googlesource.com/c/1372236 Reviewed-by: Peter Boström <pbos@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#615581} [modify] https://crrev.com/ad12d44124812f22484eb4d123e4a3b4aaac93b9/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/ad12d44124812f22484eb4d123e4a3b4aaac93b9/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/ad12d44124812f22484eb4d123e4a3b4aaac93b9/chrome/common/chrome_features.cc
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6921bd825e416c7b412b454f60df8b183e906943 commit 6921bd825e416c7b412b454f60df8b183e906943 Author: Friedrich Horschig <fhorschig@chromium.org> Date: Wed Dec 12 14:10:33 2018 Revert "Reland "Disable the new tab-loading animation"" This reverts commit ad12d44124812f22484eb4d123e4a3b4aaac93b9. Reason for revert: Findit found this to be the most likely culprit for the single_process_mash_browser_tests failures on various Linux bots: https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/30518 Original change's description: > Reland "Disable the new tab-loading animation" > > This reverts commit 742b8857c4dd0ab4550f7b2c1f200ce5be406b9c. > > Reason for revert: Flaky tests should be fixed in r615470. > > Bug: chromium:912543 , chromium:913135 , chromium:913784 > > Original change's description: > > Revert "Disable the new tab-loading animation" > > > > This reverts commit 355b8185716292a8f145c77168ee89148ecabf31. > > > > Reason for revert: Made several tests flaky: https://crbug.com/912543 > > > > Original change's description: > > > Disable the new tab-loading animation > > > > > > Makes sure that a lot of animation-related code is bypassed when the > > > new-tab-animation flag is off. This should hopefully fix a couple of > > > performance regressions that have not yet been root caused so that they > > > don't go out with M72. > > > > > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > > Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 > > > Reviewed-on: https://chromium-review.googlesource.com/c/1364212 > > > Reviewed-by: Sidney San Martín <sdy@chromium.org> > > > Commit-Queue: Peter Boström <pbos@chromium.org> > > > Cr-Commit-Position: refs/heads/master@{#614199} > > > > TBR=pbos@chromium.org,sdy@chromium.org > > > > Change-Id: Ib4c022a255ad085c1716d3559a7f84dcb61c2785 > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > Reviewed-on: https://chromium-review.googlesource.com/c/1366359 > > Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org> > > Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#614440} > > TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > Change-Id: I3981455a00f743fae535de4626179dcceab9b2c4 > Reviewed-on: https://chromium-review.googlesource.com/c/1372236 > Reviewed-by: Peter Boström <pbos@chromium.org> > Commit-Queue: Peter Boström <pbos@chromium.org> > Cr-Commit-Position: refs/heads/master@{#615581} TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org Change-Id: Ib9599bb8fd44327ae756d3c9523049367409612c Bug: chromium:912543 , chromium:913135 , chromium:913784 , chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Reviewed-on: https://chromium-review.googlesource.com/c/1373832 Commit-Queue: Friedrich Horschig [CET] <fhorschig@chromium.org> Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org> Cr-Commit-Position: refs/heads/master@{#615878} [modify] https://crrev.com/6921bd825e416c7b412b454f60df8b183e906943/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/6921bd825e416c7b412b454f60df8b183e906943/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/6921bd825e416c7b412b454f60df8b183e906943/chrome/common/chrome_features.cc
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/831dad6e5036b31b683587078db3df8e32ce4f92 commit 831dad6e5036b31b683587078db3df8e32ce4f92 Author: Peter Boström <pbos@chromium.org> Date: Thu Dec 13 16:50:04 2018 Reland "Reland "Disable the new tab-loading animation"" This reverts commit 6921bd825e416c7b412b454f60df8b183e906943. Reason for revert: NOTE TO SHERIFFS: Both the revert and reland of this CL is the culprit for FindIt flakes. Context: This CL changes the duration of the tab-icon animation ("spinner"). This likely has some effects on test loops that RunUntilIdle or similar, so a pending request might either have happened or not before that loop gets idle. For instance: crbug.com/912543 had to be changed to ignore a favicon.ico response. This flakiness could be reproduced by just changing TabIcon::ShowingLoadingAnimation to "return false;" even though the test does not have anything to do with Chrome's UI directly. This reland will likely trigger crbug.com/914232 again, but the revert triggered crbug.com/914644 . It's not unlikely that they are both flaky as they implicitly rely on tab-icon load time. Bug: chromium:914232, chromium:914644 Original change's description: > Revert "Reland "Disable the new tab-loading animation"" > > This reverts commit ad12d44124812f22484eb4d123e4a3b4aaac93b9. > > Reason for revert: > Findit found this to be the most likely culprit for the single_process_mash_browser_tests failures on various Linux bots: > https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/30518 > > > Original change's description: > > Reland "Disable the new tab-loading animation" > > > > This reverts commit 742b8857c4dd0ab4550f7b2c1f200ce5be406b9c. > > > > Reason for revert: Flaky tests should be fixed in r615470. > > > > Bug: chromium:912543 , chromium:913135 , chromium:913784 > > > > Original change's description: > > > Revert "Disable the new tab-loading animation" > > > > > > This reverts commit 355b8185716292a8f145c77168ee89148ecabf31. > > > > > > Reason for revert: Made several tests flaky: https://crbug.com/912543 > > > > > > Original change's description: > > > > Disable the new tab-loading animation > > > > > > > > Makes sure that a lot of animation-related code is bypassed when the > > > > new-tab-animation flag is off. This should hopefully fix a couple of > > > > performance regressions that have not yet been root caused so that they > > > > don't go out with M72. > > > > > > > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > > > Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 > > > > Reviewed-on: https://chromium-review.googlesource.com/c/1364212 > > > > Reviewed-by: Sidney San Martín <sdy@chromium.org> > > > > Commit-Queue: Peter Boström <pbos@chromium.org> > > > > Cr-Commit-Position: refs/heads/master@{#614199} > > > > > > TBR=pbos@chromium.org,sdy@chromium.org > > > > > > Change-Id: Ib4c022a255ad085c1716d3559a7f84dcb61c2785 > > > No-Presubmit: true > > > No-Tree-Checks: true > > > No-Try: true > > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > > Reviewed-on: https://chromium-review.googlesource.com/c/1366359 > > > Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org> > > > Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org> > > > Cr-Commit-Position: refs/heads/master@{#614440} > > > > TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org > > > > # Not skipping CQ checks because original CL landed > 1 day ago. > > > > Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > > Change-Id: I3981455a00f743fae535de4626179dcceab9b2c4 > > Reviewed-on: https://chromium-review.googlesource.com/c/1372236 > > Reviewed-by: Peter Boström <pbos@chromium.org> > > Commit-Queue: Peter Boström <pbos@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#615581} > > TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org > > Change-Id: Ib9599bb8fd44327ae756d3c9523049367409612c > Bug: chromium:912543 , chromium:913135 , chromium:913784 , chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 > Reviewed-on: https://chromium-review.googlesource.com/c/1373832 > Commit-Queue: Friedrich Horschig [CET] <fhorschig@chromium.org> > Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org> > Cr-Commit-Position: refs/heads/master@{#615878} TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org,fhorschig@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:912543 , chromium:913135 , chromium:913784 , chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Change-Id: Ib8083b1320f3e1cd5c878bd55d2a007af7bf719a Reviewed-on: https://chromium-review.googlesource.com/c/1376030 Reviewed-by: Peter Boström <pbos@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#616334} [modify] https://crrev.com/831dad6e5036b31b683587078db3df8e32ce4f92/chrome/browser/ui/views/tabs/tab_icon.cc [modify] https://crrev.com/831dad6e5036b31b683587078db3df8e32ce4f92/chrome/browser/ui/views/tabs/tab_unittest.cc [modify] https://crrev.com/831dad6e5036b31b683587078db3df8e32ce4f92/chrome/common/chrome_features.cc
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1cbde78f21bba5d565b93ac1236e9ff00ed7ff24 Commit: 1cbde78f21bba5d565b93ac1236e9ff00ed7ff24 Author: pbos@chromium.org Commiter: pbos@chromium.org Date: 2018-12-06 18:09:10 +0000 UTC Disable the new tab-loading animation Makes sure that a lot of animation-related code is bypassed when the new-tab-animation flag is off. This should hopefully fix a couple of performance regressions that have not yet been root caused so that they don't go out with M72. Bug: chromium:912328 , chromium:905745 , chromium:905918 , chromium:910265 Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8 Reviewed-on: https://chromium-review.googlesource.com/c/1364212 Reviewed-by: Sidney San Martín <sdy@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#614199}(cherry picked from commit 355b8185716292a8f145c77168ee89148ecabf31) Reviewed-on: https://chromium-review.googlesource.com/c/1365983 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#118} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Dec 6