Connectivity Diagnostics API: chrome.diagnostics.sendPacket

chrome/browser/extensions/api/ping_private/ping_private_api.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 "ping_private_api.h"
+
+#include "base/values.h"
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/packet_sender.h"
+#endif  // defined(OS_CHROMEOS)
+
+namespace SendPacket = extensions::api::ping_private::SendPacket;
+
+namespace extensions {
+
+PingPrivateSendPacketFunction::PingPrivateSendPacketFunction() {}
+
+PingPrivateSendPacketFunction::~PingPrivateSendPacketFunction() {}
+
+bool PingPrivateSendPacketFunction::Prepare() {
+  parameters_ = SendPacket::Params::Create(*args_);
+  EXTENSION_FUNCTION_VALIDATE(parameters_.get());
+  return true;
+}
+
+void PingPrivateSendPacketFunction::AsyncWorkStart() {
+  std::string ip = parameters_->options.ip;

[xiaowenx, 2013/06/17 06:19:38]

const std::string& ip = parameters_->options.ip;

[Bei Zhang, 2013/06/17 07:30:54]

Done.

+  std::string type = "icmp";

[xiaowenx, 2013/06/17 05:26:41]

We should return error if the user gives any protocol that's not ICMP here.

[Bei Zhang, 2013/06/17 07:30:54]

Actually we should remove this option for now. We don't really other
implementation and it is pretty easy to extend.

On 2013/06/17 05:26:41, xiaowenx wrote:

+  long ttl = 1;

[xiaowenx, 2013/06/17 06:19:38]

The generated code for SendPacketOptions.ttl is of type int (scoped_ptr<int>
ttl), so this should be int also?

[Bei Zhang, 2013/06/17 07:30:54]

Nice catch! It definitely should.

+  long timeout = 5000;

[xiaowenx, 2013/06/17 07:19:31]

timeout is specified in seconds, so you probably want to divide this number by
1000.

[Bei Zhang, 2013/06/17 07:30:54]

If sub-second timeout could not be specified, then we should not allow user to
do it.
Stated in docs.

On 2013/06/17 07:19:31, xiaowenx wrote:

+  long size = 4;
+
+

[xiaowenx, 2013/06/17 06:19:38]

is the extra empty line needed?

[Bei Zhang, 2013/06/17 07:30:54]

Done.

+  if (parameters_->options.ttl.get())

[xiaowenx, 2013/06/17 06:19:38]

.get() isn't needed:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...

[Bei Zhang, 2013/06/17 07:30:54]

Hmm, I didn't know this. Yet I've never seen any code tests without the .get().
Maybe without the .get() it is not clear whether we are testing the pointer or
the value?

On 2013/06/17 06:19:38, xiaowenx wrote:

+    ttl = *parameters_->options.ttl;

[xiaowenx, 2013/06/17 06:19:38]

Maybe this kind of syntax would be easier: int ttl = parameters_->options.ttl ?
*parameters_->options.ttl : 1;

[Bei Zhang, 2013/06/17 07:30:54]

Done.

+  if (parameters_->options.timeout.get())
+    timeout = *parameters_->options.timeout;
+  if (parameters_->options.size.get())
+    size = *parameters_->options.size;
+
+#if defined(OS_CHROMEOS)
+  chromeos::PacketSender::GetInstance()->Send(ip, type, ttl, timeout, size,
+      base::Bind(&PingPrivateSendPacketFunction::OnComplete, this));
+#else
+  SetError("Not implemented");
+  AsyncWorkCompleted();
+#endif
+}
+
+bool PingPrivateSendPacketFunction::Respond() {
+  return error_.empty();
+}
+
+void PingPrivateSendPacketFunction::OnComplete(bool suceeded,
+                                               std::string ip, int latency) {
+  extensions::api::ping_private::SendPacketResult result;
+  result.ip = ip;
+  result.latency = latency;
+  SetResult(result.ToValue().release());
+  AsyncWorkCompleted();
+}
+
+}  // namespace extensions
+

[xiaowenx, 2013/06/17 06:19:38]

is the extra empty line needed?

[Bei Zhang, 2013/06/17 07:30:54]

Done.