Implement PPB_NetworkMonitor_Private Pepper interface.

ppapi/tests/test_network_monitor_private.cc

+// Copyright (c) 2012 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 "ppapi/tests/test_network_monitor_private.h"
+
+#include <string.h>
+
+#include "ppapi/cpp/private/network_list_private.h"
+#include "ppapi/cpp/private/network_monitor_private.h"
+#include "ppapi/tests/testing_instance.h"
+#include "ppapi/tests/test_utils.h"
+#include "ppapi/cpp/module.h"
+
+REGISTER_TEST_CASE(NetworkMonitorPrivate);
+
+#define ASSERT_SUCCESS(error_message) \

[dmichael (off chromium), 2012/03/05 19:22:00]

Please comment to explain what "SUCCESS" means here (empty string) and/or what
you're using this for (calling a separate method that follows the PPAPI test
convention of returning a non-empty string on error). I almost suggested a name
like "ASSERT_STRING_NONEMPTY", but after seeing ASSERT_SUCCESS in context, I
understand why you chose this name.

[Sergey Ulanov, 2012/03/06 02:11:06]

That was actually copied from the TCPServerSocket tests. There is also
RUN_SUBTEST() in test_transport.cc that is almost the same as this one. Maybe we
should just move this macro to test_case.h? When I wrote test_transport.cc I
initially put RUN_SUBTEST() there, but then Brett advised against it because
that was the only test that needed that macro. 

[dmichael (off chromium), 2012/03/06 16:11:28]

Sure, we can do that since there are several uses. The tricky part will be
coming up with the right name. Maybe "ASSERT_SUBTEST_SUCCESS" would be
descriptive & short enough?

I think it makes sense to put that in to test_case.h in a separate CL, though.

[Sergey Ulanov, 2012/03/06 23:38:08]

Done. I sent you a separate CL for that change.

+  if (!(error_message).empty()) \
+    return (error_message);

[dmichael (off chromium), 2012/03/05 19:22:00]

Maybe worth putting in a "do {...} while (false)" to
(a) force consistent use of a semi-colon and
(b) disallow weird constructs like:
  if (p)
    ASSERT_SUCCESS(foo);
  else
    DoSomethingElse();
In this case, the else would actually apply to the "if
(!(error_message).empty())" in your macro.

Alternatively, you could get rid of the macro...  it doesn't save that much code
or greatly improve readability.

[Sergey Ulanov, 2012/03/06 02:11:06]

Done, but see my comment about RUN_SUBTEST() above.

