Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 10378856
or point your web browser to
http://mondrian/10378856
to review the following code:
Change 10378856 by stevebl...@steveblock-gears3 on 2009/03/05 13:16:15 *pending*
Implements an improved policy for WiFi scanning frequency. The
frequency is decreased if successive scans record no change in access points,
and increased again as soon as a change is detected.
R=andreip
[email protected]
DELTA=62 (47 added, 2 deleted, 13 changed)
OCL=10378856
Affected files ...
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc#7
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h#4
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc#7
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc#9
edit
62 delta lines: 47 added, 2 deleted, 13 changed
Also consider running:
g4 lint -c 10378856
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 10378856 by stevebl...@steveblock-gears3 on 2009/03/05 13:16:15 *pending*
Implements an improved policy for WiFi scanning frequency. The
frequency is decreased if successive scans record no change in access points,
and increased again as soon as a change is detected.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc#7
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h#4
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc#7
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc#9
edit
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc#7
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc
====
# action=edit type=text
---
googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc
2009-03-05 15:40:49.000000000 +0000
+++
googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.cc
2009-03-05 15:36:19.000000000 +0000
@@ -23,9 +23,14 @@
// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#include "gears/geolocation/wifi_data_provider_common.h"
+
+// These constants are defined for each platfrom in wifi_data_provider_xxx.cc.
+extern const int kDefaultPollingInterval;
+extern const int kNoChangePollingInterval;
+extern const int kTwoNoChangePollingInterval;
+
#if defined(WIN32) || defined(OS_MACOSX)
-
-#include "gears/geolocation/wifi_data_provider_common.h"
#include <assert.h>
#if defined(WIN32)
@@ -70,3 +75,16 @@
}
#endif // WIN32 || OS_MACOSX
+
+int UpdatePollingInterval(int polling_interval, bool scan_results_differ) {
+ if (scan_results_differ) {
+ return kDefaultPollingInterval;
+ }
+ if (polling_interval == kDefaultPollingInterval) {
+ return kNoChangePollingInterval;
+ } else {
+ assert(polling_interval == kNoChangePollingInterval ||
+ polling_interval == kTwoNoChangePollingInterval);
+ return kTwoNoChangePollingInterval;
+ }
+}
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h#4
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h
2009-03-05 15:40:49.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_common.h
2009-03-05 15:23:40.000000000 +0000
@@ -29,6 +29,11 @@
#include "gears/base/common/string16.h"
#include "gears/base/common/basictypes.h"
+// Converts a MAC address stored as an array of uint8 to a string.
std::string16 MacAddressAsString16(const uint8 mac_as_int[6]);
+// Calculates the new polling interval for wiFi scans, given the previous
+// interval and whether the last scan produced new results.
+int UpdatePollingInterval(int polling_interval, bool scan_results_differ);
+
#endif // GEARS_GEOLOCATION_WIFI_DATA_PROVIDER_COMMON_H__
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc#7
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc
2009-03-05 11:46:31.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_linux.cc
2009-03-05 15:32:29.000000000 +0000
@@ -74,9 +74,12 @@
#include <ctype.h> // For isxdigit()
#include <stdio.h>
#include "gears/base/common/string_utils.h"
-
-// The time period, in milliseconds, between successive polls of the WiFi data.
-static const int kPollingInterval = 1000;
+#include "gears/geolocation/wifi_data_provider_common.h"
+
+// The time periods, in milliseconds, between successive polls of the wifi
data.
+extern const int kDefaultPollingInterval = 10000; // 10s
+extern const int kNoChangePollingInterval = 120000; // 2 mins
+extern const int kTwoNoChangePollingInterval = 600000; // 10 mins
// Local function
static bool GetAccessPointData(std::vector<AccessPointData> *access_points);
@@ -110,6 +113,7 @@
// Thread implementation
void LinuxWifiDataProvider::Run() {
// Regularly get the access point data.
+ int polling_interval = kDefaultPollingInterval;
do {
WifiData new_data;
if (GetAccessPointData(&new_data.access_point_data)) {
@@ -119,12 +123,14 @@
wifi_data_ = new_data;
is_first_scan_complete_ = true;
}
+ polling_interval =
+ UpdatePollingInterval(polling_interval, update_available);
data_mutex_.Unlock();
if (update_available) {
NotifyListeners();
}
}
- } while (!stop_event_.WaitWithTimeout(kPollingInterval));
+ } while (!stop_event_.WaitWithTimeout(polling_interval));
}
// Local functions
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc#5
-
c:\MyDocs\Gears3/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-05 11:46:31.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_osx.cc
2009-03-05 15:32:51.000000000 +0000
@@ -39,8 +39,10 @@
#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
+// The time periods, in milliseconds, between successive polls of the wifi
data.
+extern const int kDefaultPollingInterval = 30000; // 30s
+extern const int kNoChangePollingInterval = 120000; // 2 mins
+extern const int kTwoNoChangePollingInterval = 600000; // 10 mins
// static
template<>
@@ -93,6 +95,7 @@
}
// Regularly get the access point data.
+ int polling_interval = kDefaultPollingInterval;
do {
WifiData new_data;
GetAccessPointData(&new_data.access_point_data);
@@ -102,11 +105,13 @@
wifi_data_ = new_data;
is_first_scan_complete_ = true;
}
+ polling_interval =
+ UpdatePollingInterval(polling_interval, update_available);
data_mutex_.Unlock();
if (update_available) {
NotifyListeners();
}
- } while (!stop_event_.WaitWithTimeout(kPollingInterval));
+ } while (!stop_event_.WaitWithTimeout(polling_interval));
(*WirelessDetach_function_)(wifi_context_);
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc#13
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc
2009-03-05 11:46:31.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_win32.cc
2009-03-05 15:36:29.000000000 +0000
@@ -66,8 +66,10 @@
// Length for generic string buffers passed to Win32 APIs.
static const int kStringLength = 512;
-// The time period, in milliseconds, between successive polls of the wifi data.
-static const int kPollingInterval = 1000;
+// The time periods, in milliseconds, between successive polls of the wifi
data.
+extern const int kDefaultPollingInterval = 10000; // 10s
+extern const int kNoChangePollingInterval = 120000; // 2 mins
+extern const int kTwoNoChangePollingInterval = 600000; // 10 mins
// Local functions
@@ -148,6 +150,7 @@
}
assert(get_access_point_data_function);
+ int polling_interval = kDefaultPollingInterval;
// Regularly get the access point data.
do {
WifiData new_data;
@@ -158,12 +161,14 @@
wifi_data_ = new_data;
is_first_scan_complete_ = true;
}
+ polling_interval =
+ UpdatePollingInterval(polling_interval, update_available);
data_mutex_.Unlock();
if (update_available) {
NotifyListeners();
}
}
- } while (!stop_event_.WaitWithTimeout(kPollingInterval));
+ } while (!stop_event_.WaitWithTimeout(polling_interval));
FreeLibrary(library);
}
====
//depot/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc#9
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc
2008-11-10 14:29:58.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/wifi_data_provider_wince.cc
2009-03-05 15:33:30.000000000 +0000
@@ -35,11 +35,14 @@
#include <windows.h>
#include <winioctl.h> // For IOCTL_NDISUIO_QUERY_OID_VALUE
#include "gears/base/common/string_utils.h" // For UTF8ToString16()
+#include "gears/geolocation/wifi_data_provider_common.h"
#include "gears/geolocation/wifi_data_provider_windows_common.h"
#include "third_party/scoped_ptr/scoped_ptr.h"
-// The time period, in milliseconds, between successive polls of the wifi data.
-static const int kPollingInterval = 1000;
+// The time periods, in milliseconds, between successive polls of the wifi
data.
+extern const int kDefaultPollingInterval = 1000; // 1s
+extern const int kNoChangePollingInterval = 10000; // 10s
+extern const int kTwoNoChangePollingInterval = 120000; // 2 mins
// Local function.
static bool GetAccessPointData(const HANDLE &ndis_handle,
@@ -88,6 +91,7 @@
}
// Regularly get the access point data.
+ int polling_interval = kDefaultPollingInterval;
do {
WifiData new_data;
if (GetAccessPointData(ndis_handle, &new_data.access_point_data)) {
@@ -97,12 +101,14 @@
wifi_data_ = new_data;
is_first_scan_complete_ = true;
}
+ polling_interval =
+ UpdatePollingInterval(polling_interval, update_available);
data_mutex_.Unlock();
if (update_available) {
NotifyListeners();
}
}
- } while (!stop_event_.WaitWithTimeout(kPollingInterval));
+ } while (!stop_event_.WaitWithTimeout(polling_interval));
CloseHandle(ndis_handle);
}
