Skip to content

Commit

Permalink
Revert of Reland "Add enable_ipc_logging build argument" (patchset ch…
Browse files Browse the repository at this point in the history
…romium#1 id:1 of https://codereview.chromium.org/2777983005/ )

Reason for revert:
This still appears to be causing a build flake on https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng

log: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux%2Flinux_chromium_rel_ng%2F418400%2F%2B%2Frecipes%2Fsteps%2Fcompile__without_patch_%2F0%2Fstdout

Original issue's description:
> Reland "Add enable_ipc_logging build argument (patchset chromium#2 id:20001 of https://codereview.chromium.org/2770653002/ )"
>
> It introduced a build flake due to assorted missing dependencies
> on //ipc and it was reverted in https://codereview.chromium.org/2768403002.
> The public dependencies on //ipc are fixed in https://codereview.chromium.org/2767193005.
>
> Original issue's description:
> > Add enable_ipc_logging build argument
> >
> > 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/2770653002
> > Cr-Commit-Position: refs/heads/master@{#459094}
> > Committed: https://chromium.googlesource.com/chromium/src/+/76890ec11e7402591d1c9add533d4e61c30f8e08
>
> BUG=
>
> Review-Url: https://codereview.chromium.org/2777983005
> Cr-Commit-Position: refs/heads/master@{#460152}
> Committed: https://chromium.googlesource.com/chromium/src/+/b39d83360c64113ff17e64169c49e1a83329b61e

TBR=rockot@chromium.org,jam@chromium.org,kenrb@chromium.org,davidsz@inf.u-szeged.hu
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review-Url: https://codereview.chromium.org/2776393003
Cr-Commit-Position: refs/heads/master@{#460218}
  • Loading branch information
jwd authored and Commit bot committed Mar 28, 2017
1 parent 13390fe commit c102892
Show file tree
Hide file tree
Showing 24 changed files with 64 additions and 68 deletions.
1 change: 0 additions & 1 deletion AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,6 @@ Suvanjan Mukherjee <suvanjanmukherjee@gmail.com>
Swati Jaiswal <swa.jaiswal@samsung.com>
Sylvain Zimmer <sylvinus@gmail.com>
Sylvestre Ledru <sylvestre.ledru@gmail.com>
Szabolcs David <davidsz@inf.u-szeged.hu>
Szymon Piechowicz <szymonpiechowicz@o2.pl>
Taehoon Lee <taylor.hoon@gmail.com>
Takeshi Kurosawa <taken.spc@gmail.com>
Expand Down
4 changes: 2 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_features.h"
#include "ipc/ipc_message.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) && BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(OS_POSIX) && defined(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
4 changes: 2 additions & 2 deletions chrome/service/service_ipc_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ ServiceIPCServer::ServiceIPCServer(
}

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

ServiceIPCServer::~ServiceIPCServer() {
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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 // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#endif // 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 @@ -2104,7 +2104,7 @@ void RenderProcessHostImpl::OnChannelConnected(int32_t peer_pid) {
observer.RenderProcessReady(this);
}

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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 @@ -414,7 +414,7 @@ void ChildThreadImpl::Init(const Options& options) {
g_lazy_tls.Pointer()->Set(this);
on_channel_error_called_ = false;
message_loop_ = base::MessageLoop::current();
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 @@ -424,7 +424,7 @@ void ChildThreadImpl::Init(const Options& options) {
channel_ =
IPC::SyncChannel::Create(this, ChildProcess::current()->io_task_runner(),
ChildProcess::current()->GetShutDownEvent());
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef IPC_MESSAGE_LOG_ENABLED
if (!IsInBrowserProcess())
IPC::Logging::GetInstance()->SetIPCSender(this);
#endif
Expand Down Expand Up @@ -570,7 +570,7 @@ void ChildThreadImpl::Init(const Options& options) {
}

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

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

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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_features.h" // For BUILDFLAG(IPC_MESSAGE_LOG_ENABLED).
#include "ipc/ipc_message.h" // For 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 @@ -228,7 +228,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);
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 @@ -158,7 +158,7 @@ bool ChildProcessHostImpl::InitChannel() {
delegate_->OnChannelInitialized(channel_.get());

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

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

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef IPC_MESSAGE_LOG_ENABLED
if (logger->Enabled())
logger->OnPostDispatchMessage(msg);
#endif
Expand Down
3 changes: 1 addition & 2 deletions content/common/child_process_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#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 @@ -92,7 +91,7 @@ IPC_ENUM_TRAITS_MAX_VALUE(base::ThreadPriority,
// process that it's safe to shutdown.
IPC_MESSAGE_CONTROL0(ChildProcessMsg_Shutdown)

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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: 6 additions & 4 deletions content/common/content_ipc_logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <stdint.h>
#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

#include "ipc/ipc_features.h"
#include <stdint.h>

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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 @@ -34,4 +36,4 @@ void RegisterIPCLogger(uint32_t msg_id, LogFunction logger) {

} // content

#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#endif
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_features.h"
#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

namespace content {

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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: 3 additions & 1 deletion content/public/common/content_ipc_logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
#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 BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#if defined(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,7 +12,6 @@
#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 @@ -35,7 +34,6 @@
#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 @@ -47,7 +45,9 @@
#include "ui/gl/gl_implementation.h"
#include "ui/gl/gl_switches.h"

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#include "ipc/ipc_message.h" // For IPC_MESSAGE_LOG_ENABLED.

#if defined(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: 0 additions & 13 deletions ipc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,11 @@
# 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 @@ -108,7 +96,6 @@ 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);

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef IPC_MESSAGE_LOG_ENABLED
Logging::GetInstance()->OnSendMessage(message_ptr.get());
#endif // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#endif // 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_);
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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));
}
#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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();

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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);

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 {

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#endif // 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"

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

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

#if BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#ifdef 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 // BUILDFLAG(IPC_MESSAGE_LOG_ENABLED)
#endif // IPC_MESSAGE_LOG_ENABLED
Loading

0 comments on commit c102892

Please sign in to comment.