Use signal strength of detected networks when displaying remembered networks.

chrome/browser/chromeos/dom_ui/internet_options_handler.cc

 // Copyright (c) 2010 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.
 
 #include "chrome/browser/chromeos/dom_ui/internet_options_handler.h"
 
 #include <ctype.h>
 
+#include <map>
 #include <string>
 #include <vector>
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/base64.h"
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/i18n/time_formatting.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/login/ownership_service.h"
 #include "chrome/browser/chromeos/status/network_menu.h"
 #include "chrome/browser/dom_ui/dom_ui_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/window.h"
 #include "chrome/common/time_format.h"
 #include "grit/browser_resources.h"
 #include "grit/chromium_strings.h"
 #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"
 
 static const char kOtherNetworksFakePath[] = "?";
 
 namespace {
 
 // Format the hardware address like "0011AA22BB33" => "00:11:AA:22:BB:33".
 std::string FormatHardwareAddress(const std::string& address) {
   std::string output;
@@ skipping 942 matching lines @@
         true,
         chromeos::TYPE_WIFI,
         false,
         chromeos::ACTIVATION_STATE_UNKNOWN,
         false));
   }
 
   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& remembered_wifi_networks =
+      cros->remembered_wifi_networks();
   const chromeos::WifiNetworkVector& 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,
+      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,
         chromeos::ACTIVATION_STATE_UNKNOWN,
         false));
   }
   return list;
 }
 
 void InternetOptionsHandler::FillNetworkInfo(
     DictionaryValue* dictionary, chromeos::NetworkLibrary* cros) {
   dictionary->Set("wiredList", GetWiredList());
   dictionary->Set("wirelessList", GetWirelessList());
   dictionary->Set("rememberedList", GetRememberedList());
   dictionary->SetBoolean("wifiAvailable", cros->wifi_available());
   dictionary->SetBoolean("wifiEnabled", cros->wifi_enabled());
   dictionary->SetBoolean("cellularAvailable", cros->cellular_available());
   dictionary->SetBoolean("cellularEnabled", cros->cellular_enabled());
 }