Pepper: TempFile cleanup.

Pepper: TempFile cleanup.

This cleans up some usage of TempFile and some logic inside PnaclCoordinator to
make it easier to follow. I made these changes while doing some other cleanup
for DescWrapper use in the trusted plugin.

BUG=333950
R=jvoung@chromium.org

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

[jvoung (off chromium), 2014-05-23 17:53:04]

LGTM

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/temporary_file.cc (right):

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/temporary_file.cc:33:
plugin_->nacl_interface()->CreateTemporaryFile(plugin_->pp_instance());
Might have to set "existing_handle_ = file_handle" afterward if TakeFileHandle()
is to return the "internal_handle", otherwise TakeFileHandle() only works for
the case where GetNexeFd() sets the handle.

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/temporary_file.cc:95: // TODO(teravest):
Figure out why we can't reset read_wrapper_ and
=/ not sure why either...

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/temporary_file.h (right):

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/temporary_file.h:12: #include
"ppapi/cpp/completion_callback.h"
Can remove this completion_callback include now

https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/temporary_file.h:60: PP_FileHandle*
existing_handle() { return &existing_handle_; }
Maybe call this "internal_handle" ?

[teravest, 2014-05-23 20:22:40]

Thanks!

On 2014/05/23 17:53:04, jvoung wrote:
> LGTM
> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> File ppapi/native_client/src/trusted/plugin/temporary_file.cc (right):
> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> ppapi/native_client/src/trusted/plugin/temporary_file.cc:33:
> plugin_->nacl_interface()->CreateTemporaryFile(plugin_->pp_instance());
> Might have to set "existing_handle_ = file_handle" afterward if
TakeFileHandle()
> is to return the "internal_handle", otherwise TakeFileHandle() only works for
> the case where GetNexeFd() sets the handle.

Done.

> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> ppapi/native_client/src/trusted/plugin/temporary_file.cc:95: //
TODO(teravest):
> Figure out why we can't reset read_wrapper_ and
> =/ not sure why either...
> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> File ppapi/native_client/src/trusted/plugin/temporary_file.h (right):
> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> ppapi/native_client/src/trusted/plugin/temporary_file.h:12: #include
> "ppapi/cpp/completion_callback.h"
> Can remove this completion_callback include now

Done.

> 
>
https://codereview.chromium.org/299143004/diff/10001/ppapi/native_client/src/...
> ppapi/native_client/src/trusted/plugin/temporary_file.h:60: PP_FileHandle*
> existing_handle() { return &existing_handle_; }
> Maybe call this "internal_handle" ?

Done.

[teravest, 2014-05-23 20:59:01]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-23 21:00:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/299143004/80001

[commit-bot: I haz the power, 2014-05-24 02:53:41]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-24 04:55:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-24 04:55:47]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[teravest, 2014-05-24 15:43:06]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-24 15:43:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/299143004/80001

[commit-bot: I haz the power, 2014-05-24 23:20:12]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-25 01:14:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-25 01:14:14]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[teravest, 2014-05-27 18:05:07]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-27 18:05:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/299143004/100001

[teravest, 2014-05-27 20:58:55]

Committed patchset #4 manually as r273024 (presubmit successful).