Content: Implement PowerSaveBlocker2 for the Mac.

content/browser/power_save_blocker_mac.cc

Index: content/browser/power_save_blocker_mac.cc
diff --git a/content/browser/power_save_blocker_mac.cc b/content/browser/power_save_blocker_mac.cc
index edb5bdd3e4fdcd4fba55a5ac825a10fc14d92d5a..71bd57bcabed84cece23cfd2ab33659eaa5d43b0 100644
--- a/content/browser/power_save_blocker_mac.cc
+++ b/content/browser/power_save_blocker_mac.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -7,6 +7,7 @@
 #include <IOKit/pwr_mgt/IOPMLib.h>
 
 #include "base/bind.h"
+#include "base/lazy_instance.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "content/public/browser/browser_thread.h"
@@ -71,3 +72,106 @@ void PowerSaveBlocker::ApplyBlock(PowerSaveBlockerType type) {
   g_power_thread->message_loop()->
       PostTask(FROM_HERE, base::Bind(CreateSleepAssertion, type));
 }
+
+// ====== ^^ the deprecated way ^^ vv the way that doesn't work yet vv ======

[Nico, 2012/06/05 20:30:11]

Why not wait with committing until it works?

[Mike Mammarella, 2012/06/05 20:39:30]

It would work, but only on OS X. There are three of us working on the different
platforms, and we can't switch over the callers of this API until we have all
the platforms in place.

[Avi (use Gerrit), 2012/06/05 20:40:24]

This is a joke; the new code works perfectly.

[Nico, 2012/06/05 21:08:45]

Reword the comment to "// TODO: The below will replace the above code soon,
http://crbug.com/..."

+
+namespace {
+
+// Power management cannot be done on the UI thread. IOPMAssertionCreate does a
+// synchronous MIG call to configd, so if it is called on the main thread the UI
+// is at the mercy of another process. See http://crbug.com/79559 and
+// http://www.opensource.apple.com/source/IOKitUser/IOKitUser-514.16.31/pwr_mgt.subproj/IOPMLibPrivate.c .
+struct PowerSaveBlockerLazyInstanceTraits {
+  static const bool kRegisterOnExit = false;
+  static const bool kAllowedToAccessOnNonjoinableThread = true;
+
+  static base::Thread* New(void* instance) {
+    base::Thread* thread = new (instance) base::Thread("PowerSaveBlocker");
+    thread->Start();

[Nico, 2012/06/05 21:08:45]

It's also surprising to me that g_power_thread2->Pointer() implicitly starts a
thread -- can't you keep the thread creation technique used in the class above?
That's more explicit, and from what I understand the "Start()" code path
executed in just one function anyway (PSB() constructor) (?)

[Avi (use Gerrit), 2012/06/05 21:11:41]

That worked in the code above because PowerSaveBlocker::ApplyBlock was
guaranteed to always be on the UI thread. Therefore we didn't have to worry
about threading. Here, there's no longer any guarantees about what thread
PowerSaveBlocker2 will be instantiated on, so we need protection against races.

+    return thread;
+  }
+  static void Delete(base::Thread* instance) { }
+};
+base::LazyInstance<base::Thread, PowerSaveBlockerLazyInstanceTraits>
+    g_power_thread2 = LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+// TODO(avi): Remove after the old interface goes away.
+#define PowerSaveBlocker PowerSaveBlocker2

[Nico, 2012/06/05 20:30:11]

This is confusing to me. Why not remove this define and call the new interface
PSB2 until PSB is gone? That's how we usually do this I believe.

[Mike Mammarella, 2012/06/05 20:39:30]

This makes the eventual patch smaller since we just delete this one line, rather
than changing all the instances and potentially needing to reformat the code.

The timescale here is a few days at most so it seems reasonable to do it this
way.

[Avi (use Gerrit), 2012/06/05 20:40:24]

The new interface _is_ called PSB2. This is used in the implementation file to
save renaming everything. See https://chromiumcodereview.appspot.com/10538013/

[Nico, 2012/06/05 20:42:02]

With "Everything" just being the 77 lines below this define, right?

[Avi (use Gerrit), 2012/06/05 20:58:30]

Yes, that is the definition of "everything" we're working with here.

[Nico, 2012/06/05 21:08:45]

Changing that takes about 2s in a text editor. I don't think having this define
is worth it.

+
+namespace content {
+
+class PowerSaveBlocker::Delegate
+    : public base::RefCountedThreadSafe<PowerSaveBlocker::Delegate> {
+ public:
+  Delegate(PowerSaveBlockerType type, const std::string& reason)
+      : type_(type), reason_(reason), assertion_(kIOPMNullAssertionID) {}
+
+  // Does the actual work to apply or remove the desired power save block.
+  void ApplyBlock();
+  void RemoveBlock();
+
+ private:
+  friend class base::RefCountedThreadSafe<PowerSaveBlocker::Delegate>;
+  ~Delegate() {}
+  PowerSaveBlockerType type_;
+  std::string reason_;
+  IOPMAssertionID assertion_;
+};
+
+void PowerSaveBlocker::Delegate::ApplyBlock() {
+  DCHECK_EQ(base::PlatformThread::CurrentId(),
+            g_power_thread2.Pointer()->thread_id());
+
+  CFStringRef level = NULL;
+  // See QA1340 <http://developer.apple.com/library/mac/#qa/qa1340/> for more
+  // details.
+  switch (type_) {
+    case PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension:
+      level = kIOPMAssertionTypeNoIdleSleep;
+      break;
+    case PowerSaveBlocker::kPowerSaveBlockPreventDisplaySleep:
+      level = kIOPMAssertionTypeNoDisplaySleep;
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+  if (level) {
+    // TODO(avi): Switch to IOPMAssertionCreateWithName when 10.6 is the minimum
+    // system supported by Chromium.
+    IOReturn result = IOPMAssertionCreate(level,
+                                          kIOPMAssertionLevelOn,
+                                          &assertion_);
+    LOG_IF(ERROR, result != kIOReturnSuccess)
+        << "IOPMAssertionCreate: " << result;
+  }
+}
+
+void PowerSaveBlocker::Delegate::RemoveBlock() {
+  DCHECK_EQ(base::PlatformThread::CurrentId(),
+            g_power_thread2.Pointer()->thread_id());
+
+  if (assertion_ != kIOPMNullAssertionID) {
+    IOReturn result = IOPMAssertionRelease(assertion_);
+    LOG_IF(ERROR, result != kIOReturnSuccess)
+        << "IOPMAssertionRelease: " << result;
+  }
+}
+
+PowerSaveBlocker::PowerSaveBlocker(PowerSaveBlockerType type,
+                                   const std::string& reason)
+    : delegate_(new PowerSaveBlocker::Delegate(type, reason)) {
+  g_power_thread2.Pointer()->message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&PowerSaveBlocker::Delegate::ApplyBlock, delegate_));
+}
+
+PowerSaveBlocker::~PowerSaveBlocker() {
+  g_power_thread2.Pointer()->message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&PowerSaveBlocker::Delegate::RemoveBlock, delegate_));
+}
+
+}  // namespace content