Connectivity Diagnostics API: chrome.diagnostics.sendPacket

chrome/browser/extensions/api/diagnostics_private/send_ping_packet_chromeos.cc

+// Copyright (c) 2013 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 "send_ping_packet.h"
+
+#include "base/bind.h"
+#include "base/json/json_reader.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/debug_daemon_client.h"
+
+namespace extensions {
+
+namespace {
+
+void ParseResult(const SendPingPacketCallback& callback,
+                 bool succeeded,
+                 const std::string& status) {
+  if (!succeeded) {
+    callback.Run("Failed sending packet", "", 0.0);
+    return;
+  }
+  // Parses the result and returns IP and latency.
+  scoped_ptr<base::Value> parsed_value(base::JSONReader::Read(status));
+  base::DictionaryValue* result;
+  if (!parsed_value.get() || !parsed_value->GetAsDictionary(&result)) {
+    callback.Run("Malformed reply from the backend.", "", 0.0);
+    return;
+  }
+
+  // Returns the first item.
+  base::DictionaryValue::Iterator iterator(*result);
+  if (iterator.IsAtEnd()) {
+    callback.Run("Empty result from the backend.", "", 0.0);
+    return;
+  }
+
+  std::string ip = iterator.key();

[xiaowenx, 2013/06/18 19:07:37]

const std::string& ip = iterator.key()?

[Bei Zhang, 2013/06/18 22:04:46]

Done.

+  const base::DictionaryValue* info;
+  if (iterator.value().GetAsDictionary(&info)) {
+    double latency;
+    if (info->GetDouble("avg", &latency)) {
+      callback.Run("", ip, latency);
+      return;
+    }
+  }
+
+  callback.Run("Malformed reply from the backend.", "", 0.0);
+  return;

[xiaowenx, 2013/06/18 19:07:37]

Unnecessary return?  The other way to do this is to continue checking for error,
and then do callback.Run(<success case>) at the very bottom of this method.

[stevenjb, 2013/06/18 19:19:28]

+1. I'd suggest:

  const base::DictionaryValue* info;
  double latency;
  if (!iterator.value().GetAsDictionary(&info) ||
      !info->GetDouble("avg", &latency)) {
    callback.Run("Malformed reply from the backend.", "", 0.0);
    return;
  }
  // Success
  callback.Run("", ip, latency);
}

(Note, you won't need a "Success" comment if you switch to using a result code
instead of using an empty error string to indicate success)
  

[Bei Zhang, 2013/06/18 22:04:46]

Done.

+}
+
+}  // namespace
+
+void SendPingPacket(
+    const std::string& ip,
+    const int* ttl,
+    const int* timeout,
+    const int* size,
+    const SendPingPacketCallback& callback) {
+  chromeos::DebugDaemonClient* debugd_client =
+      chromeos::DBusThreadManager::Get()->GetDebugDaemonClient();

[stevenjb, 2013/06/18 19:19:28]

no need for local used just once further down

[Bei Zhang, 2013/06/18 22:04:46]

Actually, I should test whether they are available. 

+  std::map<std::string, std::string> config;
+  config["count"] = "1";

[stevenjb, 2013/06/18 19:19:28]

"count" and other keys should be consts defined at the top of the file (in the
anon namespace)

[Bei Zhang, 2013/06/18 22:04:46]

Done.

+  if (ttl) config["ttl"] = base::IntToString(*ttl);

[stevenjb, 2013/06/18 19:19:28]

2 lines here and below

[Bei Zhang, 2013/06/18 22:04:46]

Done.

+  if (timeout) config["timeout"] = base::IntToString(*timeout);
+  if (size) config["size"] = base::IntToString(*size);
+  debugd_client->TestICMPWithOptions(
+      ip,
+      config,
+      base::Bind(ParseResult, callback)
+  );
+}
+
+}  // namespace extensions