Skip to content

Commit

Permalink
Deletes base::MessageLoop::set_thread_name().
Browse files Browse the repository at this point in the history
Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.

Review-Url: https://codereview.chromium.org/1942053002
Cr-Commit-Position: refs/heads/master@{#399694}
  • Loading branch information
alokp-chromium authored and Commit bot committed Jun 14, 2016
1 parent 2e75c70 commit d52f9cb
Show file tree
Hide file tree
Showing 16 changed files with 85 additions and 68 deletions.
29 changes: 23 additions & 6 deletions base/message_loop/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/metrics/statistics_recorder.h"
#include "base/run_loop.h"
#include "base/third_party/dynamic_annotations/dynamic_annotations.h"
#include "base/threading/thread_id_name_manager.h"
#include "base/threading/thread_local.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -395,7 +396,8 @@ MessageLoop::MessageLoop(Type type, MessagePumpFactoryCallback pump_factory)
incoming_task_queue_(new internal::IncomingTaskQueue(this)),
unbound_task_runner_(
new internal::MessageLoopTaskRunner(incoming_task_queue_)),
task_runner_(unbound_task_runner_) {
task_runner_(unbound_task_runner_),
thread_id_(kInvalidThreadId) {
// If type is TYPE_CUSTOM non-null pump_factory must be given.
DCHECK(type_ != TYPE_CUSTOM || !pump_factory_.is_null());
}
Expand All @@ -414,6 +416,22 @@ void MessageLoop::BindToCurrentThread() {
unbound_task_runner_->BindToCurrentThread();
unbound_task_runner_ = nullptr;
SetThreadTaskRunnerHandle();
{
// Save the current thread's ID for potential use by other threads
// later from GetThreadName().
thread_id_ = PlatformThread::CurrentId();
subtle::MemoryBarrier();
}
}

std::string MessageLoop::GetThreadName() const {
if (thread_id_ == kInvalidThreadId) {
// |thread_id_| may already have been initialized but this thread might not
// have received the update yet.
subtle::MemoryBarrier();
DCHECK_NE(kInvalidThreadId, thread_id_);
}
return ThreadIdNameManager::GetInstance()->GetName(thread_id_);
}

void MessageLoop::SetTaskRunner(
Expand Down Expand Up @@ -554,13 +572,12 @@ void MessageLoop::StartHistogrammer() {
#if !defined(OS_NACL) // NaCl build has no metrics code.
if (enable_histogrammer_ && !message_histogram_
&& StatisticsRecorder::IsActive()) {
DCHECK(!thread_name_.empty());
std::string thread_name = GetThreadName();
DCHECK(!thread_name.empty());
message_histogram_ = LinearHistogram::FactoryGetWithRangeDescription(
"MsgLoop:" + thread_name_,
kLeastNonZeroMessageId, kMaxMessageId,
"MsgLoop:" + thread_name, kLeastNonZeroMessageId, kMaxMessageId,
kNumberOfDistinctMessagesDisplayed,
HistogramBase::kHexRangePrintingFlag,
event_descriptions_);
HistogramBase::kHexRangePrintingFlag, event_descriptions_);
}
#endif
}
Expand Down
14 changes: 7 additions & 7 deletions base/message_loop/message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,10 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
// Returns the type passed to the constructor.
Type type() const { return type_; }

// Optional call to connect the thread name with this loop.
void set_thread_name(const std::string& thread_name) {
DCHECK(thread_name_.empty()) << "Should not rename this thread!";
thread_name_ = thread_name;
}
const std::string& thread_name() const { return thread_name_; }
// Returns the name of the thread this message loop is bound to.
// This function is only valid when this message loop is running and
// BindToCurrentThread has already been called.
std::string GetThreadName() const;

// Gets the TaskRunner associated with this message loop.
const scoped_refptr<SingleThreadTaskRunner>& task_runner() {
Expand Down Expand Up @@ -517,7 +515,6 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
// if type_ is TYPE_CUSTOM and pump_ is null.
MessagePumpFactoryCallback pump_factory_;

std::string thread_name_;
// A profiling histogram showing the counts of various messages and events.
HistogramBase* message_histogram_;

Expand All @@ -536,6 +533,9 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
scoped_refptr<SingleThreadTaskRunner> task_runner_;
std::unique_ptr<ThreadTaskRunnerHandle> thread_task_runner_handle_;

// Id of the thread this message loop is bound to.
PlatformThreadId thread_id_;

template <class T, class R> friend class base::subtle::DeleteHelperInternal;
template <class T, class R> friend class base::subtle::ReleaseHelperInternal;

Expand Down
16 changes: 16 additions & 0 deletions base/message_loop/message_loop_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -979,4 +979,20 @@ TEST(MessageLoopTest, DeleteUnboundLoop) {
EXPECT_EQ(loop.task_runner(), ThreadTaskRunnerHandle::Get());
}

TEST(MessageLoopTest, ThreadName) {
{
std::string kThreadName("foo");
MessageLoop loop;
PlatformThread::SetName(kThreadName);
EXPECT_EQ(kThreadName, loop.GetThreadName());
}

{
std::string kThreadName("bar");
base::Thread thread(kThreadName);
ASSERT_TRUE(thread.StartAndWaitForTesting());
EXPECT_EQ(kThreadName, thread.message_loop()->GetThreadName());
}
}

} // namespace base
1 change: 0 additions & 1 deletion base/threading/thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ void Thread::ThreadMain() {
DCHECK(message_loop_);
std::unique_ptr<MessageLoop> message_loop(message_loop_);
message_loop_->BindToCurrentThread();
message_loop_->set_thread_name(name_);
message_loop_->SetTimerSlack(message_loop_timer_slack_);

#if defined(OS_WIN)
Expand Down
2 changes: 1 addition & 1 deletion base/trace_event/trace_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ void TraceLog::OnFlushTimeout(int generation, bool discard_events) {
for (hash_set<MessageLoop*>::const_iterator it =
thread_message_loops_.begin();
it != thread_message_loops_.end(); ++it) {
LOG(WARNING) << "Thread: " << (*it)->thread_name();
LOG(WARNING) << "Thread: " << (*it)->GetThreadName();
}
}
FinishFlush(generation, discard_events);
Expand Down
1 change: 0 additions & 1 deletion chrome/app_shim/chrome_main_app_mode_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,6 @@ int ChromeAppModeStart_v4(const app_mode::ChromeAppModeInfo* info) {

AppShimController controller;
base::MessageLoopForUI main_message_loop;
main_message_loop.set_thread_name("MainThread");
base::PlatformThread::SetName("CrAppShimMain");

// In tests, launching Chrome does nothing, and we won't get a ping response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/synchronization/waitable_event.h"
#include "base/test/multiprocess_test.h"
#include "base/test/test_timeouts.h"
#include "base/threading/platform_thread.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -214,8 +215,8 @@ typedef base::Callback<void(MockServiceIPCServer* server)>
// service process. Any non-zero return value will be printed out and can help
// determine the failure.
int CloudPrintMockService_Main(SetExpectationsCallback set_expectations) {
base::PlatformThread::SetName("Main Thread");
base::MessageLoopForUI main_message_loop;
main_message_loop.set_thread_name("Main Thread");
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
content::RegisterPathProvider();

Expand Down
3 changes: 2 additions & 1 deletion chrome/chrome_watcher/chrome_watcher_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "base/strings/string_piece.h"
#include "base/strings/utf_string_conversions.h"
#include "base/synchronization/waitable_event.h"
#include "base/threading/platform_thread.h"
#include "base/threading/thread.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -231,8 +232,8 @@ extern "C" int WatcherMain(const base::char16* registry_path,
#endif // BUILDFLAG(ENABLE_KASKO)

// Run a UI message loop on the main thread.
base::PlatformThread::SetName("WatcherMainThread");
base::MessageLoop msg_loop(base::MessageLoop::TYPE_UI);
msg_loop.set_thread_name("WatcherMainThread");

base::RunLoop run_loop;
BrowserMonitor monitor(registry_path, &run_loop);
Expand Down
3 changes: 2 additions & 1 deletion chrome/common/service_process_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/process/launch.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_split.h"
#include "base/threading/platform_thread.h"
#include "build/build_config.h"

#if !defined(OS_MACOSX)
Expand Down Expand Up @@ -227,8 +228,8 @@ MULTIPROCESS_TEST_MAIN(ServiceProcessStateTestReadyFalse) {
}

MULTIPROCESS_TEST_MAIN(ServiceProcessStateTestShutdown) {
base::PlatformThread::SetName("ServiceProcessStateTestShutdownMainThread");
base::MessageLoop message_loop;
message_loop.set_thread_name("ServiceProcessStateTestShutdownMainThread");
base::Thread io_thread_("ServiceProcessStateTestShutdownIOThread");
base::Thread::Options options(base::MessageLoop::TYPE_IO, 0);
EXPECT_TRUE(io_thread_.StartWithOptions(options));
Expand Down
3 changes: 1 addition & 2 deletions chrome/service/service_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ int ServiceProcessMain(const content::MainFunctionParams& parameters) {
// cookies should go through the browser process.
net::URLRequest::SetDefaultCookiePolicyToBlock();

base::PlatformThread::SetName("CrServiceMain");
base::MessageLoopForUI main_message_loop;
main_message_loop.set_thread_name("MainThread");
if (parameters.command_line.HasSwitch(switches::kWaitForDebugger)) {
base::debug::WaitForDebugger(60, true);
}

VLOG(1) << "Service process launched: "
<< parameters.command_line.GetCommandLineString();

base::PlatformThread::SetName("CrServiceMain");
base::StatisticsRecorder::Initialize();

// If there is already a service process running, quit now.
Expand Down
5 changes: 1 addition & 4 deletions content/browser/browser_main_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1144,10 +1144,7 @@ void BrowserMainLoop::StopStartupTracingTimer() {

void BrowserMainLoop::InitializeMainThread() {
TRACE_EVENT0("startup", "BrowserMainLoop::InitializeMainThread");
static const char kThreadName[] = "CrBrowserMain";
base::PlatformThread::SetName(kThreadName);
if (main_message_loop_)
main_message_loop_->set_thread_name(kThreadName);
base::PlatformThread::SetName("CrBrowserMain");

// Register the main thread by instantiating it, but don't call any methods.
main_thread_.reset(
Expand Down
34 changes: 15 additions & 19 deletions content/browser/browser_thread_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/macros.h"
#include "base/profiler/scoped_tracker.h"
#include "base/single_thread_task_runner.h"
#include "base/threading/platform_thread.h"
#include "base/threading/sequenced_worker_pool.h"
#include "build/build_config.h"
#include "content/public/browser/browser_thread_delegate.h"
Expand All @@ -40,6 +41,14 @@ static const char* const g_browser_thread_names[BrowserThread::ID_COUNT] = {
"Chrome_IOThread", // IO
};

static const char* GetThreadName(BrowserThread::ID thread) {
if (BrowserThread::UI < thread && thread < BrowserThread::ID_COUNT)
return g_browser_thread_names[thread];
if (thread == BrowserThread::UI)
return "Chrome_UIThread";
return "Unknown Thread";
}

// An implementation of SingleThreadTaskRunner to be used in conjunction
// with BrowserThread.
class BrowserThreadTaskRunner : public base::SingleThreadTaskRunner {
Expand Down Expand Up @@ -120,14 +129,13 @@ base::LazyInstance<BrowserThreadGlobals>::Leaky
} // namespace

BrowserThreadImpl::BrowserThreadImpl(ID identifier)
: Thread(g_browser_thread_names[identifier]),
identifier_(identifier) {
: Thread(GetThreadName(identifier)), identifier_(identifier) {
Initialize();
}

BrowserThreadImpl::BrowserThreadImpl(ID identifier,
base::MessageLoop* message_loop)
: Thread(message_loop->thread_name()), identifier_(identifier) {
: Thread(GetThreadName(identifier)), identifier_(identifier) {
set_message_loop(message_loop);
Initialize();
}
Expand Down Expand Up @@ -417,24 +425,12 @@ bool BrowserThread::CurrentlyOn(ID identifier) {
base::MessageLoop::current();
}

static const char* GetThreadName(BrowserThread::ID thread) {
if (BrowserThread::UI < thread && thread < BrowserThread::ID_COUNT)
return g_browser_thread_names[thread];
if (thread == BrowserThread::UI)
return "Chrome_UIThread";
return "Unknown Thread";
}

// static
std::string BrowserThread::GetDCheckCurrentlyOnErrorMessage(ID expected) {
const base::MessageLoop* message_loop = base::MessageLoop::current();
ID actual_browser_thread;
const char* actual_name = "Unknown Thread";
if (message_loop && !message_loop->thread_name().empty()) {
actual_name = message_loop->thread_name().c_str();
} else if (GetCurrentThreadIdentifier(&actual_browser_thread)) {
actual_name = GetThreadName(actual_browser_thread);
}
std::string actual_name = base::PlatformThread::GetName();
if (actual_name.empty())
actual_name = "Unknown Thread";

std::string result = "Must be called on ";
result += GetThreadName(expected);
result += "; actually called on ";
Expand Down
6 changes: 1 addition & 5 deletions ios/web/app/web_main_loop.mm
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,7 @@
}

void WebMainLoop::InitializeMainThread() {
const char* kThreadName = "CrWebMain";
base::PlatformThread::SetName(kThreadName);
if (main_message_loop_) {
main_message_loop_->set_thread_name(kThreadName);
}
base::PlatformThread::SetName("CrWebMain");

// Register the main thread by instantiating it, but don't call any methods.
main_thread_.reset(
Expand Down
28 changes: 12 additions & 16 deletions ios/web/web_thread_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ const char* const g_web_thread_names[WebThread::ID_COUNT] = {
"Web_IOThread", // IO
};

static const char* GetThreadName(WebThread::ID thread) {
if (WebThread::UI <= thread && thread < WebThread::ID_COUNT)
return g_web_thread_names[thread];
return "Unknown Thread";
}

// An implementation of SingleThreadTaskRunner to be used in conjunction
// with WebThread.
class WebThreadTaskRunner : public base::SingleThreadTaskRunner {
Expand Down Expand Up @@ -110,12 +116,12 @@ base::LazyInstance<WebThreadGlobals>::Leaky g_globals =
} // namespace

WebThreadImpl::WebThreadImpl(ID identifier)
: Thread(g_web_thread_names[identifier]), identifier_(identifier) {
: Thread(GetThreadName(identifier)), identifier_(identifier) {
Initialize();
}

WebThreadImpl::WebThreadImpl(ID identifier, base::MessageLoop* message_loop)
: Thread(message_loop->thread_name()), identifier_(identifier) {
: Thread(GetThreadName(identifier)), identifier_(identifier) {
set_message_loop(message_loop);
Initialize();
}
Expand Down Expand Up @@ -371,22 +377,12 @@ bool WebThread::CurrentlyOn(ID identifier) {
base::MessageLoop::current();
}

static const char* GetThreadName(WebThread::ID thread) {
if (WebThread::UI <= thread && thread < WebThread::ID_COUNT)
return g_web_thread_names[thread];
return "Unknown Thread";
}

// static
std::string WebThread::GetDCheckCurrentlyOnErrorMessage(ID expected) {
const base::MessageLoop* message_loop = base::MessageLoop::current();
ID actual_web_thread;
const char* actual_name = "Unknown Thread";
if (message_loop && !message_loop->thread_name().empty()) {
actual_name = message_loop->thread_name().c_str();
} else if (GetCurrentThreadIdentifier(&actual_web_thread)) {
actual_name = GetThreadName(actual_web_thread);
}
std::string actual_name = base::PlatformThread::GetName();
if (actual_name.empty())
actual_name = "Unknown Thread";

std::string result = "Must be called on ";
result += GetThreadName(expected);
result += "; actually called on ";
Expand Down
1 change: 0 additions & 1 deletion remoting/base/auto_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ void AutoThread::ThreadMain() {
// Complete the initialization of our AutoThread object.
base::PlatformThread::SetName(name_);
ANNOTATE_THREAD_NAME(name_.c_str()); // Tell the name to race detector.
message_loop.set_thread_name(name_);

// Return an AutoThreadTaskRunner that will cleanly quit this thread when
// no more references to it remain.
Expand Down
4 changes: 2 additions & 2 deletions sync/engine/sync_scheduler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#include "base/compiler_specific.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/threading/platform_thread.h"
#include "base/threading/thread_task_runner_handle.h"
#include "sync/engine/backoff_delay_provider.h"
#include "sync/engine/syncer.h"
Expand Down Expand Up @@ -228,7 +228,7 @@ void SyncSchedulerImpl::OnServerConnectionErrorFixed() {

void SyncSchedulerImpl::Start(Mode mode, base::Time last_poll_time) {
DCHECK(CalledOnValidThread());
std::string thread_name = base::MessageLoop::current()->thread_name();
std::string thread_name = base::PlatformThread::GetName();
if (thread_name.empty())
thread_name = "<Main thread>";
SDVLOG(2) << "Start called from thread "
Expand Down

0 comments on commit d52f9cb

Please sign in to comment.