Convert URLLoader to the new proxy design

Implementation of URLLoader using PluginResource/ResourceHost.

This doesn't use the resource call/reply infrastructure, but rather pipes WebKit callbacks to the plugin via unsolicited callbacks. This eliminates state tracking in the host which makes things simpler.

[brettw, 2012-12-03 18:55:57]

Review hint: diff url_loader_resource.cc to patch set 1. Patch set 1 is exactly
the old ppb_url_loader_impl.cc file from webkit, so you can see what code I
changed from the old implementation (and don't need to re-review unchanged
code).

[bbudge, 2012-12-05 01:47:08]

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/c...
File content/renderer/pepper/content_renderer_pepper_host_factory.cc (right):

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/c...
content/renderer/pepper/content_renderer_pepper_host_factory.cc:72: host_,
false, instance, params.pp_resource()));
Should be in the public switch, right?

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
File content/renderer/pepper/pepper_plugin_delegate_impl.cc (right):

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
content/renderer/pepper/pepper_plugin_delegate_impl.cc:1501: loader_host = NULL;
This comment is confusing. Why null out this raw pointer as it's going out of
scope? Or do you want to prevent people from using it here?

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
File content/renderer/pepper/pepper_url_loader_host.cc (right):

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
content/renderer/pepper/pepper_url_loader_host.cc:223: int32_t
PepperURLLoaderHost::InternalOnHostMsgOpen(
Since this is private, and only called once, I don't understand why this is
needed.

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
File content/renderer/pepper/pepper_url_loader_host.h (right):

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
content/renderer/pepper/pepper_url_loader_host.h:128: // Messages send while the
resource host is pending. These will be forwarded
s/send/sent

https://codereview.chromium.org/11416064/diff/10104/ppapi/api/ppp_instance.idl
File ppapi/api/ppp_instance.idl (right):

https://codereview.chromium.org/11416064/diff/10104/ppapi/api/ppp_instance.id...
ppapi/api/ppp_instance.idl:214: * to all Native Client plugins currently), your
implementation of this
This is confusing. Most NaCl plugins don't handle MIME types.

https://codereview.chromium.org/11416064/diff/10104/ppapi/proxy/ppapi_messages.h
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/11416064/diff/10104/ppapi/proxy/ppapi_message...
ppapi/proxy/ppapi_messages.h:1559: // notification, we don't use the result
field of the response info.
I don't know what 'result field' is. Do you mean status_code?

https://codereview.chromium.org/11416064/diff/10104/ppapi/proxy/ppapi_message...
ppapi/proxy/ppapi_messages.h:1565: // progress updates, and only the fields
requestsed by the plugin will be
sp. requested

https://codereview.chromium.org/11416064/diff/10104/ppapi/proxy/ppp_instance_...
File ppapi/proxy/ppp_instance_proxy.cc (right):

https://codereview.chromium.org/11416064/diff/10104/ppapi/proxy/ppp_instance_...
ppapi/proxy/ppp_instance_proxy.cc:90: // spedially.
sp. specially

https://codereview.chromium.org/11416064/diff/10104/ppapi/thunk/ppb_url_loade...
File ppapi/thunk/ppb_url_loader_thunk.cc (right):

https://codereview.chromium.org/11416064/diff/10104/ppapi/thunk/ppb_url_loade...
ppapi/thunk/ppb_url_loader_thunk.cc:129: NULL  // TODO(brettw) remove
SetStatusCallback from the heade.
sp. header

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/plug...
File webkit/plugins/ppapi/plugin_delegate.h (right):

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/plug...
webkit/plugins/ppapi/plugin_delegate.h:703: // Notifies the plugin that of the
document load. This should initiate the
s/that//

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:597: // If the load was not
abandoned, document_loader_ will now be set.
Could we DCHECK it?

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
File webkit/plugins/ppapi/ppapi_plugin_instance.h (right):

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
webkit/plugins/ppapi/ppapi_plugin_instance.h:345: // This pointer is non-owning,
so the loader uses set_document_loader to
s/uses/must use/

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
File webkit/plugins/ppapi/ppapi_webplugin_impl.h (right):

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
webkit/plugins/ppapi/ppapi_webplugin_impl.h:99:
//scoped_refptr<PPB_URLLoader_Impl> document_loader_;
Remove this?

[brettw, 2012-12-13 05:48:14]

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
File content/renderer/pepper/pepper_url_loader_host.cc (right):

https://codereview.chromium.org/11416064/diff/10104/content/renderer/pepper/p...
content/renderer/pepper/pepper_url_loader_host.cc:223: int32_t
PepperURLLoaderHost::InternalOnHostMsgOpen(
I added an extra comment. This was to make the error handling reasonable.

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

https://codereview.chromium.org/11416064/diff/10104/webkit/plugins/ppapi/ppap...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:597: // If the load was not
abandoned, document_loader_ will now be set.
No, I clarified this.

[brettw, 2012-12-14 05:30:32]

Because this issue got messed up, I'm moving to
https://codereview.chromium.org/11416363/