PPB_URLRequestInfo::AppendFileToBody using sync ipc

AppendFileToBody by blocking ipc

Another version of change to fix PPB_URLRequestInfo::AppendFileToBody using synchronous IPC.

BUG=90878
TEST=PPAPITest.TestURLLoader

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97140

[michaeln, 2011-08-15 18:12:11]

I definitely prefer the focused fix to the problem in this change as compared to
other possible approaches (like http://codereview.chromium.org/7564009/). This
change leverages the std file handling code more, but doesn't preclude doing
better BLOB or FILESYSTEM specific handling down the road.

Since darin and eric were on the review list for the first change, do you want
to get them a chance to look it it too?

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc
File net/base/upload_data_stream.cc (right):

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream....
net/base/upload_data_stream.cc:113: base::ThreadRestrictions::ScopedAllowIO
allow_io;
Was this just missing before or does something about this change actually not
conduct blocking IO on the IO thread where it didn't before?

[kinuko, 2011-08-16 04:00:53]

(+darin, ericu, brettw)

This is the other patch with alternative approach using sync IPC for
PPB_URLRequestInfo::AppendFileToBody.   Darin, eric, wdyt about making sync IPC
when we add File to the url loader?  Could you take a look at this one (this
patch is much smaller than the other one)?  Thanks,

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc
File net/base/upload_data_stream.cc (right):

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream....
net/base/upload_data_stream.cc:113: base::ThreadRestrictions::ScopedAllowIO
allow_io;
On 2011/08/15 18:12:11, michaeln wrote:
> Was this just missing before or does something about this change actually not
> conduct blocking IO on the IO thread where it didn't before?

In my testing whenever I try uploading a file with or without this change the
browser (debug build) crashes here, so I assume this was missing before.

[darin (slow to review), 2011-08-16 04:09:35]

This is an alternative to using WebHTTPBody::appendURLRange?  Can you say
more about why you were motivated to build this alternative?

-Darin


On Mon, Aug 15, 2011 at 9:00 PM, <kinuko@chromium.org> wrote:

> Reviewers: michaeln, darin, ericu,
>
> Message:
> (+darin, ericu, brettw)
>
> This is the other patch with alternative approach using sync IPC for
> PPB_URLRequestInfo::**AppendFileToBody.   Darin, eric, wdyt about making
> sync IPC
> when we add File to the url loader?  Could you take a look at this one
> (this
> patch is much smaller than the other one)?  Thanks,
>
>
> http://codereview.chromium.**org/7618039/diff/2001/net/**
>
base/upload_data_stream.cc<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc>
> File net/base/upload_data_stream.cc (right):
>
> http://codereview.chromium.**org/7618039/diff/2001/net/**
>
base/upload_data_stream.cc#**newcode113<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc#newcode113>
> net/base/upload_data_stream.**cc:113:
> base::ThreadRestrictions::**ScopedAllowIO allow_io;
> On 2011/08/15 18:12:11, michaeln wrote:
>
>> Was this just missing before or does something about this change
>>
> actually not
>
>> conduct blocking IO on the IO thread where it didn't before?
>>
>
> In my testing whenever I try uploading a file with or without this
> change the browser (debug build) crashes here, so I assume this was
> missing before.
>
> Description:
> AppendFileToBody by blocking ipc
>
> Another version of change to fix PPB_URLRequestInfo::**AppendFileToBody
> using
> synchronous IPC.
>
> BUG=90878
> TEST=PPAPITest.TestURLLoader
>
>
> Please review this at
http://codereview.chromium.**org/7618039/<http://codereview.chromium.org/7618...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M content/browser/file_system/**file_system_dispatcher_host.h
>  M content/browser/file_system/**file_system_dispatcher_host.cc
>  M content/common/file_system/**file_system_dispatcher.h
>  M content/common/file_system_**messages.h
>  M content/renderer/pepper_**plugin_delegate_impl.h
>  M content/renderer/pepper_**plugin_delegate_impl.cc
>  M net/base/upload_data_stream.cc
>  M ppapi/tests/test_url_loader.h
>  M ppapi/tests/test_url_loader.cc
>  M webkit/plugins/ppapi/mock_**plugin_delegate.h
>  M webkit/plugins/ppapi/mock_**plugin_delegate.cc
>  M webkit/plugins/ppapi/plugin_**delegate.h
>  M webkit/plugins/ppapi/ppb_file_**ref_impl.h
>  M webkit/plugins/ppapi/ppb_file_**system_impl.h
>  M webkit/plugins/ppapi/ppb_url_**request_info_impl.cc
>
>
>

[kinuko, 2011-08-16 05:05:14]

On 2011/08/16 04:09:35, darin wrote:
> This is an alternative to using WebHTTPBody::appendURLRange?  Can you say
> more about why you were motivated to build this alternative?

I wasn't particularly motivated to make a sync IPC to fix this but this approach
seems to have some benefit: compact and focused solution, while giving us more
time for introducing better blob/file handlings.

Probably michael's comment above says more about this approach:
> I definitely prefer the focused fix to the problem in this change as compared
to
> other possible approaches (like http://codereview.chromium.org/7564009/). This
> change leverages the std file handling code more, but doesn't preclude doing
> better BLOB or FILESYSTEM specific handling down the road.


> -Darin
> 
> 
> On Mon, Aug 15, 2011 at 9:00 PM, <mailto:kinuko@chromium.org> wrote:
> 
> > Reviewers: michaeln, darin, ericu,
> >
> > Message:
> > (+darin, ericu, brettw)
> >
> > This is the other patch with alternative approach using sync IPC for
> > PPB_URLRequestInfo::**AppendFileToBody.   Darin, eric, wdyt about making
> > sync IPC
> > when we add File to the url loader?  Could you take a look at this one
> > (this
> > patch is much smaller than the other one)?  Thanks,
> >
> >
> > http://codereview.chromium.**org/7618039/diff/2001/net/**
> >
>
base/upload_data_stream.cc<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc>
> > File net/base/upload_data_stream.cc (right):
> >
> > http://codereview.chromium.**org/7618039/diff/2001/net/**
> >
>
base/upload_data_stream.cc#**newcode113<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc#newcode113>
> > net/base/upload_data_stream.**cc:113:
> > base::ThreadRestrictions::**ScopedAllowIO allow_io;
> > On 2011/08/15 18:12:11, michaeln wrote:
> >
> >> Was this just missing before or does something about this change
> >>
> > actually not
> >
> >> conduct blocking IO on the IO thread where it didn't before?
> >>
> >
> > In my testing whenever I try uploading a file with or without this
> > change the browser (debug build) crashes here, so I assume this was
> > missing before.
> >
> > Description:
> > AppendFileToBody by blocking ipc
> >
> > Another version of change to fix PPB_URLRequestInfo::**AppendFileToBody
> > using
> > synchronous IPC.
> >
> > BUG=90878
> > TEST=PPAPITest.TestURLLoader
> >
> >
> > Please review this at
>
http://codereview.chromium.**org/7618039/%3Chttp://codereview.chromium.org/76...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
> >
> > Affected files:
> >  M content/browser/file_system/**file_system_dispatcher_host.h
> >  M content/browser/file_system/**file_system_dispatcher_host.cc
> >  M content/common/file_system/**file_system_dispatcher.h
> >  M content/common/file_system_**messages.h
> >  M content/renderer/pepper_**plugin_delegate_impl.h
> >  M content/renderer/pepper_**plugin_delegate_impl.cc
> >  M net/base/upload_data_stream.cc
> >  M ppapi/tests/test_url_loader.h
> >  M ppapi/tests/test_url_loader.cc
> >  M webkit/plugins/ppapi/mock_**plugin_delegate.h
> >  M webkit/plugins/ppapi/mock_**plugin_delegate.cc
> >  M webkit/plugins/ppapi/plugin_**delegate.h
> >  M webkit/plugins/ppapi/ppb_file_**ref_impl.h
> >  M webkit/plugins/ppapi/ppb_file_**system_impl.h
> >  M webkit/plugins/ppapi/ppb_url_**request_info_impl.cc
> >
> >
> >

[darin (slow to review), 2011-08-16 05:25:36]

On Mon, Aug 15, 2011 at 10:05 PM, <kinuko@chromium.org> wrote:

> On 2011/08/16 04:09:35, darin wrote:
>
>> This is an alternative to using WebHTTPBody::appendURLRange?  Can you say
>> more about why you were motivated to build this alternative?
>>
>
> I wasn't particularly motivated to make a sync IPC to fix this but this
> approach
> seems to have some benefit: compact and focused solution, while giving us
> more
> time for introducing better blob/file handlings.
>

OK, so your goal is to do a stop-gap patch, and then work on the
alternative?  Is
the idea to back port this to M14?

-Darin



>
> Probably michael's comment above says more about this approach:
>
>> I definitely prefer the focused fix to the problem in this change as
>> compared
>>
> to
>
>> other possible approaches (like
http://codereview.chromium.**org/7564009/<http://codereview.chromium.org/7564...).
>> This
>> change leverages the std file handling code more, but doesn't preclude
>> doing
>> better BLOB or FILESYSTEM specific handling down the road.
>>
>
>
>  -Darin
>>
>
>
>  On Mon, Aug 15, 2011 at 9:00 PM, <mailto:kinuko@chromium.org> wrote:
>>
>
>  > Reviewers: michaeln, darin, ericu,
>> >
>> > Message:
>> > (+darin, ericu, brettw)
>> >
>> > This is the other patch with alternative approach using sync IPC for
>> > PPB_URLRequestInfo::****AppendFileToBody.   Darin, eric, wdyt about
>> making
>> > sync IPC
>> > when we add File to the url loader?  Could you take a look at this one
>> > (this
>> > patch is much smaller than the other one)?  Thanks,
>> >
>> >
>> > http://codereview.chromium.****org/7618039/diff/2001/net/**
>> >
>>
>
> base/upload_data_stream.cc<htt**p://codereview.chromium.org/**
>
7618039/diff/2001/net/base/**upload_data_stream.cc<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc>
> >
>
>  > File net/base/upload_data_stream.cc (right):
>> >
>> > http://codereview.chromium.****org/7618039/diff/2001/net/**
>> >
>>
>
> base/upload_data_stream.cc#****newcode113<http://codereview.**
> chromium.org/7618039/diff/**2001/net/base/upload_data_**
>
stream.cc#newcode113<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc#newcode113>
> >
>
>  > net/base/upload_data_stream.****cc:113:
>> > base::ThreadRestrictions::****ScopedAllowIO allow_io;
>> > On 2011/08/15 18:12:11, michaeln wrote:
>> >
>> >> Was this just missing before or does something about this change
>> >>
>> > actually not
>> >
>> >> conduct blocking IO on the IO thread where it didn't before?
>> >>
>> >
>> > In my testing whenever I try uploading a file with or without this
>> > change the browser (debug build) crashes here, so I assume this was
>> > missing before.
>> >
>> > Description:
>> > AppendFileToBody by blocking ipc
>> >
>> > Another version of change to fix PPB_URLRequestInfo::****
>> AppendFileToBody
>> > using
>> > synchronous IPC.
>> >
>> > BUG=90878
>> > TEST=PPAPITest.TestURLLoader
>> >
>> >
>> > Please review this at
>>
>
> http://codereview.chromium.****org/7618039/%3Chttp://coderevi**
> ew.chromium.org/7618039/ <http://codereview.chromium.org/7618039/>>
>
>> >
>> > SVN Base:
>>
>
>
svn://svn.chromium.org/chrome/****trunk/src<http://svn.chromium.org/chrome/**trunk/src>
>
<http://svn.**chromium.org/chrome/trunk/src<http://svn.chromium.org/chrome/tru...
> >
>
>  >
>> > Affected files:
>> >  M content/browser/file_system/****file_system_dispatcher_host.h
>> >  M content/browser/file_system/****file_system_dispatcher_host.cc
>> >  M content/common/file_system/****file_system_dispatcher.h
>> >  M content/common/file_system_****messages.h
>> >  M content/renderer/pepper_****plugin_delegate_impl.h
>> >  M content/renderer/pepper_****plugin_delegate_impl.cc
>> >  M net/base/upload_data_stream.cc
>> >  M ppapi/tests/test_url_loader.h
>> >  M ppapi/tests/test_url_loader.cc
>> >  M webkit/plugins/ppapi/mock_****plugin_delegate.h
>> >  M webkit/plugins/ppapi/mock_****plugin_delegate.cc
>> >  M webkit/plugins/ppapi/plugin_****delegate.h
>> >  M webkit/plugins/ppapi/ppb_file_****ref_impl.h
>> >  M webkit/plugins/ppapi/ppb_file_****system_impl.h
>> >  M webkit/plugins/ppapi/ppb_url_****request_info_impl.cc
>> >
>> >
>> >
>>
>
>
>
>
http://codereview.chromium.**org/7618039/<http://codereview.chromium.org/7618...
>

[kinuko, 2011-08-16 11:43:05]

On 2011/08/16 05:25:36, darin wrote:
> On Mon, Aug 15, 2011 at 10:05 PM,
<https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=kinuko@chromium.org> wrote:
> 
> > On 2011/08/16 04:09:35, darin wrote:
> >
> >> This is an alternative to using WebHTTPBody::appendURLRange?  Can you say
> >> more about why you were motivated to build this alternative?
> >>
> >
> > I wasn't particularly motivated to make a sync IPC to fix this but this
> > approach
> > seems to have some benefit: compact and focused solution, while giving us
> > more
> > time for introducing better blob/file handlings.
> >
> 
> OK, so your goal is to do a stop-gap patch, and then work on the
> alternative?  Is
> the idea to back port this to M14?

Yes that's the idea.  While the other patch still has some rough/ad-hoc part
this one feels a lot more concrete/easier to merge to M14.  Wdyt?  (We could
still continue to work on the original one if this doesn't sound good.)

> -Darin
> 
> 
> 
> >
> > Probably michael's comment above says more about this approach:
> >
> >> I definitely prefer the focused fix to the problem in this change as
> >> compared
> >>
> > to
> >
> >> other possible approaches (like
>
http://codereview.chromium.**org/7564009/%3Chttp://codereview.chromium.org/75...>).
> >> This
> >> change leverages the std file handling code more, but doesn't preclude
> >> doing
> >> better BLOB or FILESYSTEM specific handling down the road.
> >>
> >
> >
> >  -Darin
> >>
> >
> >
> >  On Mon, Aug 15, 2011 at 9:00 PM, <mailto:kinuko@chromium.org> wrote:
> >>
> >
> >  > Reviewers: michaeln, darin, ericu,
> >> >
> >> > Message:
> >> > (+darin, ericu, brettw)
> >> >
> >> > This is the other patch with alternative approach using sync IPC for
> >> > PPB_URLRequestInfo::****AppendFileToBody.   Darin, eric, wdyt about
> >> making
> >> > sync IPC
> >> > when we add File to the url loader?  Could you take a look at this one
> >> > (this
> >> > patch is much smaller than the other one)?  Thanks,
> >> >
> >> >
> >> > http://codereview.chromium.****org/7618039/diff/2001/net/**
> >> >
> >>
> >
> > base/upload_data_stream.cc<htt**p://codereview.chromium.org/**
> >
>
7618039/diff/2001/net/base/**upload_data_stream.cc<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc>
> > >
> >
> >  > File net/base/upload_data_stream.cc (right):
> >> >
> >> > http://codereview.chromium.****org/7618039/diff/2001/net/**
> >> >
> >>
> >
> > base/upload_data_stream.cc#****newcode113<http://codereview.**
> > chromium.org/7618039/diff/**2001/net/base/upload_data_**
> >
>
stream.cc#newcode113<http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc#newcode113>
> > >
> >
> >  > net/base/upload_data_stream.****cc:113:
> >> > base::ThreadRestrictions::****ScopedAllowIO allow_io;
> >> > On 2011/08/15 18:12:11, michaeln wrote:
> >> >
> >> >> Was this just missing before or does something about this change
> >> >>
> >> > actually not
> >> >
> >> >> conduct blocking IO on the IO thread where it didn't before?
> >> >>
> >> >
> >> > In my testing whenever I try uploading a file with or without this
> >> > change the browser (debug build) crashes here, so I assume this was
> >> > missing before.
> >> >
> >> > Description:
> >> > AppendFileToBody by blocking ipc
> >> >
> >> > Another version of change to fix PPB_URLRequestInfo::****
> >> AppendFileToBody
> >> > using
> >> > synchronous IPC.
> >> >
> >> > BUG=90878
> >> > TEST=PPAPITest.TestURLLoader
> >> >
> >> >
> >> > Please review this at
> >>
> >
> > http://codereview.chromium.****org/7618039/%253Chttp://coderevi**
> > ew.chromium.org/7618039/ <http://codereview.chromium.org/7618039/>>
> >
> >> >
> >> > SVN Base:
> >>
> >
> >
>
svn://svn.chromium.org/chrome/****trunk/src<http://svn.chromium.org/chrome/**trunk/src>
> >
>
<http://svn.**chromium.org/chrome/trunk/src%3Chttp://svn.chromium.org/chrome/t...>
> > >
> >
> >  >
> >> > Affected files:
> >> >  M content/browser/file_system/****file_system_dispatcher_host.h
> >> >  M content/browser/file_system/****file_system_dispatcher_host.cc
> >> >  M content/common/file_system/****file_system_dispatcher.h
> >> >  M content/common/file_system_****messages.h
> >> >  M content/renderer/pepper_****plugin_delegate_impl.h
> >> >  M content/renderer/pepper_****plugin_delegate_impl.cc
> >> >  M net/base/upload_data_stream.cc
> >> >  M ppapi/tests/test_url_loader.h
> >> >  M ppapi/tests/test_url_loader.cc
> >> >  M webkit/plugins/ppapi/mock_****plugin_delegate.h
> >> >  M webkit/plugins/ppapi/mock_****plugin_delegate.cc
> >> >  M webkit/plugins/ppapi/plugin_****delegate.h
> >> >  M webkit/plugins/ppapi/ppb_file_****ref_impl.h
> >> >  M webkit/plugins/ppapi/ppb_file_****system_impl.h
> >> >  M webkit/plugins/ppapi/ppb_url_****request_info_impl.cc
> >> >
> >> >
> >> >
> >>
> >
> >
> >
> >
>
http://codereview.chromium.**org/7618039/%3Chttp://codereview.chromium.org/76...>
> >

[darin (slow to review), 2011-08-16 17:28:36]

LGTM

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream.cc
File net/base/upload_data_stream.cc (right):

http://codereview.chromium.org/7618039/diff/2001/net/base/upload_data_stream....
net/base/upload_data_stream.cc:113: base::ThreadRestrictions::ScopedAllowIO
allow_io;
Yeah, I'd expect that this is needed.  I'm not sure why we didn't have it
before.

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader.cc
File ppapi/tests/test_url_loader.cc (right):

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader....
ppapi/tests/test_url_loader.cc:183: pp::FileSystem file_system(instance_,
PP_FILESYSTEMTYPE_LOCALTEMPORARY);
it looks like TestBasicFilePOST and TestBasicFileRangePOST share a ton of code. 
there should probably be a helper function for the shared code.

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/ppb_url...
File webkit/plugins/ppapi/ppb_url_request_info_impl.cc (right):

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/ppb_url...
webkit/plugins/ppapi/ppb_url_request_info_impl.cc:233:
instance()->delegate()->SyncGetFileSystemPlatformPath(
can you insert a TODO(kinuko) here about switching to the better solution to
avoid this IPC in the future?

[michaeln, 2011-08-17 00:16:24]

lgtm2

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader.cc
File ppapi/tests/test_url_loader.cc (right):

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader....
ppapi/tests/test_url_loader.cc:87: int32_t WriteEntireBuffer(PP_Instance
instance,
should this be static or in an anon namespace or maybe another private method?

[ericu, 2011-08-17 01:14:31]

LGTM

http://codereview.chromium.org/7618039/diff/2001/content/browser/file_system/...
File content/browser/file_system/file_system_dispatcher_host.h (right):

http://codereview.chromium.org/7618039/diff/2001/content/browser/file_system/...
content/browser/file_system/file_system_dispatcher_host.h:50: BrowserThread::ID*
thread);
How about an OVERRIDE tag here?

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader.cc
File ppapi/tests/test_url_loader.cc (right):

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader....
ppapi/tests/test_url_loader.cc:183: pp::FileSystem file_system(instance_,
PP_FILESYSTEMTYPE_LOCALTEMPORARY);
On 2011/08/16 17:28:36, darin wrote:
> it looks like TestBasicFilePOST and TestBasicFileRangePOST share a ton of
code. 
> there should probably be a helper function for the shared code.

+1

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/mock_pl...
File webkit/plugins/ppapi/mock_plugin_delegate.cc (right):

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/mock_pl...
webkit/plugins/ppapi/mock_plugin_delegate.cc:193: FilePath* platform_path) {
Perhaps this should clear platform_path, as the real implementation does on
error?

[kinuko, 2011-08-17 07:55:28]

Thanks, addressed all the comments.  Will be submitting and then merging.

http://codereview.chromium.org/7618039/diff/2001/content/browser/file_system/...
File content/browser/file_system/file_system_dispatcher_host.h (right):

http://codereview.chromium.org/7618039/diff/2001/content/browser/file_system/...
content/browser/file_system/file_system_dispatcher_host.h:50: BrowserThread::ID*
thread);
On 2011/08/17 01:14:31, ericu wrote:
> How about an OVERRIDE tag here?

Done.

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader.cc
File ppapi/tests/test_url_loader.cc (right):

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader....
ppapi/tests/test_url_loader.cc:87: int32_t WriteEntireBuffer(PP_Instance
instance,
On 2011/08/17 00:16:24, michaeln wrote:
> should this be static or in an anon namespace or maybe another private method?

Put this in an anon namespace and placed at top over other TestURLLoader
methods.

http://codereview.chromium.org/7618039/diff/2001/ppapi/tests/test_url_loader....
ppapi/tests/test_url_loader.cc:183: pp::FileSystem file_system(instance_,
PP_FILESYSTEMTYPE_LOCALTEMPORARY);
On 2011/08/17 01:14:31, ericu wrote:
> On 2011/08/16 17:28:36, darin wrote:
> > it looks like TestBasicFilePOST and TestBasicFileRangePOST share a ton of
> code. 
> > there should probably be a helper function for the shared code.
> 
> +1

Done.

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/mock_pl...
File webkit/plugins/ppapi/mock_plugin_delegate.cc (right):

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/mock_pl...
webkit/plugins/ppapi/mock_plugin_delegate.cc:193: FilePath* platform_path) {
On 2011/08/17 01:14:31, ericu wrote:
> Perhaps this should clear platform_path, as the real implementation does on
> error?

Done.

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/ppb_url...
File webkit/plugins/ppapi/ppb_url_request_info_impl.cc (right):

http://codereview.chromium.org/7618039/diff/2001/webkit/plugins/ppapi/ppb_url...
webkit/plugins/ppapi/ppb_url_request_info_impl.cc:233:
instance()->delegate()->SyncGetFileSystemPlatformPath(
On 2011/08/16 17:28:36, darin wrote:
> can you insert a TODO(kinuko) here about switching to the better solution to
> avoid this IPC in the future?

Done.