+
+namespace {
+
+struct CallbackData {
+  explicit CallbackData(PP_Instance instance)
+      : call_counter(0),
+        completion_callback(instance),
+        list_resource(0) {
+  }
+  ~CallbackData() {
+    if (list_resource)
+      pp::Module::Get()->core()->ReleaseResource(list_resource);
+  }
+  int call_counter;
+  TestCompletionCallback completion_callback;
+  PP_Resource list_resource;
+};
+
+void TestCallback(void* user_data, PP_Resource network_list) {
+  CallbackData* data = static_cast<CallbackData*>(user_data);
+  data->call_counter++;
+
+  if (data->list_resource)
+    pp::Module::Get()->core()->ReleaseResource(data->list_resource);
+  data->list_resource = network_list;
+
+  // Invoke completion callback only for the first change notification.
+  if (data->call_counter == 1)
+    static_cast<pp::CompletionCallback>(data->completion_callback).Run(PP_OK);
+}
+
+}  // namespace
+
+TestNetworkMonitorPrivate::TestNetworkMonitorPrivate(TestingInstance* instance)
+    : TestCase(instance) {
+}
+
+bool TestNetworkMonitorPrivate::Init() {
+  if (!pp::NetworkMonitorPrivate::IsAvailable())
+    return false;
+
+  return true;
+}
+
+void TestNetworkMonitorPrivate::RunTests(const std::string& filter) {
+  RUN_TEST_FORCEASYNC_AND_NOT(Basic, filter);
+  RUN_TEST_FORCEASYNC_AND_NOT(2Monitors, filter);
+}
+
+std::string TestNetworkMonitorPrivate::VerifyNetworkList(
+    PP_Resource network_resource) {
+  pp::NetworkListPrivate network_list(network_resource);
+
+  // Verify that there is at least one interface

[dmichael (off chromium), 2012/03/05 19:22:00]

nit: might be clearer to say "network interface" or something, to distinguish
from PPAPI interfaces.

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+  size_t count = network_list.GetCount();
+  ASSERT_TRUE(count >= 1U);
+
+  // Iterate over all interface and verify their properties.

[dmichael (off chromium), 2012/03/05 19:22:00]

nit: interface->interfaces

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+  for (size_t iface = 0; iface < count; ++iface) {
+

[dmichael (off chromium), 2012/03/05 19:22:00]

style nit: extra carriage return

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+    // Verify that the first interface has at least one address.
+    std::vector<PP_NetAddress_Private> addresses;
+    network_list.GetIpAddresses(iface, &addresses);
+    ASSERT_TRUE(addresses.size() >= 1U);
+    // Verify that the addresses are valid.
+    for (size_t i = 0; i < addresses.size(); ++i) {
+      ASSERT_TRUE(addresses[i].size == 4 || addresses[i].size == 16);
+
+      size_t j;
+      for (j = 0; j < addresses[i].size; ++j) {
+        if (addresses[i].data[j] != 0)
+          break;
+      }
+      ASSERT_TRUE(j != addresses[i].size);

[dmichael (off chromium), 2012/03/05 19:22:00]

A quick comment above this loop saying "Make sure the address is non-zero" or
whatever might be nice.

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+    }
+
+    // Verify that each interface has a unique name and a display name.
+    ASSERT_TRUE(!network_list.GetName(iface).empty());
+    ASSERT_TRUE(!network_list.GetDisplayName(iface).empty());

[dmichael (off chromium), 2012/03/05 19:22:00]

why not use ASSERT_FALSE?

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+
+    PP_NetworkListType_Private type = network_list.GetType(iface);
+    ASSERT_TRUE(type >= PP_NETWORKLIST_UNKNOWN);
+    ASSERT_TRUE(type <= PP_NETWORKLIST_CELLULAR);
+
+    PP_NetworkListState_Private state = network_list.GetState(iface);
+    ASSERT_TRUE(state >= PP_NETWORKLIST_DOWN);
+    ASSERT_TRUE(state <= PP_NETWORKLIST_UP);
+  }
+
+  // Try to call GetIpAddresses() without C++ wrapper and verify that
+  // it always returns correct value.
+  const PPB_NetworkList_Private* interface =
+      static_cast<const PPB_NetworkList_Private*>(
+          pp::Module::Get()->GetBrowserInterface(
+              PPB_NETWORKLIST_PRIVATE_INTERFACE));
+  ASSERT_TRUE(interface);
+  std::vector<PP_NetAddress_Private> addresses;
+  network_list.GetIpAddresses(0, &addresses);
+  size_t address_count = addresses.size();
+  addresses.resize(addresses.size() + 3);
+  for (size_t i = 0; i < addresses.size(); ++i) {
+    const char kFillValue = 123;
+    memset(&addresses.front(), kFillValue,
+           addresses.size() * sizeof(PP_NetAddress_Private));
+    int result = interface->GetIpAddresses(network_resource, 0,
+                                           &addresses.front(), i);
+    ASSERT_EQ(result, static_cast<int>(address_count));
+
+    // Verify that nothing outside the buffer was touched.
+    for (char* pos = reinterpret_cast<char*>(&addresses[result]);
+         pos != reinterpret_cast<char*>(&*addresses.end()); ++pos) {

[dmichael (off chromium), 2012/03/05 19:22:00]

I think dereferencing end() is technically undefined (though it should be fine
in practice), so it might be better to do something like:
reinterpret_cast<char*>(&addresses[0] + addresses.size())

[Sergey Ulanov, 2012/03/06 02:11:06]

Done.

+      ASSERT_TRUE(*pos == kFillValue);
+    }
+  }
+
+  PASS();
+}
+
+std::string TestNetworkMonitorPrivate::TestBasic() {
+  CallbackData callback_data(instance_->pp_instance());
+
+  pp::NetworkMonitorPrivate network_monitor(
+      instance_, &TestCallback, reinterpret_cast<void*>(&callback_data));
+  ASSERT_EQ(callback_data.completion_callback.WaitForResult(), PP_OK);
+  ASSERT_EQ(callback_data.call_counter, 1);
+
+  std::string list_result = VerifyNetworkList(callback_data.list_resource);
+  ASSERT_SUCCESS(list_result);
+
+  PASS();
+}
+
+std::string TestNetworkMonitorPrivate::Test2Monitors() {
+  CallbackData callback_data(instance_->pp_instance());
+
+  pp::NetworkMonitorPrivate network_monitor(
+      instance_, &TestCallback, reinterpret_cast<void*>(&callback_data));
+  ASSERT_EQ(callback_data.completion_callback.WaitForResult(), PP_OK);
+  ASSERT_EQ(callback_data.call_counter, 1);
+
+  std::string list_result = VerifyNetworkList(callback_data.list_resource);
+  ASSERT_SUCCESS(list_result);
+
+  CallbackData callback_data_2(instance_->pp_instance());
+
+  pp::NetworkMonitorPrivate network_monitor_2(
+      instance_, &TestCallback, reinterpret_cast<void*>(&callback_data_2));
+  ASSERT_EQ(callback_data_2.completion_callback.WaitForResult(), PP_OK);
+  ASSERT_EQ(callback_data_2.call_counter, 1);
+
+  list_result = VerifyNetworkList(callback_data_2.list_resource);
+  ASSERT_SUCCESS(list_result);
+
+  PASS();
+}