Commit d7ba6c1c authored by Marina Ciocea's avatar Marina Ciocea Committed by Commit Bot

Remove temporary global histogram allocator check from audio service output controller.

This reverts commit 57c883df.

Reason for revert: Temporary check is no longer needed because we gathered enough
data for investigating https://crbug.com/867827#c40. Conclusion: global allocator
is initialized by the time audio service reports UMA metrics.

Original change's description:
> Temporary check global histogram allocator in audio service output controller.
>
> Add a temporary check to investigate audio service experiment output controller
> histograms count mismatch. If histogram persistent memory is not setup before
> output controller metrics are reported, the check will fail
> (see https://crbug.com/867827#c40). This CL will be reverted in a couple of days
> after gathering data from canary.
>
> Bug: 867827
> Change-Id: I08cce13476f851d9ab24434add30f65abbf1e600
> Reviewed-on: https://chromium-review.googlesource.com/1215246
> Reviewed-by: Olga Sharonova <olka@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Marina Ciocea <marinaciocea@chromium.org>
> Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589879}

TBR=guidou@chromium.org,olka@chromium.org,marinaciocea@chromium.org


Bug: 867827
Change-Id: Ie9f4ebabf8dfd4773466be0e75d878ce3508f19e
Reviewed-on: https://chromium-review.googlesource.com/1250881
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarMarina Ciocea <marinaciocea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595029}
parent 12e458fa
......@@ -4,7 +4,6 @@
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h"
......@@ -39,15 +38,6 @@ class WebRtcAudioBrowserTest : public WebRtcContentBrowserTestBase,
// Force audio service out of process to disabled.
audio_service_features_.InitWithFeatures({}, audio_service_oop_features);
}
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator creation when
// removing output controller checks.
if (!base::GlobalHistogramAllocator::Get()) {
const int32_t kAllocatorMemorySize = 8 << 20;
base::GlobalHistogramAllocator::CreateWithLocalMemory(
kAllocatorMemorySize, 0, "HistogramAllocatorTest");
}
#endif
}
~WebRtcAudioBrowserTest() override {}
......
......@@ -7,7 +7,6 @@
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/json/json_reader.h"
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "base/values.h"
......@@ -119,15 +118,6 @@ class WebRtcGetUserMediaBrowserTest : public WebRtcContentBrowserTestBase,
// Force audio service out of process to disabled.
audio_service_features_.InitWithFeatures({}, audio_service_oop_features);
}
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator creation when
// removing output controller checks.
if (!base::GlobalHistogramAllocator::Get()) {
const int32_t kAllocatorMemorySize = 8 << 20;
base::GlobalHistogramAllocator::CreateWithLocalMemory(
kAllocatorMemorySize, 0, "HistogramAllocatorTest");
}
#endif
}
~WebRtcGetUserMediaBrowserTest() override {}
......
......@@ -12,14 +12,12 @@
#include "base/bind_helpers.h"
#include "base/compiler_specific.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/numerics/safe_conversions.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "base/task_runner_util.h"
#include "base/threading/platform_thread.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
#include "media/base/audio_timestamp_helper.h"
#include "services/audio/stream_monitor.h"
......@@ -45,10 +43,6 @@ enum StreamCreationResult {
void LogStreamCreationResult(bool for_device_change,
StreamCreationResult result) {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
if (for_device_change) {
UMA_HISTOGRAM_ENUMERATION(
"Media.AudioOutputController.ProxyStreamCreationResultForDeviceChange",
......@@ -97,10 +91,6 @@ OutputController::ErrorStatisticsTracker::ErrorStatisticsTracker()
}
OutputController::ErrorStatisticsTracker::~ErrorStatisticsTracker() {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
UMA_HISTOGRAM_LONG_TIMES("Media.OutputStreamDuration",
base::TimeTicks::Now() - start_time_);
UMA_HISTOGRAM_BOOLEAN("Media.AudioOutputController.CallbackError",
......@@ -166,10 +156,6 @@ OutputController::~OutputController() {
}
bool OutputController::Create(bool is_for_device_change) {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
DCHECK(task_runner_->BelongsToCurrentThread());
SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.CreateTime");
TRACE_EVENT0("audio", "OutputController::Create");
......@@ -240,10 +226,6 @@ bool OutputController::Create(bool is_for_device_change) {
}
void OutputController::Play() {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
DCHECK(task_runner_->BelongsToCurrentThread());
SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.PlayTime");
TRACE_EVENT0("audio", "OutputController::Play");
......@@ -289,10 +271,6 @@ void OutputController::StopStream() {
}
void OutputController::Pause() {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
DCHECK(task_runner_->BelongsToCurrentThread());
SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.PauseTime");
TRACE_EVENT0("audio", "OutputController::Pause");
......@@ -312,10 +290,6 @@ void OutputController::Pause() {
}
void OutputController::Close() {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
DCHECK(task_runner_->BelongsToCurrentThread());
SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.CloseTime");
TRACE_EVENT0("audio", "OutputController::Close");
......@@ -567,10 +541,6 @@ void OutputController::OnMemberLeftGroup(StreamMonitor* monitor) {
}
void OutputController::OnDeviceChange() {
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator check.
CHECK(base::GlobalHistogramAllocator::Get());
#endif
DCHECK(task_runner_->BelongsToCurrentThread());
SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.DeviceChangeTime");
TRACE_EVENT0("audio", "OutputController::OnDeviceChange");
......
......@@ -18,7 +18,6 @@
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/optional.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
......@@ -27,7 +26,6 @@
#include "base/threading/thread.h"
#include "base/time/time.h"
#include "base/unguessable_token.h"
#include "build/build_config.h"
#include "media/audio/audio_device_description.h"
#include "media/audio/fake_audio_log_factory.h"
#include "media/audio/fake_audio_manager.h"
......@@ -39,13 +37,13 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using ::testing::_;
using ::testing::AtLeast;
using ::testing::Invoke;
using ::testing::Mock;
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::StrictMock;
using ::testing::_;
using media::AudioBus;
using media::AudioManager;
......@@ -332,15 +330,6 @@ class OutputControllerTest : public ::testing::Test {
std::string(), &mock_sync_reader_,
&stream_monitor_coordinator_, processing_id_);
controller_->SetVolume(kTestVolume);
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator creation when
// removing output controller checks.
if (!base::GlobalHistogramAllocator::Get()) {
const int32_t kAllocatorMemorySize = 8 << 20;
base::GlobalHistogramAllocator::CreateWithLocalMemory(
kAllocatorMemorySize, 0, "HistogramAllocatorTest");
}
#endif
}
void TearDown() override { controller_ = base::nullopt; }
......
......@@ -6,11 +6,9 @@
#include <utility>
#include "base/metrics/persistent_histogram_allocator.h"
#include "base/test/mock_callback.h"
#include "base/test/scoped_task_environment.h"
#include "base/unguessable_token.h"
#include "build/build_config.h"
#include "media/audio/audio_io.h"
#include "media/audio/mock_audio_manager.h"
#include "media/audio/test_audio_thread.h"
......@@ -118,15 +116,6 @@ class TestEnvironment {
stream_factory_binding_(&stream_factory_,
mojo::MakeRequest(&stream_factory_ptr_)) {
mojo::core::SetDefaultProcessErrorCallback(bad_message_callback_.Get());
#if defined(OS_WIN)
// TODO(https://crbug.com/867827) remove histogram allocator creation when
// removing output controller checks.
if (!base::GlobalHistogramAllocator::Get()) {
const int32_t kAllocatorMemorySize = 8 << 20;
base::GlobalHistogramAllocator::CreateWithLocalMemory(
kAllocatorMemorySize, 0, "HistogramAllocatorTest");
}
#endif
}
~TestEnvironment() {
......
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