Use signal strength of detected networks when displaying remembered networks.

chrome/browser/chromeos/dom_ui/internet_options_handler.cc

Index: chrome/browser/chromeos/dom_ui/internet_options_handler.cc
diff --git a/chrome/browser/chromeos/dom_ui/internet_options_handler.cc b/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
index 1b37ceadaa14506277ab47645ea9ead226e5723e..a41beb9380f8e42c3ce7b23f0e2e5eb6d71b96bc 100644
--- a/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
+++ b/chrome/browser/chromeos/dom_ui/internet_options_handler.cc
@@ -6,6 +6,7 @@
 
 #include <ctype.h>
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -34,6 +35,7 @@
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "grit/theme_resources.h"
+#include "net/base/escape.h"
 #include "views/window/window.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -996,27 +998,63 @@ ListValue* InternetOptionsHandler::GetWirelessList() {
   return list;
 }
 
+std::string GetWifiNetworkKey(const chromeos::WifiNetwork* wifi)

[Charlie Lee, 2011/01/25 17:56:07]

parenthesis should be on the same line. also, I suggest renaming this to
GetWifiUniqueIdentifier or GetWifiUniqueName... something like that. network key
is interchangeable with password in some places, so avoid that.

[falken, 2011/01/26 05:49:20]

I see what you mean about network key.  I don't understand which parenthesis
you're referring to, though..
On 2011/01/25 17:56:07, Charlie Lee wrote:

+{
+  std::stringstream ss;
+  ss << wifi->encryption() << "|" << EscapePath(wifi->name());

[stevenjb, 2011/01/25 18:36:55]

stringsteam is unnecessary, and I'm not clear why we need EscapePath() here. I
think this can just be:
return wifi->encryption() + "|" + wifi->name();
(If there is a reason for EscapePath() that I missed, we should document it).

[falken, 2011/01/26 05:49:20]

Oops, for some reason I was thinking we had to escape in case name has a '|' but
now I see it wouldn't matter.
On 2011/01/25 18:36:55, Steven Bennetts wrote:

+  return ss.str();
+}
+
 ListValue* InternetOptionsHandler::GetRememberedList() {
   chromeos::NetworkLibrary* cros =
       chromeos::CrosLibrary::Get()->GetNetworkLibrary();
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   ListValue* list = new ListValue();
 
-  const chromeos::WifiNetworkVector& wifi_networks =
+  const chromeos::WifiNetworkVector& remembered_wifi_networks =
       cros->remembered_wifi_networks();
-  for (chromeos::WifiNetworkVector::const_iterator it =
-      wifi_networks.begin(); it != wifi_networks.end(); ++it) {
-    SkBitmap icon = *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS0_BLACK);
-    if ((*it)->encrypted()) {
-      icon = chromeos::NetworkMenu::IconForDisplay(icon,
+  const chromeos::WifiNetworkVector& wifi_networks =
+      cros->wifi_networks();
+
+  // The remembered networks from libcros/flimflam don't include the signal
+  // strength, so fall back to the detected networks for this data.  We
+  // consider networks to be the same if they have the same name and encryption
+  // type, so create a map of detected networks indexed by name + encryption.
+  std::map<std::string, chromeos::WifiNetwork*> wifi_map;
+  for (chromeos::WifiNetworkVector::const_iterator it = wifi_networks.begin()
+      ; it != wifi_networks.end(); ++it) {

[Charlie Lee, 2011/01/25 17:56:07]

I believe the ; should be at the end of the previous line.

+    wifi_map[GetWifiNetworkKey(*it)] = *it;
+  }
+
+  for (chromeos::WifiNetworkVector::const_iterator rit =
+      remembered_wifi_networks.begin(); rit != remembered_wifi_networks.end();
+      ++rit) {

[stevenjb, 2011/01/25 18:36:55]

Either put the second clause on its own line and fix the indentation of the
first clause:
  for (chromeos::WifiNetworkVector::const_iterator rit =
           remembered_wifi_networks.begin();
       rit != remembered_wifi_networks.end(); ++rit()) {
or declare rit above like so:
chromeos::WifiNetworkVector::const_iterator rit;
for (rit = remembered_wifi_networks.begin();
     rit != remembered_wifi_networks.end(); ++rit) {

+    // Check if this remembered network has a matching detected network.
+    std::map<std::string, chromeos::WifiNetwork*>::const_iterator it
+        = wifi_map.find(GetWifiNetworkKey(*rit));

[stevenjb, 2011/01/25 18:36:55]

= should be on first line.

+    bool found = it != wifi_map.end();
+
+    // Don't show the active network in the remembered list.
+    if (found && (it->second)->connected())
+      continue;
+    SkBitmap icon;
+    if (found)
+      icon = chromeos::NetworkMenu::IconForNetworkStrength((it->second), true);
+    else
+      icon = *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_BARS0_BLACK);
+    // Place the secure badge on the icon if the remembered network is
+    // encrypted (the matching detected network, if any, will have the same
+    // encrypted property by definition).
+    if ((*rit)->encrypted()) {

[stevenjb, 2011/01/25 18:36:55]

nit: since (*rit) is used a bunch here, might be more readable to assign it to a
local variable, i.e.
    chromeos::WifiNetwork* wifi = *rit;

+        icon = chromeos::NetworkMenu::IconForDisplay(icon,

[Charlie Lee, 2011/01/25 17:56:07]

spacing

           *rb.GetBitmapNamed(IDR_STATUSBAR_NETWORK_SECURE));
     }
     list->Append(GetNetwork(
-        (*it)->service_path(),
+        (*rit)->service_path(),
         icon,
-        (*it)->name(),
-        (*it)->connecting(),
-        (*it)->connected(),
+        (*rit)->name(),
+        (*rit)->connecting(),
+        (*rit)->connected(),
         true,
         chromeos::TYPE_WIFI,
         true,