Skip to content

Commit

Permalink
Reland "Add enable_ipc_logging build argument"
Browse files Browse the repository at this point in the history
It caused a build flake due to incorrect dependents on //ipc
and it was reverted. Fixes committed in order to fix these:
- https://codereview.chromium.org/2767193005
- https://codereview.chromium.org/2852273002

> Implement a build option to enable the IPC logging system
> in release builds. It's useful to save time and resources when
> debugging IPC communication (e.g. in automated testing
> environments).
>
> It also turns IPC_MESSAGE_LOG_ENABLED macro to a build flag.

BUG=

Review-Url: https://codereview.chromium.org/2847003005
Cr-Commit-Position: refs/heads/master@{#471255}
  • Loading branch information
davidsz authored and Commit bot committed May 12, 2017
1 parent 2e623e7 commit 041528a
Show file tree
Hide file tree
Showing 23 changed files with 68 additions and 64 deletions.
5 changes: 3 additions & 2 deletions chrome/common/logging_chrome.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
// IPC_MESSAGE_LOG_ENABLED. We need to use it to define
// IPC_MESSAGE_MACROS_LOG_ENABLED so render_messages.h will generate the
// ViewMsgLog et al. functions.
#include "ipc/ipc_message.h"
#include "ipc/ipc_features.h"

// On Windows, the about:ipc dialog shows IPCs; on POSIX, we hook up a
// logger in this file. (We implement about:ipc on Mac but implement
// the loggers here anyway). We need to do this real early to be sure
// IPC_MESSAGE_MACROS_LOG_ENABLED doesn't get undefined.
#if defined(OS_POSIX) && defined(IPC_MESSAGE_LOG_ENABLED)
#if defined(OS_POSIX) && BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#define IPC_MESSAGE_MACROS_LOG_ENABLED
#include "content/public/common/content_ipc_logging.h"
#define IPC_LOG_TABLE_ADD_ENTRY(msg_id, logger) \
Expand All @@ -33,6 +33,7 @@
#include <string> // NOLINT

#include "base/base_switches.h"
#include "base/bind.h"
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/debug/debugger.h"
Expand Down
4 changes: 2 additions & 2 deletions chrome/service/service_ipc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ServiceIPCServer::ServiceIPCServer(
}

bool ServiceIPCServer::Init() {
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
IPC::Logging::GetInstance()->SetIPCSender(this);
#endif
CreateChannel();
Expand All @@ -41,7 +41,7 @@ void ServiceIPCServer::CreateChannel() {
}

ServiceIPCServer::~ServiceIPCServer() {
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
IPC::Logging::GetInstance()->SetIPCSender(NULL);
#endif
}
Expand Down
4 changes: 2 additions & 2 deletions content/browser/browser_ipc_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace content {

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

void EnableIPCLoggingForChildProcesses(bool enabled) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
Expand Down Expand Up @@ -46,6 +46,6 @@ void EnableIPCLogging(bool enable) {
i.GetCurrentValue()->Send(new ChildProcessMsg_SetIPCLoggingEnabled(enable));
}

#endif // IPC_MESSAGE_LOG_ENABLED
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

} // namespace content
2 changes: 1 addition & 1 deletion content/browser/renderer_host/render_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,7 @@ void RenderProcessHostImpl::OnChannelConnected(int32_t peer_pid) {
observer.RenderProcessReady(this);
}

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Send(new ChildProcessMsg_SetIPCLoggingEnabled(
IPC::Logging::GetInstance()->Enabled()));
#endif
Expand Down
10 changes: 5 additions & 5 deletions content/child/child_thread_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ void ChildThreadImpl::Init(const Options& options) {
g_lazy_tls.Pointer()->Set(this);
on_channel_error_called_ = false;
message_loop_ = base::MessageLoop::current();
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
// We must make sure to instantiate the IPC Logger *before* we create the
// channel, otherwise we can get a callback on the IO thread which creates
// the logger, and the logger does not like being created on the IO thread.
Expand All @@ -430,7 +430,7 @@ void ChildThreadImpl::Init(const Options& options) {
channel_ =
IPC::SyncChannel::Create(this, ChildProcess::current()->io_task_runner(),
ChildProcess::current()->GetShutDownEvent());
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
if (!IsInBrowserProcess())
IPC::Logging::GetInstance()->SetIPCSender(this);
#endif
Expand Down Expand Up @@ -565,7 +565,7 @@ void ChildThreadImpl::Init(const Options& options) {
}

ChildThreadImpl::~ChildThreadImpl() {
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
IPC::Logging::GetInstance()->SetIPCSender(NULL);
#endif

Expand Down Expand Up @@ -691,7 +691,7 @@ bool ChildThreadImpl::OnMessageReceived(const IPC::Message& msg) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(ChildThreadImpl, msg)
IPC_MESSAGE_HANDLER(ChildProcessMsg_Shutdown, OnShutdown)
#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
IPC_MESSAGE_HANDLER(ChildProcessMsg_SetIPCLoggingEnabled,
OnSetIPCLoggingEnabled)
#endif
Expand Down Expand Up @@ -758,7 +758,7 @@ void ChildThreadImpl::OnShutdown() {
base::MessageLoop::current()->QuitWhenIdle();
}

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
void ChildThreadImpl::OnSetIPCLoggingEnabled(bool enable) {
if (enable)
IPC::Logging::GetInstance()->Enable();
Expand Down
4 changes: 2 additions & 2 deletions content/child/child_thread_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "content/common/content_export.h"
#include "content/public/child/child_thread.h"
#include "ipc/ipc.mojom.h"
#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.
#include "ipc/ipc_features.h" // For BUILDFLAG(IPC_MESSAGE_LOG_ENABLED).
#include "ipc/ipc_platform_file.h"
#include "ipc/message_router.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
Expand Down Expand Up @@ -219,7 +219,7 @@ class CONTENT_EXPORT ChildThreadImpl
void OnSetProfilerStatus(tracked_objects::ThreadData::Status status);
void OnGetChildProfilerData(int sequence_number, int current_profiling_phase);
void OnProfilingPhaseCompleted(int profiling_phase);
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
void OnSetIPCLoggingEnabled(bool enable);
#endif

Expand Down
6 changes: 3 additions & 3 deletions content/common/child_process_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ bool ChildProcessHostImpl::InitChannel() {
delegate_->OnChannelInitialized(channel_.get());

// Make sure these messages get sent first.
#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
bool enabled = IPC::Logging::GetInstance()->Enabled();
Send(new ChildProcessMsg_SetIPCLoggingEnabled(enabled));
#endif
Expand Down Expand Up @@ -213,7 +213,7 @@ uint64_t ChildProcessHostImpl::ChildProcessUniqueIdToTracingProcessId(
}

bool ChildProcessHostImpl::OnMessageReceived(const IPC::Message& msg) {
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
IPC::Logging* logger = IPC::Logging::GetInstance();
if (msg.type() == IPC_LOGGING_ID) {
logger->OnReceivedLoggingMessage(msg);
Expand Down Expand Up @@ -244,7 +244,7 @@ bool ChildProcessHostImpl::OnMessageReceived(const IPC::Message& msg) {
handled = delegate_->OnMessageReceived(msg);
}

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
if (logger->Enabled())
logger->OnPostDispatchMessage(msg);
#endif
Expand Down
3 changes: 2 additions & 1 deletion content/common/child_process_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "gpu/command_buffer/common/sync_token.h"
#include "gpu/ipc/common/gpu_param_traits_macros.h"
#include "ipc/ipc_channel_handle.h"
#include "ipc/ipc_features.h"
#include "ipc/ipc_message_macros.h"
#include "ipc/ipc_platform_file.h"
#include "ui/gfx/gpu_memory_buffer.h"
Expand Down Expand Up @@ -91,7 +92,7 @@ IPC_ENUM_TRAITS_MAX_VALUE(base::ThreadPriority,
// process that it's safe to shutdown.
IPC_MESSAGE_CONTROL0(ChildProcessMsg_Shutdown)

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
// Tell the child process to begin or end IPC message logging.
IPC_MESSAGE_CONTROL1(ChildProcessMsg_SetIPCLoggingEnabled,
bool /* on or off */)
Expand Down
10 changes: 4 additions & 6 deletions content/common/content_ipc_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,17 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

#include <stdint.h>

#if defined(IPC_MESSAGE_LOG_ENABLED)
#include "ipc/ipc_features.h"

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#define IPC_MESSAGE_MACROS_LOG_ENABLED
#include "content/public/common/content_ipc_logging.h"
#define IPC_LOG_TABLE_ADD_ENTRY(msg_id, logger) \
content::RegisterIPCLogger(msg_id, logger)
#include "content/common/all_messages.h"
#endif

#if defined(IPC_MESSAGE_LOG_ENABLED)

#include "base/containers/hash_tables.h"
#include "base/lazy_instance.h"
Expand All @@ -36,4 +34,4 @@ void RegisterIPCLogger(uint32_t msg_id, LogFunction logger) {

} // content

#endif
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
4 changes: 2 additions & 2 deletions content/public/browser/browser_ipc_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
#define CONTENT_PUBLIC_BROWSER_BROWSER_IPC_LOGGING_H_

#include "content/common/content_export.h"
#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.
#include "ipc/ipc_features.h"

namespace content {

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

// Enable or disable IPC logging for the browser, all processes
// derived from ChildProcess (plugin etc), and all
Expand Down
4 changes: 1 addition & 3 deletions content/public/common/content_ipc_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@
#ifndef CONTENT_PUBLIC_COMMON_CONTENT_IPC_LOGGING_H_
#define CONTENT_PUBLIC_COMMON_CONTENT_IPC_LOGGING_H_

#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

#include <stdint.h>

#include "content/common/content_export.h"
#include "ipc/ipc_logging.h"

namespace content {

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

// Register a logger for the given IPC message. Use
//
Expand Down
6 changes: 3 additions & 3 deletions content/shell/app/shell_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/trace_event/trace_log.h"
#include "build/build_config.h"
#include "cc/base/switches.h"
#include "content/common/content_constants_internal.h"
Expand All @@ -34,6 +35,7 @@
#include "content/shell/renderer/shell_content_renderer_client.h"
#include "content/shell/utility/shell_content_utility_client.h"
#include "gpu/config/gpu_switches.h"
#include "ipc/ipc_features.h"
#include "media/base/media_switches.h"
#include "media/base/mime_util.h"
#include "net/cookies/cookie_monster.h"
Expand All @@ -45,9 +47,7 @@
#include "ui/gl/gl_implementation.h"
#include "ui/gl/gl_switches.h"

#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

#if defined(IPC_MESSAGE_LOG_ENABLED)
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#define IPC_MESSAGE_MACROS_LOG_ENABLED
#include "content/public/common/content_ipc_logging.h"
#define IPC_LOG_TABLE_ADD_ENTRY(msg_id, logger) \
Expand Down
13 changes: 13 additions & 0 deletions ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,23 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/buildflag_header.gni")
import("//build/config/nacl/config.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//testing/test.gni")
import("//tools/ipc_fuzzer/ipc_fuzzer.gni")

declare_args() {
# Enabling debug builds automatically sets enable_ipc_logging to true.
enable_ipc_logging = is_debug
}

buildflag_header("ipc_features") {
header = "ipc_features.h"

flags = [ "IPC_MESSAGE_LOG_ENABLED=$enable_ipc_logging" ]
}

component("ipc") {
sources = [
"export_template.h",
Expand Down Expand Up @@ -96,6 +108,7 @@ component("ipc") {
defines = [ "IPC_IMPLEMENTATION" ]

public_deps = [
":ipc_features",
":mojom",
":param_traits",
"//mojo/public/cpp/bindings",
Expand Down
4 changes: 2 additions & 2 deletions ipc/ipc_channel_nacl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,9 @@ bool ChannelNacl::Send(Message* message) {
<< " with type " << message->type();
std::unique_ptr<Message> message_ptr(message);

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Logging::GetInstance()->OnSendMessage(message_ptr.get());
#endif // IPC_MESSAGE_LOG_ENABLED
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

TRACE_EVENT_WITH_FLOW0(TRACE_DISABLED_BY_DEFAULT("ipc.flow"),
"ChannelNacl::Send",
Expand Down
10 changes: 5 additions & 5 deletions ipc/ipc_channel_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void ChannelProxy::Context::CreateChannel(

bool ChannelProxy::Context::TryFilters(const Message& message) {
DCHECK(message_filter_router_);
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Logging* logger = Logging::GetInstance();
if (logger->Enabled())
logger->OnPreDispatchMessage(message);
Expand All @@ -89,7 +89,7 @@ bool ChannelProxy::Context::TryFilters(const Message& message) {
listener_task_runner_->PostTask(
FROM_HERE, base::Bind(&Context::OnDispatchBadMessage, this, message));
}
#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
if (logger->Enabled())
logger->OnPostDispatchMessage(message);
#endif
Expand Down Expand Up @@ -315,7 +315,7 @@ void ChannelProxy::Context::OnDispatchMessage(const Message& message) {

OnDispatchConnected();

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Logging* logger = Logging::GetInstance();
if (message.type() == IPC_LOGGING_ID) {
logger->OnReceivedLoggingMessage(message);
Expand All @@ -330,7 +330,7 @@ void ChannelProxy::Context::OnDispatchMessage(const Message& message) {
if (message.dispatch_error())
listener_->OnBadMessageReceived(message);

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
if (logger->Enabled())
logger->OnPostDispatchMessage(message);
#endif
Expand Down Expand Up @@ -530,7 +530,7 @@ bool ChannelProxy::Send(Message* message) {
message = outgoing_message_filter()->Rewrite(message);
#endif

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Logging::GetInstance()->OnSendMessage(message);
#endif

Expand Down
4 changes: 2 additions & 2 deletions ipc/ipc_channel_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
namespace IPC {
namespace internal {

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

namespace {
std::string GetMessageText(const Message& message) {
Expand All @@ -42,7 +42,7 @@ std::string GetMessageText(const Message& message) {
(message).flags(), TRACE_EVENT_FLAG_FLOW_IN, "class", \
IPC_MESSAGE_ID_CLASS((message).type()), "line", \
IPC_MESSAGE_ID_LINE((message).type()));
#endif // IPC_MESSAGE_LOG_ENABLED
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

ChannelReader::ChannelReader(Listener* listener)
: listener_(listener),
Expand Down
6 changes: 3 additions & 3 deletions ipc/ipc_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "ipc/ipc_logging.h"

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#define IPC_MESSAGE_MACROS_LOG_ENABLED
#endif

Expand All @@ -31,7 +31,7 @@
#include <unistd.h>
#endif

#ifdef IPC_MESSAGE_LOG_ENABLED
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)

using base::Time;

Expand Down Expand Up @@ -310,4 +310,4 @@ void GenerateLogData(const Message& message, LogData* data, bool get_params) {

}

#endif // IPC_MESSAGE_LOG_ENABLED
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
Loading

0 comments on commit 041528a

Please sign in to comment.