PPAPI: Introduce PPB_NetworkProxy_Dev

ppapi/proxy/network_proxy_resource.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 "ppapi/proxy/network_proxy_resource.h"
+
+#include "base/bind.h"
+#include "ppapi/proxy/dispatch_reply_message.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/shared_impl/tracked_callback.h"
+#include "ppapi/shared_impl/var.h"
+
+namespace ppapi {
+namespace proxy {
+
+NetworkProxyResource::NetworkProxyResource(Connection connection,
+                                           PP_Instance instance)
+    : PluginResource(connection, instance) {
+  SendCreate(BROWSER, PpapiHostMsg_NetworkProxy_Create());
+}
+
+NetworkProxyResource::~NetworkProxyResource() {
+}
+
+thunk::PPB_NetworkProxy_API* NetworkProxyResource::AsPPB_NetworkProxy_API() {
+  return this;
+}
+
+int32_t NetworkProxyResource::GetProxyForURL(
+      PP_Instance /* instance */,
+      PP_Var url,
+      PP_Var* proxy_string,
+      scoped_refptr<TrackedCallback> callback) {
+  StringVar* string_url = StringVar::FromPPVar(url);
+  if (!string_url)
+    return PP_ERROR_BADARGUMENT;
+  Call<PpapiPluginMsg_NetworkProxy_GetProxyForURLReply>(
+      BROWSER,
+      PpapiHostMsg_NetworkProxy_GetProxyForURL(string_url->value()),
+      base::Bind(&NetworkProxyResource::OnPluginMsgGetProxyForURLReply,
+                 base::Unretained(this),
+                 base::Unretained(proxy_string),
+                 callback));
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void NetworkProxyResource::OnPluginMsgGetProxyForURLReply(
+    PP_Var* proxy_string_out_param,
+    scoped_refptr<TrackedCallback> callback,
+    const ResourceMessageReplyParams& params,
+    const std::string& proxy_string) {
+  DCHECK(TrackedCallback::IsPending(callback));

[yzshen1, 2013/06/18 04:41:26]

It seems better to me to still check whether it is pending and return if not 
(the original code).

Although I agree with you that DCHECK should be enough for now, it relies on a
few things implicitly:
- callbacks are cleared when the last plugin ref is gone.
- this is the only class that will ever try to run |callback|. (|callback| is
ref-counted, it is possible that someone else holds a ref to it and ran it
already.)

Besides, it is hard to hit this DCHECK if later we change code and some of those
implicit prerequisites are not true anymore. And we may not realize it and cause
memory corruption. 

What do you think?

[dmichael (off chromium), 2013/06/18 22:22:43]

(This is also true of using base::Unretained(this) above instead of letting the
callback refcount it)
If we do that, it's probably a bug.
I went for a compromise...  I put the old code back in, so that it will behave
reasonably if the callback has already been run. But I stuck in NOTIMPLEMENTED
this time, since if we hit that, I think it's probably a bug.

[yzshen1, 2013/06/19 16:50:59]

There is a comment in plugin_resource.h about passing base::Unretained(), so it
is not implicit. :)
That sounds good to me.

+  if (params.result() == PP_OK) {
+    *proxy_string_out_param = (new StringVar(proxy_string))->GetPPVar();
+  }
+  callback->Run(params.result());
+}
+
+}  // namespace proxy
+}  // namespace ppapi