Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 10347932
or point your web browser to
http://mondrian/10347932
(this changelist has been uploaded to Mondrian)
to review the following code:
Change 10347932 by stevebl...@steveblock-gearsosx1 on 2009/03/03 13:22:22
*pending*
Changes to WiFi scanning on OSX ...
- Further decreases regular scanning frequency.
- Make additional scans on resume from sleep.
PRESUBMIT=passed
R=andreip
[email protected]
DELTA=103 (95 added, 0 deleted, 8 changed)
OCL=10347932
Affected files ...
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h#3
edit
... //depot/googleclient/gears/opensource/gears/tools/config.mk#104 edit
103 delta lines: 95 added, 0 deleted, 8 changed
Also consider running:
g4 lint -c 10347932
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 10347932 by stevebl...@steveblock-gearsosx1 on 2009/03/03 13:22:22
*pending*
Changes to WiFi scanning on OSX ...
- Further decreases regular scanning frequency.
- Make additional scans on resume from sleep.
OCL=10347932
Affected files ...
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h#3
edit
... //depot/googleclient/gears/opensource/gears/tools/config.mk#104 edit
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
-
/Users/steveblock/GearsOsx1/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc
2009-03-03 13:22:36.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc
2009-03-03 13:53:43.000000000 +0000
@@ -35,12 +35,16 @@
#include "gears/geolocation/wifi_data_provider_osx.h"
#include <dlfcn.h>
+#include <IOKit/pwr_mgt/IOPMLib.h>
+#include <IOKit/IOMessage.h>
#include <stdio.h>
#include "gears/base/common/string_utils.h"
#include "gears/geolocation/wifi_data_provider_common.h"
// The time period, in milliseconds, between successive polls of the WiFi data.
-static const int kPollingInterval = 30000; // 30s
+static const int kPollingInterval = 300000; // 5 mins
+
+static io_connect_t root_port; // The Root Power Domain IOService.
// static
template<>
@@ -49,12 +53,19 @@
}
-OsxWifiDataProvider::OsxWifiDataProvider() : is_first_scan_complete_(false) {
+OsxWifiDataProvider::OsxWifiDataProvider()
+ : is_first_scan_complete_(false),
+ is_shutting_down_(false) {
+ if (!RegisterForPowerNotifications()) {
+ LOG(("Failed to register for power notifications.\n"));
+ }
Start();
}
OsxWifiDataProvider::~OsxWifiDataProvider() {
- stop_event_.Signal();
+ UnregisterForPowerNotifications();
+ is_shutting_down_ = true;
+ run_event_.Signal();
Join();
}
@@ -93,7 +104,7 @@
}
// Regularly get the access point data.
- do {
+ while (!is_shutting_down_) {
WifiData new_data;
GetAccessPointData(&new_data.access_point_data);
bool update_available;
@@ -106,7 +117,10 @@
if (update_available) {
NotifyListeners();
}
- } while (!stop_event_.WaitWithTimeout(kPollingInterval));
+
+ // Wait for timeout or the event.
+ run_event_.WaitWithTimeout(kPollingInterval);
+ }
(*WirelessDetach_function_)(wifi_context_);
@@ -158,4 +172,69 @@
}
}
+bool OsxWifiDataProvider::RegisterForPowerNotifications() {
+ // Register to receive system sleep notifications.
+ root_port = IORegisterForSystemPower(this,
+ ¬ification_port_,
+ OnPowerNotification,
+ ¬ifier_object_);
+ if (root_port == 0) {
+ return false;
+ }
+
+ // Add the notification port to the application runloop.
+ CFRunLoopAddSource(CFRunLoopGetCurrent(),
+ IONotificationPortGetRunLoopSource(notification_port_),
+ kCFRunLoopCommonModes);
+
+ // The main browser thread runs the run loop and we receive notifications on
+ // that thread.
+ return true;
+}
+
+void OsxWifiDataProvider::UnregisterForPowerNotifications() {
+ // Remove the sleep notification port from the application runloop.
+ CFRunLoopRemoveSource(CFRunLoopGetCurrent(),
+ IONotificationPortGetRunLoopSource(notification_port_),
+ kCFRunLoopCommonModes);
+
+ // Deregister for system sleep notifications.
+ IODeregisterForSystemPower(¬ifier_object_);
+
+ // IORegisterForSystemPower implicitly opens the Root Power Domain IOService
+ // so we close it here.
+ IOServiceClose(root_port);
+
+ // Destroy the notification port allocated by IORegisterForSystemPower.
+ IONotificationPortDestroy(notification_port_);
+}
+
+void OsxWifiDataProvider::OnResume() {
+ LOG(("Resumed from sleep.\n"));
+ run_event_.Signal();
+}
+
+
+// Local function
+void OnPowerNotification(void *user_data,
+ io_service_t, // service
+ natural_t message_type,
+ void *message_argument) {
+ switch (message_type)
+ {
+ case kIOMessageCanSystemSleep:
+ case kIOMessageSystemWillSleep:
+ // Allow idle sleep.
+ IOAllowPowerChange(root_port, reinterpret_cast<long>(message_argument));
+ break;
+
+ case kIOMessageSystemWillPowerOn:
+ // System has started the wake up process.
+ OsxWifiDataProvider *me =
+ reinterpret_cast<OsxWifiDataProvider*>(user_data);
+ me->OnResume();
+ break;
+ }
+}
+
#endif // OS_MACOSX
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h#3
-
/Users/steveblock/GearsOsx1/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h
2009-03-03 13:22:36.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.h
2009-03-03 13:25:40.000000000 +0000
@@ -36,6 +36,10 @@
class OsxWifiDataProvider
: public WifiDataProviderImplBase,
public Thread {
+friend void OnPowerNotification(void *user_data,
+ io_service_t, // service
+ natural_t message_type,
+ void *message_argument);
public:
OsxWifiDataProvider();
virtual ~OsxWifiDataProvider();
@@ -49,6 +53,11 @@
void GetAccessPointData(std::vector<AccessPointData> *access_points);
+ // For power notofications.
+ bool RegisterForPowerNotifications();
+ void UnregisterForPowerNotifications();
+ void OnResume();
+
// Context and function pointers for Apple80211 library.
WirelessContextPtr wifi_context_;
WirelessAttachFunction WirelessAttach_function_;
@@ -58,13 +67,20 @@
WifiData wifi_data_;
Mutex data_mutex_;
- // Event signalled to shut down the thread that polls for wifi data.
- Event stop_event_;
+ // Event signalled to wake up the thread that polls for wifi data.
+ Event run_event_;
// Whether we've successfully completed a scan for WiFi data (or the polling
// thread has terminated early).
bool is_first_scan_complete_;
+ // Whether the worker thread should shut down.
+ bool is_shutting_down_;
+
+ // For power notifications.
+ IONotificationPortRef notification_port_;
+ io_object_t notifier_object_;
+
DISALLOW_EVIL_CONSTRUCTORS(OsxWifiDataProvider);
};
==== //depot/googleclient/gears/opensource/gears/tools/config.mk#104 -
/Users/steveblock/GearsOsx1/googleclient/gears/opensource/gears/tools/config.mk
====
# action=edit type=text
--- googleclient/gears/opensource/gears/tools/config.mk 2009-03-03
13:22:36.000000000 +0000
+++ googleclient/gears/opensource/gears/tools/config.mk 2009-03-03
12:14:33.000000000 +0000
@@ -692,7 +692,7 @@
MKDLL = g++
DLL_PREFIX = lib
DLL_SUFFIX = .dylib
-DLLFLAGS = $(SHARED_LINKFLAGS) -bundle -framework Carbon -framework
CoreServices -framework Cocoa -framework SystemConfiguration
+DLLFLAGS = $(SHARED_LINKFLAGS) -bundle -framework Carbon -framework
CoreServices -framework Cocoa -framework SystemConfiguration -framework IOKit
ifeq ($(BROWSER),SF)
DLLFLAGS += -mmacosx-version-min=10.4 -framework WebKit -lcurl
DLLFLAGS += -Ltools/osx -lleopard_support
