Skip to content

Commit

Permalink
Revert "Cros: Report metrics for the animation of traslucent background"
Browse files Browse the repository at this point in the history 
This reverts commit 64465df.

Reason for revert: Speculative revert to fix use-of-uninitialized member flake / crash; see https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8886180914914709040/+/steps/browser_tests/0/logs/Deterministic_failure:_PublicSessionOobeTestImpl__x2f_PublicSessionOobeTest.NoTermsOfService__x2f_4__status_CRASH_/0

==18002==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x5640af44e1e6 in ui::AnimationMetricsRecorder::OnAnimationEnd(base::Optional<int>, float) ./../../ui/compositor/animation_metrics_recorder.cc:64:14
    #1 0x5640af4e4b37 in ui::LayerAnimationElement::ProgressToEnd(ui::LayerAnimationDelegate*) ./../../ui/compositor/layer_animation_element.cc:669:34
    #2 0x5640af4fbb95 in ui::LayerAnimationSequence::ProgressToEnd(ui::LayerAnimationDelegate*) ./../../ui/compositor/layer_animation_sequence.cc:147:35
    #3 0x5640af513c54 in ProgressAnimationToEnd ./../../ui/compositor/layer_animator.cc:476:13
    #4 0x5640af513c54 in ui::LayerAnimator::FinishAnimation(ui::LayerAnimationSequence*, bool) ./../../ui/compositor/layer_animator.cc:622:5
    #5 0x5640af519c6d in ui::LayerAnimator::StopAnimatingInternal(bool) ./../../ui/compositor/layer_animator.cc:533:5
    #6 0x5640af489c93 in StopAnimating ./../../ui/compositor/layer_animator.h:198:26
    #7 0x5640af489c93 in ui::Layer::CompleteAllAnimations() ./../../ui/compositor/layer.cc:1128:12
    #8 0x5640af3c6ff1 in aura::Window::~Window() ./../../ui/aura/window.cc:124:14
    #9 0x5640af3ca7ac in aura::Window::~Window() ./../../ui/aura/window.cc:119:19
    #10 0x5640a9a51655 in views::NativeWidgetAura::~NativeWidgetAura() ./../../ui/views/widget/native_widget_aura.cc:0:0
    #11 0x5640a9a51b9c in views::NativeWidgetAura::~NativeWidgetAura() ./../../ui/views/widget/native_widget_aura.cc:1054:39
    #12 0x5640a99ba714 in views::Widget::~Widget() ./../../ui/views/widget/widget.cc:185:5
    #13 0x5640b72c7e39 in ash::HotseatWidget::~HotseatWidget() ./../../ash/shelf/hotseat_widget.cc:354:1
    #14 0x5640b72c7f9c in ash::HotseatWidget::~HotseatWidget() ./../../ash/shelf/hotseat_widget.cc:350:33
    #15 0x5640b7319b53 in operator() ./../../buildtools/third_party/libc++/trunk/include/memory:2378:5
    #16 0x5640b7319b53 in reset ./../../buildtools/third_party/libc++/trunk/include/memory:2633:7
    #17 0x5640b7319b53 in ash::Shelf::ShutdownShelfWidget() ./../../ash/shelf/shelf.cc:377:19
    #18 0x5640b724c95f in ash::RootWindowController::CloseChildWindows() ./../../ash/root_window_controller.cc:659:11
    #19 0x5640b741f9c5 in ash::Shell::CloseAllRootWindowChildWindows() ./../../ash/shell.cc:1269:19
    #20 0x5640b741532a in ash::Shell::~Shell() ./../../ash/shell.cc:720:3
    #21 0x5640b741fd3c in ash::Shell::~Shell() ./../../ash/shell.cc:589:17
    #22 0x5640b9be4d75 in operator() ./../../buildtools/third_party/libc++/trunk/include/memory:2378:5
    #23 0x5640b9be4d75 in reset ./../../buildtools/third_party/libc++/trunk/include/memory:2633:7
    #24 0x5640b9be4d75 in ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() ./../../chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc:245:19
    #25 0x5640a5abe85d in ChromeBrowserMainParts::PostMainMessageLoopRun() ./../../chrome/browser/chrome_browser_main.cc:1721:29
    #26 0x564094a9d525 in chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() ./../../chrome/browser/chromeos/chrome_browser_main_chromeos.cc:1136:32
    #27 0x56409b175e3f in content::BrowserMainLoop::ShutdownThreadsAndCleanUp() ./../../content/browser/browser_main_loop.cc:1095:13
    #28 0x56409b17ea9a in content::BrowserMainRunnerImpl::Shutdown() ./../../content/browser/browser_main_runner_impl.cc:178:17
    #29 0x56409b165bd4 in content::BrowserMain(content::MainFunctionParams const&) ./../../content/browser/browser_main.cc:49:16
    #30 0x5640a3b3284b in RunBrowserProcessMain ./../../content/app/content_main_runner_impl.cc:529:10
    #31 0x5640a3b3284b in content::ContentMainRunnerImpl::RunServiceManager(content::MainFunctionParams&, bool) ./../../content/app/content_main_runner_impl.cc:978:10
    #32 0x5640a3b311b2 in content::ContentMainRunnerImpl::Run(bool) ./../../content/app/content_main_runner_impl.cc:878:12
    #33 0x5640b1028f28 in service_manager::Main(service_manager::MainParams const&) ./../../services/service_manager/embedder/main.cc:455:29
    #34 0x56409f705fd0 in content::ContentMain(content::ContentMainParams const&) ./../../content/app/content_main.cc:19:10
    #35 0x5640a762696a in content::BrowserTestBase::SetUp() ./../../content/public/test/browser_test_base.cc:513:3
    chromium#36 0x5640a53fb49d in InProcessBrowserTest::SetUp() ./../../chrome/test/base/in_process_browser_test.cc:299:20
    #37 0x5640960c6b50 in HandleExceptionsInMethodIfSupported<testing::Test, void> ./../../third_party/googletest/src/googletest/src/gtest.cc:0:0
    #38 0x5640960c6b50 in testing::Test::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2663:3
    chromium#39 0x5640960cb24d in testing::TestInfo::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2845:11
    #40 0x5640960cd5f9 in testing::TestSuite::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:2977:28
    #41 0x564096106f6e in testing::internal::UnitTestImpl::RunAllTests() ./../../third_party/googletest/src/googletest/src/gtest.cc:5518:44
    chromium#42 0x5640961053fd in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> ./../../third_party/googletest/src/googletest/src/gtest-internal-inl.h:0:10
    #43 0x5640961053fd in testing::UnitTest::Run() ./../../third_party/googletest/src/googletest/src/gtest.cc:5105:10
    #44 0x5640a5902835 in RUN_ALL_TESTS ./../../third_party/googletest/src/googletest/include/gtest/gtest.h:2472:46
    #45 0x5640a5902835 in base::TestSuite::Run() ./../../base/test/test_suite.cc:458:16
    #46 0x5640a5393dd1 in BrowserTestSuiteRunnerChromeOS::RunTestSuite(int, char**) ./../../chrome/test/base/browser_tests_main_chromeos.cc:37:23
    #47 0x5640a773a27e in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) ./../../content/public/test/test_launcher.cc:375:31
    #48 0x5640a5394f45 in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) ./../../chrome/test/base/chrome_test_launcher.cc:246:10
    #49 0x5640a5393bb8 in main ./../../chrome/test/base/browser_tests_main_chromeos.cc:52:10
    #50 0x7f194b36282f in __libc_start_main /build/glibc-LK5gWL/glibc-2.23/csu/../csu/libc-start.c:291:0
    #51 0x56408b4293c9 in _start ??:0:0

  Uninitialized value was created by a heap deallocation
    #0 0x56408b497790 in operator delete(void*) /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/msan/msan_new_delete.cpp:74:44
    #1 0x5640b72c7d74 in operator() ./../../buildtools/third_party/libc++/trunk/include/memory:2378:5
    #2 0x5640b72c7d74 in reset ./../../buildtools/third_party/libc++/trunk/include/memory:2633:7
    #3 0x5640b72c7d74 in ~unique_ptr ./../../buildtools/third_party/libc++/trunk/include/memory:2587:19
    #4 0x5640b72c7d74 in ash::HotseatWidget::~HotseatWidget() ./../../ash/shelf/hotseat_widget.cc:354:1
    #5 0x5640b72c7f9c in ash::HotseatWidget::~HotseatWidget() ./../../ash/shelf/hotseat_widget.cc:350:33
    #6 0x5640b7319b53 in operator() ./../../buildtools/third_party/libc++/trunk/include/memory:2378:5
    #7 0x5640b7319b53 in reset ./../../buildtools/third_party/libc++/trunk/include/memory:2633:7
    #8 0x5640b7319b53 in ash::Shelf::ShutdownShelfWidget() ./../../ash/shelf/shelf.cc:377:19
    #9 0x5640b724c95f in ash::RootWindowController::CloseChildWindows() ./../../ash/root_window_controller.cc:659:11
    #10 0x5640b741f9c5 in ash::Shell::CloseAllRootWindowChildWindows() ./../../ash/shell.cc:1269:19
    #11 0x5640b741532a in ash::Shell::~Shell() ./../../ash/shell.cc:720:3
    #12 0x5640b741fd3c in ash::Shell::~Shell() ./../../ash/shell.cc:589:17
    #13 0x5640b9be4d75 in operator() ./../../buildtools/third_party/libc++/trunk/include/memory:2378:5
    #14 0x5640b9be4d75 in reset ./../../buildtools/third_party/libc++/trunk/include/memory:2633:7
    #15 0x5640b9be4d75 in ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() ./../../chrome/browser/ui/ash/chrome_browser_main_extra_parts_ash.cc:245:19
    #16 0x5640a5abe85d in ChromeBrowserMainParts::PostMainMessageLoopRun() ./../../chrome/browser/chrome_browser_main.cc:1721:29
    #17 0x564094a9d525 in chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() ./../../chrome/browser/chromeos/chrome_browser_main_chromeos.cc:1136:32
    #18 0x56409b175e3f in content::BrowserMainLoop::ShutdownThreadsAndCleanUp() ./../../content/browser/browser_main_loop.cc:1095:13
    #19 0x56409b17ea9a in content::BrowserMainRunnerImpl::Shutdown() ./../../content/browser/browser_main_runner_impl.cc:178:17
    #20 0x56409b165bd4 in content::BrowserMain(content::MainFunctionParams const&) ./../../content/browser/browser_main.cc:49:16
    #21 0x5640a3b3284b in RunBrowserProcessMain ./../../content/app/content_main_runner_impl.cc:529:10
    #22 0x5640a3b3284b in content::ContentMainRunnerImpl::RunServiceManager(content::MainFunctionParams&, bool) ./../../content/app/content_main_runner_impl.cc:978:10
    #23 0x5640a3b311b2 in content::ContentMainRunnerImpl::Run(bool) ./../../content/app/content_main_runner_impl.cc:878:12
    #24 0x5640b1028f28 in service_manager::Main(service_manager::MainParams const&) ./../../services/service_manager/embedder/main.cc:455:29
    #25 0x56409f705fd0 in content::ContentMain(content::ContentMainParams const&) ./../../content/app/content_main.cc:19:10
    #26 0x5640a762696a in content::BrowserTestBase::SetUp() ./../../content/public/test/browser_test_base.cc:513:3
    #27 0x5640a53fb49d in InProcessBrowserTest::SetUp() ./../../chrome/test/base/in_process_browser_test.cc:299:20

