Connectivity Diagnostics API: chrome.diagnostics.sendPacket

chrome/browser/extensions/api/diagnostics/diagnostics_api_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 "chrome/browser/extensions/api/diagnostics/diagnostics_api.h"
+
+#include "base/bind.h"
+#include "base/callback.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 {
+
+const char kCount[] = "count";
+const char kDefaultCount[] = "1";
+const char kTTL[] = "ttl";
+const char kTimeout[] = "timeout";
+const char kSize[] = "size";
+
+void ParseResult(
+    const base::Callback<
+        void(DiagnosticsSendPacketFunction::SendPacketResultCode result_code,
+             const std::string& ip, double latency)>& callback,

[not at google - send to devlin, 2013/06/21 17:13:24]

TestICMPCallback?

[Bei Zhang, 2013/06/21 19:31:44]

Done. I prefer SendPacketCallback because it's not directly called from
DebugDaemonClient.

[not at google - send to devlin, 2013/06/22 00:14:32]

Well there was already a TestICMPCallback in that other file.

+    bool succeeded, const std::string& status) {

[stevenjb, 2013/06/21 18:35:21]

Each parameter on its own line

[Bei Zhang, 2013/06/21 19:31:44]

Done.

+  do {
+    if (!succeeded)
+      break;
+
+    // Parses the result and returns IP and latency.
+    scoped_ptr<base::Value> parsed_value(base::JSONReader::Read(status));
+    if (!parsed_value)
+      break;
+
+    base::DictionaryValue* result = NULL;
+    if (!parsed_value->GetAsDictionary(&result) || result->size() != 1)
+      break;
+
+    // Returns the first item.
+    base::DictionaryValue::Iterator iterator(*result);
+    const std::string& ip = iterator.key();
+    const base::DictionaryValue* info;
+    if (!iterator.value().GetAsDictionary(&info))
+      break;
+
+    double latency;
+    if (info->GetDouble("avg", &latency))
+      break;
+
+    callback.Run(DiagnosticsSendPacketFunction::SEND_PACKET_OK, ip, latency);
+    return;
+  } while (false);

[not at google - send to devlin, 2013/06/21 17:13:24]

I hope there is a way to factor this without emulating goto.

[Jeffrey Yasskin, 2013/06/21 17:33:50]

I suppose another way is to just write the goto instead of obfuscating it inside
a do/while. One of the few places goto is useful is for error handling. You'd
still need the scope though, since goto can't jump into the middle of a
variable's lifetime.

[not at google - send to devlin, 2013/06/21 17:40:44]

I would prefer to factor it into a function that returns success/failure, on
failure do the SEND_PACKET_FAILED.

Incidentally - what does it mean for the response from that library to be
malformed? I am wondering whether these should CHECK. But, whatever.

[stevenjb, 2013/06/21 18:35:21]

+1, this should be broken up into a subfunction that sets ip and latency and
returns true, or returns false. Then the callback can be run with OK or FAILED
from here.

[Bei Zhang, 2013/06/21 19:31:44]

This idea is great! We are actually using the do-while-false pattern on some
other places. I probably should clean them up.

On 2013/06/21 18:35:21, stevenjb (chromium) wrote:

+
+  callback.Run(DiagnosticsSendPacketFunction::SEND_PACKET_FAILED, "", 0.0);
+}
+
+}  // namespace
+
+void DiagnosticsSendPacketFunction::AsyncWorkStart() {
+  std::map<std::string, std::string> config;
+  config[kCount] = kDefaultCount;
+  if (parameters_->options.ttl)
+    config[kTTL] = base::IntToString(*parameters_->options.ttl);
+  if (parameters_->options.timeout)
+    config[kTimeout] = base::IntToString(*parameters_->options.timeout);
+  if (parameters_->options.size)
+    config[kSize] = base::IntToString(*parameters_->options.size);
+
+  chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->
+      TestICMPWithOptions(
+          parameters_->options.ip, config,
+          base::Bind(
+              ParseResult,
+              base::Bind(&DiagnosticsSendPacketFunction::OnCompleted, this)));
+}
+
+}  // namespace extensions