diff options
9 files changed, 50 insertions, 29 deletions
diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc index 7fbffab..4acd1ca 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.cc +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.cc @@ -518,8 +518,8 @@ void ChromeBrowserMainPartsChromeos::PostProfileInit() { resume_observer_.reset(new ResumeObserver()); screen_lock_observer_.reset(new ScreenLockObserver()); if (KioskModeSettings::Get()->IsKioskModeEnabled()) { - power_state_override_.reset(new PowerStateOverride( - PowerStateOverride::BLOCK_DISPLAY_SLEEP)); + power_state_override_ = new PowerStateOverride( + PowerStateOverride::BLOCK_DISPLAY_SLEEP); } primary_display_switch_observer_.reset( @@ -618,7 +618,7 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() { resume_observer_.reset(); brightness_observer_.reset(); output_observer_.reset(); - power_state_override_.reset(); + power_state_override_ = NULL; // The XInput2 event listener needs to be shut down earlier than when // Singletons are finally destroyed in AtExitManager. diff --git a/chrome/browser/chromeos/chrome_browser_main_chromeos.h b/chrome/browser/chromeos/chrome_browser_main_chromeos.h index c47e21d..8deadca 100644 --- a/chrome/browser/chromeos/chrome_browser_main_chromeos.h +++ b/chrome/browser/chromeos/chrome_browser_main_chromeos.h @@ -69,7 +69,7 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux { scoped_ptr<chromeos::ResumeObserver> resume_observer_; scoped_ptr<chromeos::ScreenLockObserver> screen_lock_observer_; scoped_ptr<chromeos::PowerButtonObserver> power_button_observer_; - scoped_ptr<chromeos::PowerStateOverride> power_state_override_; + scoped_refptr<chromeos::PowerStateOverride> power_state_override_; scoped_ptr<chromeos::PrimaryDisplaySwitchObserver> primary_display_switch_observer_; scoped_ptr<chromeos::UserActivityNotifier> user_activity_notifier_; diff --git a/chrome/browser/chromeos/extensions/power/power_api_browsertest.cc b/chrome/browser/chromeos/extensions/power/power_api_browsertest.cc index f2896d9..99f5918 100644 --- a/chrome/browser/chromeos/extensions/power/power_api_browsertest.cc +++ b/chrome/browser/chromeos/extensions/power/power_api_browsertest.cc @@ -64,6 +64,8 @@ class PowerApiTest : public InProcessBrowserTest { bool boolean_value; result->GetAsBoolean(&boolean_value); EXPECT_EQ(boolean_value, true); + + MessageLoop::current()->RunUntilIdle(); } // Adds an expectation that RequestPowerStateOverrides() will be called once diff --git a/chrome/browser/chromeos/extensions/power/power_api_manager.cc b/chrome/browser/chromeos/extensions/power/power_api_manager.cc index 306a98b..f5627ab 100644 --- a/chrome/browser/chromeos/extensions/power/power_api_manager.cc +++ b/chrome/browser/chromeos/extensions/power/power_api_manager.cc @@ -37,7 +37,7 @@ void PowerApiManager::Observe(int type, UpdatePowerSettings(); } else if (type == chrome::NOTIFICATION_APP_TERMINATING) { // If the Chrome app is terminating, ensure we release our power overrides. - power_state_override_.reset(NULL); + power_state_override_ = NULL; } else { NOTREACHED() << "Unexpected notification " << type; } @@ -57,11 +57,11 @@ PowerApiManager::~PowerApiManager() {} void PowerApiManager::UpdatePowerSettings() { // If we have a wake lock and don't have the power state overriden. if (extension_ids_set_.size() && !power_state_override_.get()) { - power_state_override_.reset(new chromeos::PowerStateOverride( - chromeos::PowerStateOverride::BLOCK_DISPLAY_SLEEP)); + power_state_override_ = new chromeos::PowerStateOverride( + chromeos::PowerStateOverride::BLOCK_DISPLAY_SLEEP); // else, if we don't have any wake locks and do have a power override. } else if (extension_ids_set_.empty() && power_state_override_.get()) { - power_state_override_.reset(NULL); + power_state_override_ = NULL; } } diff --git a/chrome/browser/chromeos/extensions/power/power_api_manager.h b/chrome/browser/chromeos/extensions/power/power_api_manager.h index 30e25bf..cfaf3b9 100644 --- a/chrome/browser/chromeos/extensions/power/power_api_manager.h +++ b/chrome/browser/chromeos/extensions/power/power_api_manager.h @@ -51,7 +51,7 @@ class PowerApiManager : public content::NotificationObserver { content::NotificationRegistrar registrar_; - scoped_ptr<chromeos::PowerStateOverride> power_state_override_; + scoped_refptr<chromeos::PowerStateOverride> power_state_override_; // Set of extension IDs that have a keep awake lock. std::set<std::string> extension_ids_set_; diff --git a/chromeos/power/power_state_override.cc b/chromeos/power/power_state_override.cc index 7c07771..ab9af01 100644 --- a/chromeos/power/power_state_override.cc +++ b/chromeos/power/power_state_override.cc @@ -28,8 +28,7 @@ namespace chromeos { PowerStateOverride::PowerStateOverride(Mode mode) : override_types_(0), request_id_(0), - dbus_thread_manager_(DBusThreadManager::Get()), - weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { + dbus_thread_manager_(DBusThreadManager::Get()) { switch (mode) { case BLOCK_DISPLAY_SLEEP: override_types_ |= (PowerManagerClient::DISABLE_IDLE_DIM | @@ -46,11 +45,20 @@ PowerStateOverride::PowerStateOverride(Mode mode) dbus_thread_manager_->AddObserver(this); // request_id_ = 0 will create a new override request. - CallRequestPowerStateOverrides(); + // We do a post task here to ensure that this request runs 'after' our + // constructor is done. If not, there is a possibility (though only in + // tests at the moment) that the power state override request executes + // and returns before the constructor has finished executing. This will + // cause an AddRef and a Release, the latter destructing our current + // instance even before the constructor has finished executing (as it does + // in the DownloadExtensionTest browsertests currently). + MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&PowerStateOverride::CallRequestPowerStateOverrides, this)); heartbeat_.Start(FROM_HERE, base::TimeDelta::FromSeconds(kHeartbeatTimeInSecs), - weak_ptr_factory_.GetWeakPtr(), + this, &PowerStateOverride::CallRequestPowerStateOverrides); } @@ -78,8 +86,7 @@ void PowerStateOverride::CallRequestPowerStateOverrides() { base::TimeDelta::FromSeconds( kHeartbeatTimeInSecs + kRequestSlackInSecs), override_types_, - base::Bind(&PowerStateOverride::SetRequestId, - weak_ptr_factory_.GetWeakPtr())); + base::Bind(&PowerStateOverride::SetRequestId, this)); } void PowerStateOverride::CancelRequest() { diff --git a/chromeos/power/power_state_override.h b/chromeos/power/power_state_override.h index 8a180ea..b2e2572 100644 --- a/chromeos/power/power_state_override.h +++ b/chromeos/power/power_state_override.h @@ -17,7 +17,9 @@ class DBusThreadManager; // This class overrides the current power state on the machine, disabling // a set of power management features. -class CHROMEOS_EXPORT PowerStateOverride : public DBusThreadManagerObserver { +class CHROMEOS_EXPORT PowerStateOverride + : public base::RefCountedThreadSafe<PowerStateOverride>, + public DBusThreadManagerObserver { public: enum Mode { // Blocks the screen from being dimmed or blanked due to user inactivity. @@ -30,13 +32,21 @@ class CHROMEOS_EXPORT PowerStateOverride : public DBusThreadManagerObserver { }; explicit PowerStateOverride(Mode mode); - virtual ~PowerStateOverride(); // DBusThreadManagerObserver implementation: virtual void OnDBusThreadManagerDestroying(DBusThreadManager* manager) OVERRIDE; private: + friend class base::RefCountedThreadSafe<PowerStateOverride>; + + // This destructor cancels the current power state override. There might be + // a very slight delay between the the last reference to an instance being + // released and the power state override being canceled. This is only in + // the case that the current instance has JUST been created and ChromeOS + // hasn't had a chance to service the initial power override request yet. + virtual ~PowerStateOverride(); + // Callback from RequestPowerStateOverride which receives our request_id. void SetRequestId(uint32 request_id); @@ -62,8 +72,6 @@ class CHROMEOS_EXPORT PowerStateOverride : public DBusThreadManagerObserver { DBusThreadManager* dbus_thread_manager_; // not owned - base::WeakPtrFactory<PowerStateOverride> weak_ptr_factory_; - DISALLOW_COPY_AND_ASSIGN(PowerStateOverride); }; diff --git a/chromeos/power/power_state_override_unittest.cc b/chromeos/power/power_state_override_unittest.cc index cba0feb..7b3c7da 100644 --- a/chromeos/power/power_state_override_unittest.cc +++ b/chromeos/power/power_state_override_unittest.cc @@ -68,24 +68,26 @@ TEST_F(PowerStateOverrideTest, AddAndRemoveOverrides) { EXPECT_CALL(*power_manager_client_, RequestPowerStateOverrides(0, _, kDisplayOverrides, _)) .WillOnce(SaveArg<3>(&request_id_callback)); - scoped_ptr<PowerStateOverride> override( + scoped_refptr<PowerStateOverride> override( new PowerStateOverride(PowerStateOverride::BLOCK_DISPLAY_SLEEP)); + // Make sure our PostTask to the request power state overrides runs. + message_loop_.RunUntilIdle(); request_id_callback.Run(kRequestId); // The request should be canceled when the PowerStateOverride is destroyed. EXPECT_CALL(*power_manager_client_, CancelPowerStateOverrides(kRequestId)); - override.reset(); + override = NULL; // Now send a request to just block the system from suspending. - Mock::VerifyAndClearExpectations(power_manager_client_); EXPECT_CALL(*power_manager_client_, RequestPowerStateOverrides(0, _, kSystemOverrides, _)) .WillOnce(SaveArg<3>(&request_id_callback)); - override.reset( - new PowerStateOverride(PowerStateOverride::BLOCK_SYSTEM_SUSPEND)); + override = new PowerStateOverride(PowerStateOverride::BLOCK_SYSTEM_SUSPEND); + // Make sure our PostTask to the request power state overrides runs. + message_loop_.RunUntilIdle(); request_id_callback.Run(kRequestId); EXPECT_CALL(*power_manager_client_, CancelPowerStateOverrides(kRequestId)); - override.reset(); + override = NULL; } TEST_F(PowerStateOverrideTest, DBusThreadManagerShutDown) { @@ -93,8 +95,10 @@ TEST_F(PowerStateOverrideTest, DBusThreadManagerShutDown) { chromeos::PowerStateRequestIdCallback request_id_callback; EXPECT_CALL(*power_manager_client_, RequestPowerStateOverrides(0, _, _, _)) .WillOnce(SaveArg<3>(&request_id_callback)); - scoped_ptr<PowerStateOverride> override( + scoped_refptr<PowerStateOverride> override( new PowerStateOverride(PowerStateOverride::BLOCK_DISPLAY_SLEEP)); + // Make sure our PostTask to the request power state overrides runs. + message_loop_.RunUntilIdle(); request_id_callback.Run(kRequestId); // When the DBusThreadManager is about to be shut down, PowerStateOverride diff --git a/content/browser/power_save_blocker_chromeos.cc b/content/browser/power_save_blocker_chromeos.cc index afb0cc2..de9d07b 100644 --- a/content/browser/power_save_blocker_chromeos.cc +++ b/content/browser/power_save_blocker_chromeos.cc @@ -34,13 +34,13 @@ class PowerSaveBlocker::Delegate default: NOTREACHED() << "Unhandled block type " << type_; } - override_.reset(new chromeos::PowerStateOverride(mode)); + override_ = new chromeos::PowerStateOverride(mode); } // Resets the previously-created PowerStateOverride object. void RemoveBlock() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - override_.reset(); + override_ = NULL; } private: @@ -49,7 +49,7 @@ class PowerSaveBlocker::Delegate PowerSaveBlockerType type_; - scoped_ptr<chromeos::PowerStateOverride> override_; + scoped_refptr<chromeos::PowerStateOverride> override_; DISALLOW_COPY_AND_ASSIGN(Delegate); }; |