Original change's description:
> Cros: Report metrics for the animation of traslucent background
> 
> This animation happens during transition between hotseat states.
> 
> Bug: 1058609
> Change-Id: Iff1f81625e6de2bf6dc6dcfee09225c4d16cc53e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2088641
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Manu Cornet <manucornet@chromium.org>
> Reviewed-by: Alex Newcomer <newcomer@chromium.org>
> Commit-Queue: Ana Salazar <anasalazar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#748806}

TBR=rkaplow@chromium.org,newcomer@chromium.org,manucornet@chromium.org,anasalazar@chromium.org

Change-Id: I28dfaa49cae6e27801975a2863c013b9f9a5edca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1058609
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097507
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749146}
  • Loading branch information
Joshua Pawlicki authored and Commit Bot committed Mar 11, 2020
1 parent 3cf1b43 commit 9fcd754
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 89 deletions.
85 changes: 19 additions & 66 deletions ash/shelf/hotseat_widget.cc
View file
Expand Up @@ -84,53 +84,6 @@ class HotseatWindowTargeter : public aura::WindowTargeter {

} // namespace

// Records smoothness of animations for background of the hotseat widget.
class HotseatWidgetBackgroundAnimationMetricsReporter
: public HotseatTransitionAnimator::Observer,
public ui::AnimationMetricsReporter {
public:
explicit HotseatWidgetBackgroundAnimationMetricsReporter(HotseatState state)
: target_state_(state) {}

~HotseatWidgetBackgroundAnimationMetricsReporter() override = default;

void OnHotseatTransitionAnimationWillStart(HotseatState from_state,
HotseatState to_state) override {
target_state_ = to_state;
}

// ui::AnimationMetricsReporter:
void Report(int value) override {
switch (target_state_) {
case HotseatState::kShownClamshell:
case HotseatState::kShownHomeLauncher:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.TranslucentBackground."
"AnimationSmoothness.TransitionToShownHotseat",
value);
break;
case HotseatState::kExtended:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.TranslucentBackground."
"AnimationSmoothness.TransitionToExtendedHotseat",
value);
break;
case HotseatState::kHidden:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.TranslucentBackground."
"AnimationSmoothness.TransitionToHiddenHotseat",
value);
break;
default:
NOTREACHED();
}
}

