Skip to content

Commit

Permalink
ipc: Add UMA metrics for all possible errors during attachment broker…
Browse files Browse the repository at this point in the history
…ing.

This fixes all the TODOs I left for myself in ipc/. This is also the last task
left for the Mac implementation of attachment brokering.

BUG=535711

Review URL: https://codereview.chromium.org/1428483003

Cr-Commit-Position: refs/heads/master@{#356246}
  • Loading branch information
erikchen authored and Commit bot committed Oct 27, 2015
1 parent 463c2ca commit f065078
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 9 deletions.
16 changes: 16 additions & 0 deletions ipc/attachment_broker_privileged.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ class IPC_EXPORT AttachmentBrokerPrivileged : public IPC::AttachmentBroker {
DESTINATION_NOT_FOUND = 1,
// The brokerable attachment did not have a destination process.
NO_DESTINATION = 2,
// Error making an intermediate Mach port.
ERROR_MAKE_INTERMEDIATE = 3,
// Error parsing DuplicateMachPort message.
ERROR_PARSE_DUPLICATE_MACH_PORT_MESSAGE = 4,
// Couldn't get a task port for the process with a given pid.
ERROR_TASK_FOR_PID = 5,
// Couldn't make a port with receive rights in the destination process.
ERROR_MAKE_RECEIVE_PORT = 6,
// Couldn't change the attributes of a Mach port.
ERROR_SET_ATTRIBUTES = 7,
// Couldn't extract a right.
ERROR_EXTRACT_RIGHT = 8,
// Couldn't send a Mach port in a call to mach_msg().
ERROR_SEND_MACH_PORT = 9,
// Couldn't decrease the ref count on a Mach port.
ERROR_DECREASE_REF = 10,
ERROR_MAX
};

Expand Down
16 changes: 8 additions & 8 deletions ipc/attachment_broker_privileged_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ bool AttachmentBrokerPrivilegedMac::SendAttachmentToProcess(
base::mac::ScopedMachSendRight(wire_format.mach_port));
mach_port_attachment->reset_mach_port_ownership();
if (intermediate_port == MACH_PORT_NULL) {
// TODO(erikchen): UMA metric.
LogError(ERROR_MAKE_INTERMEDIATE);
return false;
}

Expand Down Expand Up @@ -110,7 +110,7 @@ void AttachmentBrokerPrivilegedMac::OnDuplicateMachPort(
const IPC::Message& message) {
AttachmentBrokerMsg_DuplicateMachPort::Param param;
if (!AttachmentBrokerMsg_DuplicateMachPort::Read(&message, &param)) {
// TODO(erikchen): UMA metric.
LogError(ERROR_PARSE_DUPLICATE_MACH_PORT_MESSAGE);
return;
}
IPC::internal::MachPortAttachmentMac::WireFormat wire_format =
Expand Down Expand Up @@ -178,7 +178,7 @@ mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort(
DCHECK_NE(pid, base::GetCurrentProcId());
mach_port_t task_port = port_provider_->TaskForPid(pid);
if (task_port == MACH_PORT_NULL) {
// TODO(erikchen): UMA metric.
LogError(ERROR_TASK_FOR_PID);
return MACH_PORT_NULL;
}
return CreateIntermediateMachPort(
Expand All @@ -196,7 +196,7 @@ mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort(
kern_return_t kr =
mach_port_allocate(task_port, MACH_PORT_RIGHT_RECEIVE, &endpoint);
if (kr != KERN_SUCCESS) {
// TODO(erikchen): UMA metric.
LogError(ERROR_MAKE_RECEIVE_PORT);
return MACH_PORT_NULL;
}

Expand All @@ -207,7 +207,7 @@ mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort(
reinterpret_cast<mach_port_info_t>(&limits),
MACH_PORT_LIMITS_INFO_COUNT);
if (kr != KERN_SUCCESS) {
// TODO(erikchen): UMA metric.
LogError(ERROR_SET_ATTRIBUTES);
mach_port_deallocate(task_port, endpoint);
return MACH_PORT_NULL;
}
Expand All @@ -219,7 +219,7 @@ mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort(
mach_port_extract_right(task_port, endpoint, MACH_MSG_TYPE_MAKE_SEND_ONCE,
&send_once_right, &send_right_type);
if (kr != KERN_SUCCESS) {
// TODO(erikchen): UMA metric.
LogError(ERROR_EXTRACT_RIGHT);
mach_port_deallocate(task_port, endpoint);
return MACH_PORT_NULL;
}
Expand All @@ -230,7 +230,7 @@ mach_port_name_t AttachmentBrokerPrivilegedMac::CreateIntermediateMachPort(
kr = SendMachPort(
send_once_right, port_to_insert.get(), MACH_MSG_TYPE_COPY_SEND);
if (kr != KERN_SUCCESS) {
// TODO(erikchen): UMA metric.
LogError(ERROR_SEND_MACH_PORT);
mach_port_deallocate(task_port, endpoint);
return MACH_PORT_NULL;
}
Expand Down Expand Up @@ -272,7 +272,7 @@ base::mac::ScopedMachSendRight AttachmentBrokerPrivilegedMac::ExtractNamedRight(
// Decrement the reference count of the send right from the source process.
kr = mach_port_mod_refs(task_port, named_right, MACH_PORT_RIGHT_SEND, -1);
if (kr != KERN_SUCCESS) {
// TODO(erikchen): UMA metric.
LogError(ERROR_DECREASE_REF);
// Failure does not actually affect attachment brokering, so there's no need
// to return |MACH_PORT_NULL|.
}
Expand Down
2 changes: 1 addition & 1 deletion ipc/attachment_broker_unprivileged_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void AttachmentBrokerUnprivilegedMac::OnMachPortHasBeenDuplicated(
const IPC::internal::MachPortAttachmentMac::WireFormat& wire_format) {
// The IPC message was intended for a different process. Ignore it.
if (wire_format.destination_process != base::Process::Current().Pid()) {
// TODO(erikchen): UMA metric.
LogError(WRONG_DESTINATION);
return;
}

Expand Down
22 changes: 22 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63664,6 +63664,28 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
<int value="2" label="NO_DESTINATION">
The brokerable attachment did not have a destination process.
</int>
<int value="3" label="ERROR_MAKE_INTERMEDIATE">
Error making an intermediate Mach port.
</int>
<int value="4" label="ERROR_PARSE_DUPLICATE_MACH_PORT_MESSAGE">
Error parsing DuplicateMachPort message.
</int>
<int value="5" label="ERROR_TASK_FOR_PID">
Couldn't get a task port for the process with a given pid.
</int>
<int value="6" label="ERROR_MAKE_RECEIVE_PORT">
Couldn't make a port with receive rights in the destination process.
</int>
<int value="7" label="ERROR_SET_ATTRIBUTES">
Couldn't change the attributes of a Mach port.
</int>
<int value="8" label="ERROR_EXTRACT_RIGHT">Couldn't extract a right.</int>
<int value="9" label="ERROR_SEND_MACH_PORT">
Couldn't send a Mach port in a call to mach_msg().
</int>
<int value="10" label="ERROR_DECREASE_REF">
Couldn't decrease the ref count on a Mach port.
</int>
</enum>

<enum name="IPCAttachmentBrokerUnprivilegedBrokerAttachmentError" type="int">
Expand Down

0 comments on commit f065078

Please sign in to comment.