Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1673)

Unified Diff: content/browser/mach_broker_mac.mm

Issue 274193007: Back out r269483 and the line of fixes that followed it. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/mac/install_from_dmg.mm ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/mach_broker_mac.mm
diff --git a/content/browser/mach_broker_mac.mm b/content/browser/mach_broker_mac.mm
index 1e790c9768e8644ae3c6c04ad9e08113ad624f84..24a0e2498134e941fab023ab22c307b156454beb 100644
--- a/content/browser/mach_broker_mac.mm
+++ b/content/browser/mach_broker_mac.mm
@@ -12,7 +12,6 @@
#include "base/command_line.h"
#include "base/logging.h"
#include "base/mac/foundation_util.h"
-#include "base/mac/mach_logging.h"
#include "base/mac/scoped_mach_port.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
@@ -29,6 +28,11 @@ namespace content {
namespace {
+// Prints a string representation of a Mach error code.
+std::string MachErrorCode(kern_return_t err) {
+ return base::StringPrintf("0x%x %s", err, mach_error_string(err));
+}
+
// Mach message structure used in the child as a sending message.
struct MachBroker_ChildSendMsg {
mach_msg_header_t header;
@@ -60,7 +64,8 @@ class MachListenerThreadDelegate : public base::PlatformThread::Delegate {
MACH_PORT_RIGHT_RECEIVE,
&port);
if (kr != KERN_SUCCESS) {
- MACH_LOG(ERROR, kr) << "mach_port_allocate";
+ LOG(ERROR) << "Failed to allocate MachBroker server port: "
+ << MachErrorCode(kr);
return false;
}
@@ -68,7 +73,8 @@ class MachListenerThreadDelegate : public base::PlatformThread::Delegate {
kr = mach_port_insert_right(
mach_task_self(), port, port, MACH_MSG_TYPE_MAKE_SEND);
if (kr != KERN_SUCCESS) {
- MACH_LOG(ERROR, kr) << "mach_port_insert_right";
+ LOG(ERROR) << "Failed to insert send right for MachBroker server port: "
+ << MachErrorCode(kr);
return false;
}
@@ -90,35 +96,33 @@ class MachListenerThreadDelegate : public base::PlatformThread::Delegate {
msg.header.msgh_size = sizeof(msg);
msg.header.msgh_local_port = server_port_.get();
- const mach_msg_option_t options = MACH_RCV_MSG |
- MACH_RCV_TRAILER_TYPE(MACH_RCV_TRAILER_AUDIT) |
- MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT);
-
kern_return_t kr;
- while ((kr = mach_msg(&msg.header,
- options,
- 0,
- sizeof(msg),
- server_port_,
- MACH_MSG_TIMEOUT_NONE,
- MACH_PORT_NULL)) == KERN_SUCCESS) {
+ do {
// Use the kernel audit information to make sure this message is from
// a task that this process spawned. The kernel audit token contains the
// unspoofable pid of the task that sent the message.
- //
- // TODO(rsesek): In the 10.7 SDK, there's audit_token_to_pid().
- pid_t child_pid;
- audit_token_to_au32(msg.trailer.msgh_audit,
- NULL, NULL, NULL, NULL, NULL, &child_pid, NULL, NULL);
-
- mach_port_t child_task_port = msg.child_task_port.name;
-
- // Take the lock and update the broker information.
- base::AutoLock lock(broker_->GetLock());
- broker_->FinalizePid(child_pid, child_task_port);
- }
-
- MACH_LOG(ERROR, kr) << "mach_msg";
+ mach_msg_option_t options = MACH_RCV_MSG |
+ MACH_RCV_TRAILER_TYPE(MACH_RCV_TRAILER_AUDIT) |
+ MACH_RCV_TRAILER_ELEMENTS(MACH_RCV_TRAILER_AUDIT);
+
+ kr = mach_msg(&msg.header, options, 0, sizeof(msg), server_port_,
+ MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+ if (kr == KERN_SUCCESS) {
+ // TODO(rsesek): In the 10.7 SDK, there's audit_token_to_pid().
+ pid_t child_pid;
+ audit_token_to_au32(msg.trailer.msgh_audit,
+ NULL, NULL, NULL, NULL, NULL, &child_pid, NULL, NULL);
+
+ mach_port_t child_task_port = msg.child_task_port.name;
+
+ // Take the lock and update the broker information.
+ base::AutoLock lock(broker_->GetLock());
+ broker_->FinalizePid(child_pid, child_task_port);
+ }
+ } while (kr == KERN_SUCCESS);
+
+ LOG(ERROR) << "MachBroker thread exiting; mach_msg() likely failed: "
+ << MachErrorCode(kr);
}
private:
@@ -137,7 +141,7 @@ bool MachBroker::ChildSendTaskPortToParent() {
mach_port_t bootstrap_port;
kern_return_t kr = task_get_bootstrap_port(mach_task_self(), &bootstrap_port);
if (kr != KERN_SUCCESS) {
- MACH_LOG(ERROR, kr) << "task_get_bootstrap_port";
+ LOG(ERROR) << "Failed to look up bootstrap port: " << MachErrorCode(kr);
return false;
}
@@ -145,7 +149,7 @@ bool MachBroker::ChildSendTaskPortToParent() {
kr = bootstrap_look_up(bootstrap_port,
const_cast<char*>(GetMachPortName().c_str()), &parent_port);
if (kr != KERN_SUCCESS) {
- BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_look_up";
+ LOG(ERROR) << "Failed to look up named parent port: " << MachErrorCode(kr);
return false;
}
@@ -165,7 +169,7 @@ bool MachBroker::ChildSendTaskPortToParent() {
kr = mach_msg(&msg.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, sizeof(msg),
0, MACH_PORT_NULL, 100 /*milliseconds*/, MACH_PORT_NULL);
if (kr != KERN_SUCCESS) {
- MACH_LOG(ERROR, kr) << "mach_msg";
+ LOG(ERROR) << "Failed to send task port to parent: " << MachErrorCode(kr);
return false;
}
@@ -275,7 +279,9 @@ void MachBroker::InvalidatePid(base::ProcessHandle pid) {
kern_return_t kr = mach_port_deallocate(mach_task_self(),
it->second);
- MACH_LOG_IF(WARNING, kr != KERN_SUCCESS, kr) << "mach_port_deallocate";
+ LOG_IF(WARNING, kr != KERN_SUCCESS)
+ << "Failed to mach_port_deallocate mach task " << it->second
+ << ", error " << MachErrorCode(kr);
mach_map_.erase(it);
}
« no previous file with comments | « chrome/browser/mac/install_from_dmg.mm ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698