Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(30)

Issue 11416363: Implementation of URLLoader using PluginResource/ResourceHost. (Closed)

Created:
8 years ago by brettw
Modified:
7 years, 2 months ago
Reviewers:
bbudge
CC:
chromium-reviews, yusukes+watch_chromium.org, jam, yzshen+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, raymes+watch_chromium.org, piman+watch_chromium.org, ihf+watch_chromium.org
Visibility:
Public.

Description

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. This fixes some bugs in Close() as well to fix the below-mentioned bug. BUG=69457

Patch Set 1 : #

Patch Set 2 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+1342 lines, -1633 lines) Patch
M content/content_renderer.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/renderer/renderer_ppapi_host.h View 2 chunks +5 lines, -0 lines 0 comments Download
M content/renderer/pepper/content_renderer_pepper_host_factory.cc View 1 2 chunks +4 lines, -0 lines 0 comments Download
M content/renderer/pepper/pepper_in_process_resource_creation.h View 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/pepper/pepper_in_process_resource_creation.cc View 2 chunks +8 lines, -0 lines 0 comments Download
M content/renderer/pepper/pepper_plugin_delegate_impl.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/renderer/pepper/pepper_plugin_delegate_impl.cc View 1 4 chunks +66 lines, -0 lines 0 comments Download
A content/renderer/pepper/pepper_url_loader_host.h View 1 1 chunk +138 lines, -0 lines 0 comments Download
A content/renderer/pepper/pepper_url_loader_host.cc View 1 1 chunk +380 lines, -0 lines 0 comments Download
M content/renderer/pepper/renderer_ppapi_host_impl.h View 2 chunks +4 lines, -1 line 0 comments Download
M content/renderer/pepper/renderer_ppapi_host_impl.cc View 2 chunks +6 lines, -1 line 0 comments Download
M ppapi/api/ppp_instance.idl View 1 2 chunks +6 lines, -9 lines 1 comment Download
M ppapi/c/ppp_instance.h View 1 3 chunks +7 lines, -10 lines 0 comments Download
M ppapi/c/private/ppb_proxy_private.h View 1 2 chunks +1 line, -6 lines 0 comments Download
M ppapi/ppapi_proxy.gypi View 2 chunks +2 lines, -2 lines 0 comments Download
M ppapi/proxy/interface_list.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M ppapi/proxy/plugin_resource.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M ppapi/proxy/ppapi_messages.h View 1 2 chunks +60 lines, -4 lines 0 comments Download
D ppapi/proxy/ppb_url_loader_proxy.h View 1 chunk +0 lines, -92 lines 0 comments Download
D ppapi/proxy/ppb_url_loader_proxy.cc View 1 chunk +0 lines, -649 lines 0 comments Download
M ppapi/proxy/ppp_instance_proxy.h View 2 chunks +3 lines, -2 lines 0 comments Download
M ppapi/proxy/ppp_instance_proxy.cc View 1 3 chunks +28 lines, -45 lines 0 comments Download
M ppapi/proxy/resource_creation_proxy.h View 1 chunk +2 lines, -0 lines 1 comment Download
M ppapi/proxy/resource_creation_proxy.cc View 3 chunks +7 lines, -2 lines 0 comments Download
A ppapi/proxy/url_loader_resource.h View 1 1 chunk +141 lines, -0 lines 1 comment Download
A ppapi/proxy/url_loader_resource.cc View 1 1 chunk +381 lines, -0 lines 0 comments Download
M ppapi/proxy/url_response_info_resource.h View 1 chunk +2 lines, -0 lines 0 comments Download
M ppapi/shared_impl/api_id.h View 1 chunk +0 lines, -1 line 0 comments Download
M ppapi/thunk/interfaces_ppb_private.h View 1 chunk +2 lines, -0 lines 0 comments Download
M ppapi/thunk/interfaces_ppb_public_stable.h View 2 chunks +1 line, -2 lines 0 comments Download
M ppapi/thunk/ppb_url_loader_api.h View 1 chunk +0 lines, -9 lines 0 comments Download
M ppapi/thunk/ppb_url_loader_thunk.cc View 1 2 chunks +1 line, -8 lines 0 comments Download
M ppapi/thunk/resource_creation_api.h View 2 chunks +9 lines, -0 lines 0 comments Download
M ppapi/thunk/thunk.h View 2 chunks +0 lines, -3 lines 0 comments Download
M webkit/glue/webkit_glue.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M webkit/plugins/ppapi/plugin_delegate.h View 1 2 chunks +11 lines, -0 lines 1 comment Download
M webkit/plugins/ppapi/plugin_module.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M webkit/plugins/ppapi/ppapi_plugin_instance.h View 1 4 chunks +19 lines, -5 lines 0 comments Download
M webkit/plugins/ppapi/ppapi_plugin_instance.cc View 1 5 chunks +17 lines, -12 lines 0 comments Download
M webkit/plugins/ppapi/ppapi_webplugin_impl.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M webkit/plugins/ppapi/ppapi_webplugin_impl.cc View 2 chunks +12 lines, -24 lines 0 comments Download
M webkit/plugins/ppapi/ppb_proxy_impl.cc View 1 3 chunks +0 lines, -9 lines 0 comments Download
D webkit/plugins/ppapi/ppb_url_loader_impl.h View 1 chunk +0 lines, -176 lines 0 comments Download
D webkit/plugins/ppapi/ppb_url_loader_impl.cc View 1 chunk +0 lines, -547 lines 0 comments Download
M webkit/plugins/ppapi/resource_creation_impl.h View 2 chunks +2 lines, -1 line 0 comments Download
M webkit/plugins/ppapi/resource_creation_impl.cc View 3 chunks +7 lines, -5 lines 0 comments Download
M webkit/plugins/ppapi/url_response_info_util.h View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
brettw
Moved from https://codereview.chromium.org/11416064/ Patch set 1 is the one from the previous issue, patch set ...
8 years ago (2012-12-14 05:33:46 UTC) #1
bbudge
8 years ago (2012-12-17 21:13:14 UTC) #2
LGTM just nits really

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

https://codereview.chromium.org/11416363/diff/7001/ppapi/api/ppp_instance.idl...
ppapi/api/ppp_instance.idl:214: * plugins can't register for MIME type
handling), your implementation of
Is this true? I thought an extension could register a NaCl plugin as a MIME type
handler.

https://codereview.chromium.org/11416363/diff/7001/ppapi/proxy/resource_creat...
File ppapi/proxy/resource_creation_proxy.h (right):

https://codereview.chromium.org/11416363/diff/7001/ppapi/proxy/resource_creat...
ppapi/proxy/resource_creation_proxy.h:44: const PPB_FileRef_CreateInfo&
serialized) OVERRIDE;
'serialized' seems like an odd name in the header. Makes more sense in the .cc
file. How about 'create_info'?

https://codereview.chromium.org/11416363/diff/7001/ppapi/proxy/url_loader_res...
File ppapi/proxy/url_loader_resource.h (right):

https://codereview.chromium.org/11416363/diff/7001/ppapi/proxy/url_loader_res...
ppapi/proxy/url_loader_resource.h:32: // response data is the response for the
already-opened connection.
Does a 'Connection' have a response? I thought a connection was just a pair of
IPC senders.

https://codereview.chromium.org/11416363/diff/7001/webkit/plugins/ppapi/plugi...
File webkit/plugins/ppapi/plugin_delegate.h (right):

https://codereview.chromium.org/11416363/diff/7001/webkit/plugins/ppapi/plugi...
webkit/plugins/ppapi/plugin_delegate.h:681: // The loader object should set
itself on the PluginInstance as the
Maybe clearer if you say the delegate should set the document loader on the
PluginInstance?

Powered by Google App Engine
This is Rietveld 408576698