private:
// The state to which the animation is transitioning.
HotseatState target_state_;
};

class HotseatWidget::DelegateView : public HotseatTransitionAnimator::Observer,
public views::WidgetDelegateView,
public OverviewObserver,
Expand All @@ -143,8 +96,7 @@ class HotseatWidget::DelegateView : public HotseatTransitionAnimator::Observer,

// Initializes the view.
void Init(ScrollableShelfView* scrollable_shelf_view,
ui::Layer* parent_layer,
ui::AnimationMetricsReporter* background_metrics_reporter);
ui::Layer* parent_layer);

// Updates the hotseat background.
void UpdateTranslucentBackground();
Expand Down Expand Up @@ -189,8 +141,6 @@ class HotseatWidget::DelegateView : public HotseatTransitionAnimator::Observer,
ScrollableShelfView* scrollable_shelf_view_ = nullptr; // unowned.
// Blur is disabled during animations to improve performance.
bool blur_lock_ = false;
// Owned by the Hotseat Widget.
ui::AnimationMetricsReporter* background_metrics_reporter_;

// The most recent color that the |translucent_background_| has been animated
// to.
Expand All @@ -202,7 +152,8 @@ class HotseatWidget::DelegateView : public HotseatTransitionAnimator::Observer,
HotseatWidget::DelegateView::~DelegateView() {
WallpaperControllerImpl* wallpaper_controller =
Shell::Get()->wallpaper_controller();
OverviewController* overview_controller = Shell::Get()->overview_controller();
OverviewController* overview_controller =
Shell::Get()->overview_controller();
if (wallpaper_controller)
wallpaper_controller->RemoveObserver(this);
if (overview_controller)
Expand All @@ -211,13 +162,13 @@ HotseatWidget::DelegateView::~DelegateView() {

void HotseatWidget::DelegateView::Init(
ScrollableShelfView* scrollable_shelf_view,
ui::Layer* parent_layer,
ui::AnimationMetricsReporter* background_metrics_reporter) {
ui::Layer* parent_layer) {
SetLayoutManager(std::make_unique<views::FillLayout>());

WallpaperControllerImpl* wallpaper_controller =
Shell::Get()->wallpaper_controller();
OverviewController* overview_controller = Shell::Get()->overview_controller();
OverviewController* overview_controller =
Shell::Get()->overview_controller();
if (wallpaper_controller)
wallpaper_controller->AddObserver(this);
if (overview_controller)
Expand All @@ -227,7 +178,6 @@ void HotseatWidget::DelegateView::Init(
DCHECK(scrollable_shelf_view);
scrollable_shelf_view_ = scrollable_shelf_view;
UpdateTranslucentBackground();
background_metrics_reporter_ = background_metrics_reporter;
}

void HotseatWidget::DelegateView::UpdateTranslucentBackground() {
Expand Down Expand Up @@ -262,8 +212,6 @@ void HotseatWidget::DelegateView::SetTranslucentBackground(
animation_setter.SetTweenType(gfx::Tween::EASE_OUT);
animation_setter.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
if (animate)
animation_setter.SetAnimationMetricsReporter(background_metrics_reporter_);

if (ShelfConfig::Get()->GetDefaultShelfColor() != target_color_) {
target_color_ = ShelfConfig::Get()->GetDefaultShelfColor();
Expand Down Expand Up @@ -365,14 +313,19 @@ void HotseatWidget::Initialize(aura::Window* container, Shelf* shelf) {
set_focus_on_creation(false);
GetFocusManager()->set_arrow_key_traversal_enabled_for_widget(true);

scrollable_shelf_view_ = GetContentsView()->AddChildView(
std::make_unique<ScrollableShelfView>(ShelfModel::Get(), shelf));
scrollable_shelf_view_->Init();
traslucent_background_metrics_reporter_ =
std::make_unique<HotseatWidgetBackgroundAnimationMetricsReporter>(
state());
delegate_view_->Init(scrollable_shelf_view(), GetLayer(),
traslucent_background_metrics_reporter_.get());
if (IsScrollableShelfEnabled()) {
scrollable_shelf_view_ = GetContentsView()->AddChildView(
std::make_unique<ScrollableShelfView>(ShelfModel::Get(), shelf));
scrollable_shelf_view_->Init();
} else {
// The shelf view observes the shelf model and creates icons as items are
// added to the model.
shelf_view_ = GetContentsView()->AddChildView(std::make_unique<ShelfView>(
ShelfModel::Get(), shelf, /*drag_and_drop_host=*/nullptr,
/*shelf_button_delegate=*/nullptr));
shelf_view_->Init();
}
delegate_view_->Init(scrollable_shelf_view(), GetLayer());
}

void HotseatWidget::OnHotseatTransitionAnimatorCreated(
Expand Down
6 changes: 0 additions & 6 deletions ash/shelf/hotseat_widget.h
View file
Expand Up @@ -23,7 +23,6 @@ class ScrollableShelfView;
class Shelf;
class ShelfView;
class HotseatTransitionAnimator;
class HotseatWidgetBackgroundAnimationMetricsReporter;

// The hotseat widget is part of the shelf and hosts app shortcuts.
class ASH_EXPORT HotseatWidget : public ShelfComponent,
Expand Down Expand Up @@ -159,11 +158,6 @@ class ASH_EXPORT HotseatWidget : public ShelfComponent,
// during an animation.
std::unique_ptr<aura::ScopedWindowTargeter> hotseat_window_targeter_;

// Metrics reporter for animations of the traslucent background in the
// hotseat.
std::unique_ptr<HotseatWidgetBackgroundAnimationMetricsReporter>
traslucent_background_metrics_reporter_;

DISALLOW_COPY_AND_ASSIGN(HotseatWidget);
};

Expand Down
6 changes: 3 additions & 3 deletions ash/shelf/shelf.cc
View file
Expand Up @@ -79,19 +79,19 @@ class HotseatWidgetAnimationMetricsReporter
case HotseatState::kShownClamshell:
case HotseatState::kShownHomeLauncher:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.Widget.AnimationSmoothness."
"Ash.HotseatWidgetAnimation.AnimationSmoothness."
"TransitionToShownHotseat",
value);
break;
case HotseatState::kExtended:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.Widget.AnimationSmoothness."
"Ash.HotseatWidgetAnimation.AnimationSmoothness."
"TransitionToExtendedHotseat",
value);
break;
case HotseatState::kHidden:
UMA_HISTOGRAM_PERCENTAGE(
"Ash.HotseatWidgetAnimation.Widget.AnimationSmoothness."
"Ash.HotseatWidgetAnimation.AnimationSmoothness."
"TransitionToHiddenHotseat",
value);
break;
Expand Down
17 changes: 3 additions & 14 deletions tools/metrics/histograms/histograms.xml
View file
Expand Up @@ -8800,16 +8800,13 @@ uploading your change for review.
<!-- Name completed by histogram suffixes
name="HotseatTransitionType" -->

<!-- Name completed by histogram suffixes
name="HotseatWidgetElement" -->

<owner>anasalazar@chromium.org</owner>
<owner>newcomer@chromium.org</owner>
<summary>
Tracks the animation smoothness for the bounds animation of the hotseat
widget's elements during transitions of the hotseat to shown, extended, and
hidden hotseat states. Check Ash.HotseatTransition.AnimationSmoothness for
smoothness of the shelf's animating background.
widget during transitions of the hotseat to shown, extended, and hidden
hotseat states. Check Ash.HotseatTransition.AnimationSmoothness for
smoothness of the animating background.
</summary>
</histogram>

Expand Down Expand Up @@ -187169,14 +187166,6 @@ regressions. -->
<affected-histogram name="Ash.NavigationWidget.AnimationSmoothness"/>
</histogram_suffixes>

<histogram_suffixes name="HotseatWidgetElement" separator="."
ordering="prefix,2">
<suffix name="TranslucentBackground"
label="Hotseat widget's translucent background"/>
<suffix name="Widget" label="Hotseat widget"/>
<affected-histogram name="Ash.HotseatWidgetAnimation.AnimationSmoothness"/>
</histogram_suffixes>

<histogram_suffixes name="HstsState" separator=".">
<suffix name="HSTSNotEnabled" label="The HSTS is not enabled."/>
<suffix name="WithHSTSEnabled" label="The HSTS is enabled."/>
Expand Down

0 comments on commit 9fcd754

Please sign in to comment.