Commit dad9f500 authored by Max Morin's avatar Max Morin Committed by Commit Bot

Remove power monitoring from AudioThreadHangMonitor.

The current code has two bugs.
1. PowerMonitor::RemoveObserver isn't called at destruction. Oops.
2. PowerMonitor::AddObserver is called from a task scheduler
thread, which can race with the power monitor being destructed on
the main thread.

1 is straightforward to fix but 2 isn't really. The power monitor
isn't adding much value, so let's just remove it.

Bug: 912922, 912997
Change-Id: I11cbc028d6ef4a9767096f2553d1cc340aa86e57
Reviewed-on: https://chromium-review.googlesource.com/c/1371390Reviewed-by: default avatarOlga Sharonova <olka@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615493}
parent 0c41037a
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/debug/alias.h"
#include "base/debug/dump_without_crashing.h" #include "base/debug/dump_without_crashing.h"
#include "base/location.h" #include "base/location.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
...@@ -80,12 +79,6 @@ AudioThreadHangMonitor::AudioThreadHangMonitor( ...@@ -80,12 +79,6 @@ AudioThreadHangMonitor::AudioThreadHangMonitor(
void AudioThreadHangMonitor::StartTimer() { void AudioThreadHangMonitor::StartTimer() {
DCHECK_CALLED_ON_VALID_SEQUENCE(monitor_sequence_); DCHECK_CALLED_ON_VALID_SEQUENCE(monitor_sequence_);
auto* pm = base::PowerMonitor::Get();
if (pm) {
// May be null in unit tests.
pm->AddObserver(this);
}
// Set the flag to true so that the first run doesn't detect a hang. // Set the flag to true so that the first run doesn't detect a hang.
alive_flag_->flag_ = true; alive_flag_->flag_ = true;
...@@ -110,12 +103,6 @@ bool AudioThreadHangMonitor::NeverLoggedThreadRecoveredAfterHung() const { ...@@ -110,12 +103,6 @@ bool AudioThreadHangMonitor::NeverLoggedThreadRecoveredAfterHung() const {
return audio_thread_status_ == ThreadStatus::kHung; return audio_thread_status_ == ThreadStatus::kHung;
} }
void AudioThreadHangMonitor::OnResume() {
DCHECK_CALLED_ON_VALID_SEQUENCE(monitor_sequence_);
last_resume_time_ = clock_->NowTicks();
recent_ping_state_ = 0;
}
void AudioThreadHangMonitor::CheckIfAudioThreadIsAlive() { void AudioThreadHangMonitor::CheckIfAudioThreadIsAlive() {
DCHECK_CALLED_ON_VALID_SEQUENCE(monitor_sequence_); DCHECK_CALLED_ON_VALID_SEQUENCE(monitor_sequence_);
...@@ -152,9 +139,6 @@ void AudioThreadHangMonitor::CheckIfAudioThreadIsAlive() { ...@@ -152,9 +139,6 @@ void AudioThreadHangMonitor::CheckIfAudioThreadIsAlive() {
LogHistogramThreadStatus(); LogHistogramThreadStatus();
if (dump_on_hang_) { if (dump_on_hang_) {
int64_t time_since_resume =
(clock_->NowTicks() - last_resume_time_).InMilliseconds();
base::debug::Alias(&time_since_resume);
base::debug::DumpWithoutCrashing(); base::debug::DumpWithoutCrashing();
} }
} }
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/power_monitor/power_observer.h"
#include "base/sequence_checker.h" #include "base/sequence_checker.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/time/time.h" #include "base/time/time.h"
...@@ -34,7 +33,7 @@ namespace media { ...@@ -34,7 +33,7 @@ namespace media {
// posting a task to the audio thread every minute and checking that it was // posting a task to the audio thread every minute and checking that it was
// executed. If three consecutive such pings are missed, the thread is // executed. If three consecutive such pings are missed, the thread is
// considered hung. // considered hung.
class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver { class MEDIA_EXPORT AudioThreadHangMonitor final {
public: public:
using Ptr = using Ptr =
std::unique_ptr<AudioThreadHangMonitor, base::OnTaskRunnerDeleter>; std::unique_ptr<AudioThreadHangMonitor, base::OnTaskRunnerDeleter>;
...@@ -57,7 +56,7 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver { ...@@ -57,7 +56,7 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver {
scoped_refptr<base::SingleThreadTaskRunner> audio_thread_task_runner, scoped_refptr<base::SingleThreadTaskRunner> audio_thread_task_runner,
scoped_refptr<base::SequencedTaskRunner> monitor_task_runner = nullptr); scoped_refptr<base::SequencedTaskRunner> monitor_task_runner = nullptr);
~AudioThreadHangMonitor() final; ~AudioThreadHangMonitor();
// Thread-safe. // Thread-safe.
bool IsAudioThreadHung() const; bool IsAudioThreadHung() const;
...@@ -85,10 +84,6 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver { ...@@ -85,10 +84,6 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver {
bool NeverLoggedThreadHung() const; bool NeverLoggedThreadHung() const;
bool NeverLoggedThreadRecoveredAfterHung() const; bool NeverLoggedThreadRecoveredAfterHung() const;
// base::PowerObserver overrides.
// Reset hang detection state when the system comes out of the suspend state.
void OnResume() final;
// This function is run by the |timer_|. It checks if the audio thread has // This function is run by the |timer_|. It checks if the audio thread has
// shown signs of life since the last time it was called (by checking the // shown signs of life since the last time it was called (by checking the
// |alive_flag_|) and updates the value of |successful_pings_| and // |alive_flag_|) and updates the value of |successful_pings_| and
...@@ -126,9 +121,6 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver { ...@@ -126,9 +121,6 @@ class MEDIA_EXPORT AudioThreadHangMonitor final : public base::PowerObserver {
// detecting the audio thread as hung. // detecting the audio thread as hung.
base::TimeTicks last_check_time_ = base::TimeTicks(); base::TimeTicks last_check_time_ = base::TimeTicks();
// Used to investigate hangs.
base::TimeTicks last_resume_time_ = base::TimeTicks();
// |recent_ping_state_| tracks the recent life signs from the audio thread. If // |recent_ping_state_| tracks the recent life signs from the audio thread. If
// the most recent ping was successful, the number indicates the number of // the most recent ping was successful, the number indicates the number of
// successive successful pings. If the most recent ping was failed, the number // successive successful pings. If the most recent ping was failed, the number
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment