PPAPI URLRequestInfo test: port NaCl version to ppapi_tests. Fix Var and Resource leaks.

ppapi/tests/test_url_request.h

Index: ppapi/tests/test_url_request.h
===================================================================
--- ppapi/tests/test_url_request.h	(revision 0)
+++ ppapi/tests/test_url_request.h	(revision 0)
@@ -0,0 +1,62 @@
+// 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.
+
+#ifndef PAPPI_TESTS_TEST_URL_REQUEST_H_
+#define PAPPI_TESTS_TEST_URL_REQUEST_H_
+
+#include <string>
+
+#include "ppapi/c/ppb_core.h"
+#include "ppapi/c/ppb_url_loader.h"
+#include "ppapi/c/ppb_url_request_info.h"
+#include "ppapi/c/ppb_url_response_info.h"
+#include "ppapi/c/ppb_var.h"
+#include "ppapi/tests/test_case.h"
+
+class TestURLRequest : public TestCase {
+ public:
+  explicit TestURLRequest(TestingInstance* instance);
+  ~TestURLRequest() { ppb_core_interface_->ReleaseResource(url_loader_); }
+
+  // TestCase implementation.
+  virtual bool Init();
+  virtual void RunTests(const std::string& filter);
+
+ private:
+  std::string TestCreate();
+  std::string TestIsURLRequestInfo();
+  std::string TestSetProperty();
+  std::string TestAppendDataToBody();
+  std::string TestStress();
+
+  // Helpers.
+  PP_Var PP_MakeString(const char* s);
+  std::string LoadAndCompareBody(PP_Resource url_request,
+                                 const std::string& expected_body);
+
+  struct PropertyTestData {
+    PropertyTestData(TestURLRequest* tester,
+                     PP_URLRequestProperty _property,
+                     const std::string& _property_name,
+                     PP_Var _var, PP_Bool _expected_value) :

[dmichael (off chromium), 2012/01/17 18:38:03]

Please don't use leading underscores in names. It's technically okay here, but
some kinds of names with leading underscores are reserved for use by the
compiler implementation. I think it's best just to avoid leading underscores
altogether.

[polina1, 2012/01/26 02:56:26]

This was a port of the NaCl code. I am not too keen on these myself, so removed.

On 2012/01/17 18:38:03, dmichael wrote:

+        property(_property), property_name(_property_name),
+        var(_var), expected_value(_expected_value) {
+      // _var has ref count of 1 on creation.
+    }
+    PP_URLRequestProperty property;
+    std::string property_name;
+    PP_Var var;

[dmichael (off chromium), 2012/01/17 18:38:03]

You *might* need your destructor to release this var. If it's a string, you
might leak it, and the test framework should complain and say the test failed.

The ref-counting semantics aren't obvious for PPB_URL It looks like since you
always pass the string immediately to a function on PPB_URLRequestInfo, but from
glancing at the code, it looks like it does not take ownership, so I think
you're responsible for releasing the parameter you send in :-/

[polina1, 2012/01/26 02:56:26]

There are no leaks because the var is released at the end of TestSetProperty.

The leaks are actually reported by the test framework. There used to be leaks in
the NaCl code because an additional var increment was done in the constructor. I
removed it and added a comment instead.

On 2012/01/17 18:38:03, dmichael wrote:

+    PP_Bool expected_value;
+  };

[bbudge, 2012/01/16 16:24:29]

struct could be hidden in the .cc file.

[polina1, 2012/01/26 02:56:26]

done
moved to the same spot where it was in the NaCl code to minimize the diff

+
+  const PPB_URLRequestInfo* ppb_url_request_interface_;
+  const PPB_URLLoader* ppb_url_loader_interface_;
+  const PPB_URLResponseInfo* ppb_url_response_interface_;
+  const PPB_Core* ppb_core_interface_;
+  const PPB_Var* ppb_var_interface_;
+
+  PP_Resource url_loader_;
+};
+
+#endif  // PAPPI_TESTS_TEST_URL_REQUEST_H_