PPAPI: Add new PPB_FileRef.MakeDirectory to support exclusive operation

PPAPI: Add new PPB_FileRef.MakeDirectory to support exclusive operation

Current PPB_FileRef.MakeDirectory returns PP_OK if a directory exists on
the given path. This makes it difficult to create POSIX compatible API on
top of PPAPI.

This change introduces new PPB_FileRef.MakeDirectory as dev channel API.
That makes a new directory according to the given PP_MakeDirectoryFlags
values. The flags provide exclusive operation option. If exclusive flag
is specified and a directory exists on the given path, the function fails
and returns PP_ERROR_FILEEXISTS.


BUG=314879
TEST=browser_tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243802

[nhiroki, 2013-12-19 08:55:02]

Hi Dave, can you review this?
yusukes@ and his team need this feature.

Thanks!

[dmichael (off chromium), 2013-12-19 17:05:34]

I'm not going to be super responsive today, so +teravest@chromium.org.

[teravest, 2013-12-19 17:18:19]

https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h
File ppapi/cpp/file_ref.h (right):

https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h#ne...
ppapi/cpp/file_ref.h:131: int32_t MakeDirectoryExclusive(PP_Bool make_ancestors,
It's sad that this looks different than
MakeDirectory/MakeDirectoryIncludingAncestors above.

What do you think of
MakeDirectoryExclusive/MakeDirectoryExclusiveIncludingAncestors?

It's a long name, but at least it would be consistent.

Otherwise, this should use "bool" instead of "PP_Bool".

[raymes, 2013-12-19 23:10:22]

https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:136: int32_t MakeDirectoryExclusive([in] PP_Resource
directory_ref,
Can we instead just add a PP_Bool to MakeDirectory() which specifies whether or
not an error should be thrown if the directory exists?

Also the comment on this function doesn't seem to explain it properly :p

https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h
File ppapi/cpp/file_ref.h (right):

https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h#ne...
ppapi/cpp/file_ref.h:131: int32_t MakeDirectoryExclusive(PP_Bool make_ancestors,
It seems a bit weird that we made a whole new function here instead of just
adding a bool to MakeDirectory =\ Not sure the reasons for that.

My preference would be to just have MakeDirectory(bool make ancestors, bool
exclusive, const CompletionCallback& cc); The existing functions can be
deprecated and just call through that one. What do you guys think?

[teravest, 2013-12-19 23:12:19]

On Thu, Dec 19, 2013 at 4:10 PM,  <raymes@chromium.org> wrote:
>
>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref.idl
> File ppapi/api/ppb_file_ref.idl (right):
>
>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref....
> ppapi/api/ppb_file_ref.idl:136: int32_t MakeDirectoryExclusive([in]
> PP_Resource directory_ref,
> Can we instead just add a PP_Bool to MakeDirectory() which specifies
> whether or not an error should be thrown if the directory exists?
>
> Also the comment on this function doesn't seem to explain it properly :p
>
>
> https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h
> File ppapi/cpp/file_ref.h (right):
>
>
https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h#ne...
> ppapi/cpp/file_ref.h:131: int32_t MakeDirectoryExclusive(PP_Bool
> make_ancestors,
> It seems a bit weird that we made a whole new function here instead of
> just adding a bool to MakeDirectory =\ Not sure the reasons for that.
>
> My preference would be to just have MakeDirectory(bool make ancestors,
> bool exclusive, const CompletionCallback& cc); The existing functions
> can be deprecated and just call through that one. What do you guys
> think?

That's fine by me. The resulting API would be better in that case, but
can we actually get rid of the deprecated things?

How does deprecating C++ methods work today?

>
> https://codereview.chromium.org/113363004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[raymes, 2013-12-19 23:14:48]

Also since this is changing the stable API it will have to go through API review
before committing.

[raymes, 2013-12-19 23:49:16]

I don't think there is a good way besides making a comment that it is
deprecated :(

On Fri, Dec 20, 2013 at 10:11 AM, Justin TerAvest <teravest@chromium.org>
wrote:
> On Thu, Dec 19, 2013 at 4:10 PM,  <raymes@chromium.org> wrote:
>>
>>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref.idl
>> File ppapi/api/ppb_file_ref.idl (right):
>>
>>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref....
>> ppapi/api/ppb_file_ref.idl:136: int32_t MakeDirectoryExclusive([in]
>> PP_Resource directory_ref,
>> Can we instead just add a PP_Bool to MakeDirectory() which specifies
>> whether or not an error should be thrown if the directory exists?
>>
>> Also the comment on this function doesn't seem to explain it properly :p
>>
>>
>> https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h
>> File ppapi/cpp/file_ref.h (right):
>>
>>
https://codereview.chromium.org/113363004/diff/110001/ppapi/cpp/file_ref.h#ne...
>> ppapi/cpp/file_ref.h:131: int32_t MakeDirectoryExclusive(PP_Bool
>> make_ancestors,
>> It seems a bit weird that we made a whole new function here instead of
>> just adding a bool to MakeDirectory =\ Not sure the reasons for that.
>>
>> My preference would be to just have MakeDirectory(bool make ancestors,
>> bool exclusive, const CompletionCallback& cc); The existing functions
>> can be deprecated and just call through that one. What do you guys
>> think?
>
> That's fine by me. The resulting API would be better in that case, but
> can we actually get rid of the deprecated things?
>
> How does deprecating C++ methods work today?
>
>>
>> https://codereview.chromium.org/113363004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[yzshen1, 2013-12-20 00:31:14]

On 2013/12/19 23:49:16, raymes wrote:
> I don't think there is a good way besides making a comment that it is
> deprecated :(

We could remove methods from C++ classes if we want. It is not part of the
binary interface between us and plugins. The only annoyance is that when a
plugin developer updates the SDK and recompiles his plugin, he may get compile
errors.

[nhiroki, 2013-12-20 01:54:20]

Thanks, guys! Added comments only.

On 2013/12/19 23:14:48, raymes wrote:
> Also since this is changing the stable API it will have to go through API
review
> before committing.

IIUC, API review process has recently changed. Is there any instructions of
that?

On 2013/12/20 00:31:14, yzshen1 wrote:
> On 2013/12/19 23:49:16, raymes wrote:
> > I don't think there is a good way besides making a comment that it is
> > deprecated :(
> 
> We could remove methods from C++ classes if we want. It is not part of the
> binary interface between us and plugins. The only annoyance is that when a
> plugin developer updates the SDK and recompiles his plugin, he may get compile
> errors.

Okay, if there is no objection, I'll merge existing C++ methods and new one into
MakeDirectory.

https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:136: int32_t MakeDirectoryExclusive([in] PP_Resource
directory_ref,
On 2013/12/19 23:10:23, raymes wrote:
> Can we instead just add a PP_Bool to MakeDirectory() which specifies whether
or
> not an error should be thrown if the directory exists?

Oh, I see. I assumed we have to add new function with new name when we want to
change an existing API in order to prevent plugins from getting broken, but now
I understand the way from other idl files.

Anyway, I'll fix this according to your comment.

[nhiroki, 2013-12-20 11:05:41]

Updated. Can you take another look?

[yzshen1, 2013-12-20 18:18:54]

(drive-by comments; I only looked at the IDL and cpp API)

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:14: M34 = 1.2
please consider [channel=dev] tag.

[yzshen1, 2013-12-20 18:23:56]

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:40: if (has_interface<PPB_FileRef_1_1>()) {
Please also check 1_2 first.

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:123: pp_resource(),
It seems incorrect to ignore "exclusive" silently.

Maybe we could consider:
(1) fail the call when exclusive==true, but we don't have interface 1.2
(2) make two versions of MakeDirectory(), one with |exclusive| parameter, one
without.

What do you think?

[raymes, 2013-12-22 23:07:36]

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:115: int32_t MakeDirectory([in] PP_Resource
directory_ref,
I think we probably want to annotate this with [deprecate=1.2]

[raymes, 2013-12-22 23:12:59]

On Fri, Dec 20, 2013 at 12:54 PM,  <nhiroki@chromium.org> wrote:
> Thanks, guys! Added comments only.
>
>
> On 2013/12/19 23:14:48, raymes wrote:
>>
>> Also since this is changing the stable API it will have to go through API
>
> review
>>
>> before committing.
>
>
> IIUC, API review process has recently changed. Is there any instructions of
> that?

No :( I started a thread to try to get some instructions up. Generally
we would need new APIs to be in Dev first, but since it's a small
change, it might be ok. Could you send an email to pepper-team@ with a
summary of the proposal and a link to the CL?

Thanks!

>
>
> On 2013/12/20 00:31:14, yzshen1 wrote:
>>
>> On 2013/12/19 23:49:16, raymes wrote:
>> > I don't think there is a good way besides making a comment that it is
>> > deprecated :(
>
>
>> We could remove methods from C++ classes if we want. It is not part of the
>> binary interface between us and plugins. The only annoyance is that when a
>> plugin developer updates the SDK and recompiles his plugin, he may get
>> compile
>> errors.
>
>
> Okay, if there is no objection, I'll merge existing C++ methods and new one
> into
> MakeDirectory.
>
>
>
>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref.idl
> File ppapi/api/ppb_file_ref.idl (right):
>
>
https://codereview.chromium.org/113363004/diff/110001/ppapi/api/ppb_file_ref....
> ppapi/api/ppb_file_ref.idl:136: int32_t MakeDirectoryExclusive([in]
> PP_Resource directory_ref,
> On 2013/12/19 23:10:23, raymes wrote:
>>
>> Can we instead just add a PP_Bool to MakeDirectory() which specifies
>
> whether or
>>
>> not an error should be thrown if the directory exists?
>
>
> Oh, I see. I assumed we have to add new function with new name when we
> want to change an existing API in order to prevent plugins from getting
> broken, but now I understand the way from other idl files.
>
> Anyway, I'll fix this according to your comment.
>
> https://codereview.chromium.org/113363004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nhiroki, 2014-01-06 13:24:21]

Updated. Can you take another look? Thanks!

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:14: M34 = 1.2
On 2013/12/20 18:18:55, yzshen1 wrote:
> please consider [channel=dev] tag.

Done.

https://codereview.chromium.org/113363004/diff/130001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:115: int32_t MakeDirectory([in] PP_Resource
directory_ref,
On 2013/12/22 23:07:37, raymes wrote:
> I think we probably want to annotate this with [deprecate=1.2]

Done.

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:40: if (has_interface<PPB_FileRef_1_1>()) {
On 2013/12/20 18:23:57, yzshen1 wrote:
> Please also check 1_2 first.

Done.

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:123: pp_resource(),
On 2013/12/20 18:23:57, yzshen1 wrote:
> It seems incorrect to ignore "exclusive" silently.
> 
> Maybe we could consider:
> (1) fail the call when exclusive==true, but we don't have interface 1.2
> (2) make two versions of MakeDirectory(), one with |exclusive| parameter, one
> without.
> 
> What do you think?

Hmm... each of them seems to have its merits and demerits.

(1) would be simple, but plugin developers might get confused the parameter
behavior. As for (2), we need to maintain both versions and that seems painful
to me...

I don't have strong opinions, but I'd somewhat prefer (1). WDYT?

[nhiroki, 2014-01-06 13:25:59]

On 2013/12/22 23:12:59, raymes wrote:
> On Fri, Dec 20, 2013 at 12:54 PM,  <mailto:nhiroki@chromium.org> wrote:
> > Thanks, guys! Added comments only.
> >
> >
> > On 2013/12/19 23:14:48, raymes wrote:
> >>
> >> Also since this is changing the stable API it will have to go through API
> >
> > review
> >>
> >> before committing.
> >
> >
> > IIUC, API review process has recently changed. Is there any instructions of
> > that?
> 
> No :( I started a thread to try to get some instructions up. Generally
> we would need new APIs to be in Dev first, but since it's a small
> change, it might be ok. Could you send an email to pepper-team@ with a
> summary of the proposal and a link to the CL?
> 
> Thanks!
> 

Thanks! I sent a review request to pepper-team@

[yzshen1, 2014-01-06 17:53:15]

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/130001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:123: pp_resource(),
On 2014/01/06 13:24:22, nhiroki wrote:
> On 2013/12/20 18:23:57, yzshen1 wrote:
> > It seems incorrect to ignore "exclusive" silently.
> > 
> > Maybe we could consider:
> > (1) fail the call when exclusive==true, but we don't have interface 1.2
> > (2) make two versions of MakeDirectory(), one with |exclusive| parameter,
one
> > without.
> > 
> > What do you think?
> 
> Hmm... each of them seems to have its merits and demerits.
> 
> (1) would be simple, but plugin developers might get confused the parameter
> behavior. As for (2), we need to maintain both versions and that seems painful
> to me...
> 
> I don't have strong opinions, but I'd somewhat prefer (1). WDYT?
I am fine with (1). Thanks!

WRT (2), I think it is also okay, because we have to maintain C API v1.0 and 1.1
forever anyway.

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:127: return cc.MayForce(PP_ERROR_BADARGUMENT);
Might be better to use PP_ERROR_NOTSUPPORTED

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:136: return cc.MayForce(PP_ERROR_BADARGUMENT);
PP_ERROR_NOTSUPPORTED?

https://codereview.chromium.org/113363004/diff/610001/ppapi/thunk/interfaces_...
File ppapi/thunk/interfaces_ppb_public_stable.h (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/thunk/interfaces_...
ppapi/thunk/interfaces_ppb_public_stable.h:50:
PROXIED_IFACE(PPB_FILEREF_INTERFACE_1_2, PPB_FileRef_1_2)
It should be in interfaces_ppb_public_dev_channel.h, right?

[dmichael (off chromium), 2014-01-06 18:08:09]

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:142: [in] PP_Bool make_ancestors,
At this point, might it make sense to move to a bit-field approach?

I'm a little concerned with having two consecutive parameters of the same type.
Especially since we're adding a new one (and adding it before the existing one).

A current plugin might have:
file_ref_if->MakeDirectory(dir_ref, PP_TRUE, callback);
When their compile breaks, they'll have to change to something like this to get
the same behavior:
file_ref_if->MakeDirectory(dir_ref, PP_FALSE, PP_TRUE, callback);
...but they could easily mess up and transpose the false/true. Same is true of
new code. It might be nicer if instead they changed to:

file_ref_if->MakeDirectory(dir_ref, PP_MAKE_DIRECTORY_WITH_ANCESTORS, callback);

It's maybe a shame that their C compiler might not warn them if they leave
PP_TRUE there. But as long as the "make_ancestors" bit is in the least
significant digit, it would actually work right (because PP_TRUE is fixed to 1).

Thoughts?

[yzshen1, 2014-01-06 18:44:10]

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:142: [in] PP_Bool make_ancestors,
I like your idea. Thanks!
On 2014/01/06 18:08:09, dmichael wrote:
> At this point, might it make sense to move to a bit-field approach?
> 
> I'm a little concerned with having two consecutive parameters of the same
type.
> Especially since we're adding a new one (and adding it before the existing
one).
> 
> A current plugin might have:
> file_ref_if->MakeDirectory(dir_ref, PP_TRUE, callback);
> When their compile breaks, they'll have to change to something like this to
get
> the same behavior:
> file_ref_if->MakeDirectory(dir_ref, PP_FALSE, PP_TRUE, callback);
> ...but they could easily mess up and transpose the false/true. Same is true of
> new code. It might be nicer if instead they changed to:
> 
> file_ref_if->MakeDirectory(dir_ref, PP_MAKE_DIRECTORY_WITH_ANCESTORS,
callback);
> 
> It's maybe a shame that their C compiler might not warn them if they leave
> PP_TRUE there. But as long as the "make_ancestors" bit is in the least
> significant digit, it would actually work right (because PP_TRUE is fixed to
1).
> 
> Thoughts?

[nhiroki, 2014-01-07 07:32:44]

Thanks! Updated.

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref.idl
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/api/ppb_file_ref....
ppapi/api/ppb_file_ref.idl:142: [in] PP_Bool make_ancestors,
On 2014/01/06 18:44:11, yzshen1 wrote:
> I like your idea. Thanks!
> On 2014/01/06 18:08:09, dmichael wrote:
> > At this point, might it make sense to move to a bit-field approach?
> > 
> > I'm a little concerned with having two consecutive parameters of the same
> type.
> > Especially since we're adding a new one (and adding it before the existing
> one).
> > 
> > A current plugin might have:
> > file_ref_if->MakeDirectory(dir_ref, PP_TRUE, callback);
> > When their compile breaks, they'll have to change to something like this to
> get
> > the same behavior:
> > file_ref_if->MakeDirectory(dir_ref, PP_FALSE, PP_TRUE, callback);
> > ...but they could easily mess up and transpose the false/true. Same is true
of
> > new code. It might be nicer if instead they changed to:
> > 
> > file_ref_if->MakeDirectory(dir_ref, PP_MAKE_DIRECTORY_WITH_ANCESTORS,
> callback);
> > 
> > It's maybe a shame that their C compiler might not warn them if they leave
> > PP_TRUE there. But as long as the "make_ancestors" bit is in the least
> > significant digit, it would actually work right (because PP_TRUE is fixed to
> 1).
> > 
> > Thoughts?
> 

Good idea! Addressed it in the latest patchset.

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:127: return cc.MayForce(PP_ERROR_BADARGUMENT);
On 2014/01/06 17:53:16, yzshen1 wrote:
> Might be better to use PP_ERROR_NOTSUPPORTED

Done.

https://codereview.chromium.org/113363004/diff/610001/ppapi/cpp/file_ref.cc#n...
ppapi/cpp/file_ref.cc:136: return cc.MayForce(PP_ERROR_BADARGUMENT);
On 2014/01/06 17:53:16, yzshen1 wrote:
> PP_ERROR_NOTSUPPORTED?

Done.

https://codereview.chromium.org/113363004/diff/610001/ppapi/thunk/interfaces_...
File ppapi/thunk/interfaces_ppb_public_stable.h (right):

https://codereview.chromium.org/113363004/diff/610001/ppapi/thunk/interfaces_...
ppapi/thunk/interfaces_ppb_public_stable.h:50:
PROXIED_IFACE(PPB_FILEREF_INTERFACE_1_2, PPB_FileRef_1_2)
On 2014/01/06 17:53:16, yzshen1 wrote:
> It should be in interfaces_ppb_public_dev_channel.h, right?

Done.

[nhiroki, 2014-01-07 09:41:25]

Hi, can I ask a question?

After moving to dev_channel interface (PatchSet 5),
OutOfProcessPPAPITest.FileRef still works fine with PPB_FileRef_1_2, but
PPAPITest.FileRef tries to use PPB_FileRef_1_1 and then fails. I'm not sure why
each of them uses different interface version. Does anyone have any idea about
this? Probably I'm missing something...

Thanks!

[yzshen1, 2014-01-07 17:59:22]

> After moving to dev_channel interface (PatchSet 5),
> OutOfProcessPPAPITest.FileRef still works fine with PPB_FileRef_1_2, but
> PPAPITest.FileRef tries to use PPB_FileRef_1_1 and then fails. I'm not sure
why
> each of them uses different interface version. Does anyone have any idea about
> this? Probably I'm missing something...

I think that is because we don't support dev-channel APIs in-process. I think
you could skip running the test in-process.
(Justin knows better.)


Thanks!

https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
File content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h
(right):

https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h:31:
int32_t make_directory_flags) OVERRIDE;
wrong indent.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:19: * behavior of
<code>FileRef::MakeDirectory()</code>.
Maybe we should use PPB_FileRef.MakeDirectory()?

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:21: [assert_size(4)]
FYI, we haven't made the change to remove all existing assert_size tags, but we
are planning to stop using it:
https://code.google.com/p/chromium/issues/detail?id=327421

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:29: * Requests that the FileRef::MakeDirectory() call
fails if the directory
PPB_FileRef.MakeDirectory()?

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
ppapi/cpp/file_ref.cc:123: if (make_directory_flags &
PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
I think it is better to test all the bits except
PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS are zero (and the
PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS bit may or may not be zero).

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
ppapi/cpp/file_ref.cc:131: if (make_directory_flags &
PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
ditto

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h
File ppapi/cpp/file_ref.h (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
ppapi/cpp/file_ref.h:98: /// @param[in] exclusive A <code>bool</code> set to
<code>true</code> to fail
Please remove this @param

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
ppapi/cpp/file_ref.h:101: /// <code>PP_MakeDirectoryFlags</code> values.  Valid
values are:
it seems unnecessary to list valid values here. readers can refer to the
definition of PP_MakeDirectoryFlags, right?

[teravest, 2014-01-07 18:15:01]

On Tue, Jan 7, 2014 at 10:59 AM,  <yzshen@chromium.org> wrote:
>> After moving to dev_channel interface (PatchSet 5),
>> OutOfProcessPPAPITest.FileRef still works fine with PPB_FileRef_1_2, but
>> PPAPITest.FileRef tries to use PPB_FileRef_1_1 and then fails. I'm not
>> sure
>
> why
>>
>> each of them uses different interface version. Does anyone have any idea
>> about
>> this? Probably I'm missing something...
>
>
> I think that is because we don't support dev-channel APIs in-process. I
> think
> you could skip running the test in-process.
> (Justin knows better.)

That sounds plausible to me, though I haven't taken a look yet. Feel
free to file a bug or a TODO for me to come back around and fix that
up.

>
>
> Thanks!
>
>
>
>
https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
> File
> content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h
> (right):
>
>
https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
> content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h:31:
> int32_t make_directory_flags) OVERRIDE;
> wrong indent.
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
> File ppapi/api/ppb_file_ref.idl (right):
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
> ppapi/api/ppb_file_ref.idl:19: * behavior of
> <code>FileRef::MakeDirectory()</code>.
> Maybe we should use PPB_FileRef.MakeDirectory()?
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
> ppapi/api/ppb_file_ref.idl:21: [assert_size(4)]
> FYI, we haven't made the change to remove all existing assert_size tags,
> but we are planning to stop using it:
> https://code.google.com/p/chromium/issues/detail?id=327421
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
> ppapi/api/ppb_file_ref.idl:29: * Requests that the
> FileRef::MakeDirectory() call fails if the directory
> PPB_FileRef.MakeDirectory()?
>
> https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc
> File ppapi/cpp/file_ref.cc (right):
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
> ppapi/cpp/file_ref.cc:123: if (make_directory_flags &
> PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
> I think it is better to test all the bits except
> PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS are zero (and the
> PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS bit may or may not be zero).
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
> ppapi/cpp/file_ref.cc:131: if (make_directory_flags &
> PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
> ditto
>
> https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h
> File ppapi/cpp/file_ref.h (right):
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
> ppapi/cpp/file_ref.h:98: /// @param[in] exclusive A <code>bool</code>
> set to <code>true</code> to fail
> Please remove this @param
>
>
https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
> ppapi/cpp/file_ref.h:101: /// <code>PP_MakeDirectoryFlags</code> values.
>  Valid values are:
> it seems unnecessary to list valid values here. readers can refer to the
> definition of PP_MakeDirectoryFlags, right?
>
> https://codereview.chromium.org/113363004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[yzshen1, 2014-01-07 18:19:41]

>
>
> > I think that is because we don't support dev-channel APIs in-process. I
> > think
> > you could skip running the test in-process.
> > (Justin knows better.)
>
> That sounds plausible to me, though I haven't taken a look yet. Feel
> free to file a bug or a TODO for me to come back around and fix that
> up.
>

Just to be clear, I was not suggesting that we add in-process dev-channel
support. I am happy with the current behavior if it is by design. :) (But
it is up to you.)
Thanks!

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmichael (off chromium), 2014-01-07 18:35:56]

I noticed this as well while working on Map. I think we're missing
including interfaces_ppb_public_dev_channel.h in a couple of places. I.e.:
https://codereview.chromium.org/69663002/diff/90001/chrome/browser/component_...
https://codereview.chromium.org/69663002/diff/90001/content/renderer/pepper/p...
Not sure if there's other work to do.


On Tue, Jan 7, 2014 at 11:19 AM, Yuzhu Shen <yzshen@chromium.org> wrote:

>
>> > I think that is because we don't support dev-channel APIs in-process. I
>> > think
>> > you could skip running the test in-process.
>> > (Justin knows better.)
>>
>> That sounds plausible to me, though I haven't taken a look yet. Feel
>> free to file a bug or a TODO for me to come back around and fix that
>> up.
>>
>
> Just to be clear, I was not suggesting that we add in-process dev-channel
> support. I am happy with the current behavior if it is by design. :) (But
> it is up to you.)
> Thanks!
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmichael (off chromium), 2014-01-07 21:03:42]

Thanks! I don't think I have anything to add over Yuzhu's comments. lgtm when
he's happy.

[nhiroki, 2014-01-08 01:27:36]

+tsepez@,

Hi Tom, can you review ppapi_messages.h?
This replaces a bool flag with a bit-field flag to support new option for
FileRef.MakeDirectory().

Thanks!

[nhiroki, 2014-01-08 06:11:04]

Thanks guys! 

As dmichael@ mentioned, missing the dev_channel header file caused in-process
test failure. The latest patchset should fix it. Can you take another look?

[nhiroki, 2014-01-08 06:12:32]

Forgot to publish comments...

https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
File content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h
(right):

https://codereview.chromium.org/113363004/diff/1060001/content/browser/render...
content/browser/renderer_host/pepper/pepper_external_file_ref_backend.h:31:
int32_t make_directory_flags) OVERRIDE;
On 2014/01/07 17:59:23, yzshen1 wrote:
> wrong indent.

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
File ppapi/api/ppb_file_ref.idl (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:19: * behavior of
<code>FileRef::MakeDirectory()</code>.
On 2014/01/07 17:59:23, yzshen1 wrote:
> Maybe we should use PPB_FileRef.MakeDirectory()?

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:21: [assert_size(4)]
On 2014/01/07 17:59:23, yzshen1 wrote:
> FYI, we haven't made the change to remove all existing assert_size tags, but
we
> are planning to stop using it:
> https://code.google.com/p/chromium/issues/detail?id=327421

Good to know. Removed.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/api/ppb_file_ref...
ppapi/api/ppb_file_ref.idl:29: * Requests that the FileRef::MakeDirectory() call
fails if the directory
On 2014/01/07 17:59:23, yzshen1 wrote:
> PPB_FileRef.MakeDirectory()?

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc
File ppapi/cpp/file_ref.cc (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
ppapi/cpp/file_ref.cc:123: if (make_directory_flags &
PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
On 2014/01/07 17:59:23, yzshen1 wrote:
> I think it is better to test all the bits except
> PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS are zero (and the
> PP_MAKEDIRECTORYFLAG_WITH_ANCESTORS bit may or may not be zero).
> 
> 
> 
> 

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.cc#...
ppapi/cpp/file_ref.cc:131: if (make_directory_flags &
PP_MAKEDIRECTORYFLAG_EXCLUSIVE)
On 2014/01/07 17:59:23, yzshen1 wrote:
> ditto

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h
File ppapi/cpp/file_ref.h (right):

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
ppapi/cpp/file_ref.h:98: /// @param[in] exclusive A <code>bool</code> set to
<code>true</code> to fail
On 2014/01/07 17:59:23, yzshen1 wrote:
> Please remove this @param

Done.

https://codereview.chromium.org/113363004/diff/1060001/ppapi/cpp/file_ref.h#n...
ppapi/cpp/file_ref.h:101: /// <code>PP_MakeDirectoryFlags</code> values.  Valid
values are:
On 2014/01/07 17:59:23, yzshen1 wrote:
> it seems unnecessary to list valid values here. readers can refer to the
> definition of PP_MakeDirectoryFlags, right?

Done.

[yzshen1, 2014-01-08 17:30:52]

LGTM

Thanks!

[Tom Sepez, 2014-01-08 20:10:20]

Messages LGTM

[commit-bot: I haz the power, 2014-01-09 05:04:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/113363004/1480001

[commit-bot: I haz the power, 2014-01-09 05:04:57]

Failed to apply patch for
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
  Hunk #1 FAILED at 1.
  Hunk #2 succeeded at 143 (offset -1 lines).
  Hunk #3 succeeded at 527 (offset -1 lines).
  Hunk #4 FAILED at 4165.
  Hunk #5 succeeded at 5184 (offset -5 lines).
  Hunk #6 succeeded at 5761 (offset -5 lines).
  2 out of 6 hunks FAILED -- saving rejects to file
ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c.rej

Patch:       ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
Index: ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
index
c83a1f6d5b812bcea6ae2c088ec575d67e31f93b..d38c3bb01cde51e05e4d9a40fc30c4f66e06daaf
100644
--- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
+++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2013 The Chromium Authors. All rights reserved.
+/* Copyright (c) 2014 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.
  */
@@ -144,6 +144,7 @@ static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_FileIO_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileIO_1_1;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileRef_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileRef_1_1;
+static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileRef_1_2;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileSystem_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_Graphics2D_1_0;
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_Graphics2D_1_1;
@@ -527,6 +528,70 @@ static int32_t
Pnacl_M28_PPB_FileRef_ReadDirectoryEntries(PP_Resource file_ref,
 
 /* End wrapper methods for PPB_FileRef_1_1 */
 
+/* Begin wrapper methods for PPB_FileRef_1_2 */
+
+static PP_Resource Pnacl_M34_PPB_FileRef_Create(PP_Resource file_system, const
char* path) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->Create(file_system, path);
+}
+
+static PP_Bool Pnacl_M34_PPB_FileRef_IsFileRef(PP_Resource resource) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->IsFileRef(resource);
+}
+
+static PP_FileSystemType Pnacl_M34_PPB_FileRef_GetFileSystemType(PP_Resource
file_ref) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->GetFileSystemType(file_ref);
+}
+
+static void Pnacl_M34_PPB_FileRef_GetName(struct PP_Var* _struct_result,
PP_Resource file_ref) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  *_struct_result = iface->GetName(file_ref);
+}
+
+static void Pnacl_M34_PPB_FileRef_GetPath(struct PP_Var* _struct_result,
PP_Resource file_ref) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  *_struct_result = iface->GetPath(file_ref);
+}
+
+static PP_Resource Pnacl_M34_PPB_FileRef_GetParent(PP_Resource file_ref) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->GetParent(file_ref);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_MakeDirectory(PP_Resource directory_ref,
int32_t make_directory_flags, struct PP_CompletionCallback* callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->MakeDirectory(directory_ref, make_directory_flags, *callback);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_Touch(PP_Resource file_ref, PP_Time
last_access_time, PP_Time last_modified_time, struct PP_CompletionCallback*
callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->Touch(file_ref, last_access_time, last_modified_time,
*callback);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_Delete(PP_Resource file_ref, struct
PP_CompletionCallback* callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->Delete(file_ref, *callback);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_Rename(PP_Resource file_ref, PP_Resource
new_file_ref, struct PP_CompletionCallback* callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->Rename(file_ref, new_file_ref, *callback);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_Query(PP_Resource file_ref, struct
PP_FileInfo* info, struct PP_CompletionCallback* callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->Query(file_ref, info, *callback);
+}
+
+static int32_t Pnacl_M34_PPB_FileRef_ReadDirectoryEntries(PP_Resource file_ref,
struct PP_ArrayOutput* output, struct PP_CompletionCallback* callback) {
+  const struct PPB_FileRef_1_2 *iface =
Pnacl_WrapperInfo_PPB_FileRef_1_2.real_iface;
+  return iface->ReadDirectoryEntries(file_ref, *output, *callback);
+}
+
+/* End wrapper methods for PPB_FileRef_1_2 */
+
 /* Begin wrapper methods for PPB_FileSystem_1_0 */
 
 static PP_Resource Pnacl_M14_PPB_FileSystem_Create(PP_Instance instance,
PP_FileSystemType type) {
@@ -4100,6 +4165,21 @@ struct PPB_FileRef_1_1 Pnacl_Wrappers_PPB_FileRef_1_1 = {
     .ReadDirectoryEntries = (int32_t (*)(PP_Resource file_ref, struct
PP_ArrayOutput output, struct PP_CompletionCallback
callback))&Pnacl_M28_PPB_FileRef_ReadDirectoryEntries
 };
 
+struct PPB_FileRef_1_2 Pnacl_Wrappers_PPB_FileRef_1_2 = {
+    .Create = (PP_Resource (*)(PP_Resource file_system, const char*
path))&Pnacl_M34_PPB_FileRef_Create,
+    .IsFileRef = (PP_Bool (*)(PP_Resource
resource))&Pnacl_M34_PPB_FileRef_IsFileRef,
+    .GetFileSystemType = (PP_FileSystemType (*)(PP_Resource
file_ref))&Pnacl_M34_PPB_FileRef_GetFileSystemType,
+    .GetName = (struct PP_Var (*)(PP_Resource
file_ref))&Pnacl_M34_PPB_FileRef_GetName,
+    .GetPath = (struct PP_Var (*)(PP_Resource
file_ref))&Pnacl_M34_PPB_FileRef_GetPath,
+    .GetParent = (PP_Resource (*)(PP_Resource
file_ref))&Pnacl_M34_PPB_FileRef_GetParent,
+    .MakeDirectory = (int32_t (*)(PP_Resource directory_ref, int32_t
make_directory_flags, struct PP_CompletionCallback
callback))&Pnacl_M34_PPB_FileRef_MakeDirectory,
+    .Touch = (int32_t (*)(PP_Resource file_ref, PP_Time last_access_time,
PP_Time last_modified_time, struct PP_CompletionCallback
callback))&Pnacl_M34_PPB_FileRef_Touch,
+    .Delete = (int32_t (*)(PP_Resource file_ref, struct PP_CompletionCallback
callback))&Pnacl_M34_PPB_FileRef_Delete,
+    .Rename = (int32_t (*)(PP_Resource file_ref, PP_Resource new_file_ref,
struct PP_CompletionCallback callback))&Pnacl_M34_PPB_FileRef_Rename,
+    .Query = (int32_t (*)(PP_Resource file_ref, struct PP_FileInfo* info,
struct PP_CompletionCallback callback))&Pnacl_M34_PPB_FileRef_Query,
+    .ReadDirectoryEntries = (int32_t (*)(PP_Resource file_ref, struct
PP_ArrayOutput output, struct PP_CompletionCallback
callback))&Pnacl_M34_PPB_FileRef_ReadDirectoryEntries
+};
+
 struct PPB_FileSystem_1_0 Pnacl_Wrappers_PPB_FileSystem_1_0 = {
     .Create = (PP_Resource (*)(PP_Instance instance, PP_FileSystemType
type))&Pnacl_M14_PPB_FileSystem_Create,
     .IsFileSystem = (PP_Bool (*)(PP_Resource
resource))&Pnacl_M14_PPB_FileSystem_IsFileSystem,
@@ -5124,6 +5204,12 @@ static struct __PnaclWrapperInfo
Pnacl_WrapperInfo_PPB_FileRef_1_1 = {
   .real_iface = NULL
 };
 
+static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileRef_1_2 = {
+  .iface_macro = PPB_FILEREF_INTERFACE_1_2,
+  .wrapped_iface = (void *) &Pnacl_Wrappers_PPB_FileRef_1_2,
+  .real_iface = NULL
+};
+
 static struct __PnaclWrapperInfo Pnacl_WrapperInfo_PPB_FileSystem_1_0 = {
   .iface_macro = PPB_FILESYSTEM_INTERFACE_1_0,
   .wrapped_iface = (void *) &Pnacl_Wrappers_PPB_FileSystem_1_0,
@@ -5695,6 +5781,7 @@ static struct __PnaclWrapperInfo *s_ppb_wrappers[] = {
   &Pnacl_WrapperInfo_PPB_FileIO_1_1,
   &Pnacl_WrapperInfo_PPB_FileRef_1_0,
   &Pnacl_WrapperInfo_PPB_FileRef_1_1,
+  &Pnacl_WrapperInfo_PPB_FileRef_1_2,
   &Pnacl_WrapperInfo_PPB_FileSystem_1_0,
   &Pnacl_WrapperInfo_PPB_Graphics2D_1_0,
   &Pnacl_WrapperInfo_PPB_Graphics2D_1_1,

[commit-bot: I haz the power, 2014-01-09 05:27:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nhiroki@chromium.org/113363004/1810001

[commit-bot: I haz the power, 2014-01-09 07:45:33]

Change committed as 243802