PNaCl cleanup: Simplify error checking when opening temp files

PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

Committed: https://crrev.com/2be51e74fd35932038e3031ab2345b367ba34bce
Cr-Commit-Position: refs/heads/master@{#370518}

[Mark Seaborn, 2016-01-20 19:21:32]

The CQ bit was checked by mseaborn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-20 19:23:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1608313002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1608313002/1

[commit-bot: I haz the power, 2016-01-20 20:15:12]

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

[commit-bot: I haz the power, 2016-01-20 20:15:13]

Dry run: This issue passed the CQ dry run.

[Mark Seaborn, 2016-01-20 21:09:28]

Description was changed from

==========
NaCl cleanup: Remove error code from NexeReadDidOpen(); use bool instead
==========

to

==========
NaCl cleanup: Remove error code from NexeReadDidOpen(); use bool instead

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

[Mark Seaborn, 2016-01-20 21:09:29]

mseaborn@chromium.org changed reviewers:
+ bbudge@chromium.org

[bbudge, 2016-01-20 21:25:57]

LGTM w/question

https://codereview.chromium.org/1608313002/diff/1/components/nacl/renderer/pl...
File components/nacl/renderer/plugin/pnacl_coordinator.cc (right):

https://codereview.chromium.org/1608313002/diff/1/components/nacl/renderer/pl...
components/nacl/renderer/plugin/pnacl_coordinator.cc:209: if (!success) {
Could you just test temp_nexe_file_->IsValid() here and avoid the parameter
altogether?

[Mark Seaborn, 2016-01-20 21:49:14]

Description was changed from

==========
NaCl cleanup: Remove error code from NexeReadDidOpen(); use bool instead

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

to

==========
PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

[Mark Seaborn, 2016-01-20 21:49:47]

https://codereview.chromium.org/1608313002/diff/1/components/nacl/renderer/pl...
File components/nacl/renderer/plugin/pnacl_coordinator.cc (right):

https://codereview.chromium.org/1608313002/diff/1/components/nacl/renderer/pl...
components/nacl/renderer/plugin/pnacl_coordinator.cc:209: if (!success) {
On 2016/01/20 21:25:57, bbudge wrote:
> Could you just test temp_nexe_file_->IsValid() here and avoid the parameter
> altogether?

Good point!  Done.

[Mark Seaborn, 2016-01-20 21:49:51]

The CQ bit was checked by mseaborn@chromium.org

[Mark Seaborn, 2016-01-20 21:49:52]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org
Link to the patchset: https://codereview.chromium.org/1608313002/#ps20001
(title: "Review: remove arg & move check")

[commit-bot: I haz the power, 2016-01-20 21:50:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1608313002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1608313002/20001

[commit-bot: I haz the power, 2016-01-20 22:51:12]

Description was changed from

==========
PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

to

==========
PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

[commit-bot: I haz the power, 2016-01-20 22:51:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-20 22:52:26]

Description was changed from

==========
PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)
==========

to

==========
PNaCl cleanup: Simplify error checking when opening temp files

TempFile::CheckValidity() only ever returned PP_OK or PP_ERROR_FAILED.
We can replace its result with a bool.

This makes it clear that ReportPpapiError() was only ever called with
PP_ERROR_FAILED, so we can remove it and call ReportNonPpapiError()
instead.

We can also move an error check into NexeReadDidOpen(), instead of
having its callers check.

BUG=302078
TEST=e.g. NaClBrowserTestPnacl.PPAPICore (tests PNaCl translation)

Committed: https://crrev.com/2be51e74fd35932038e3031ab2345b367ba34bce
Cr-Commit-Position: refs/heads/master@{#370518}
==========

[commit-bot: I haz the power, 2016-01-20 22:52:27]

Patchset 2 (id:??) landed as
https://crrev.com/2be51e74fd35932038e3031ab2345b367ba34bce
Cr-Commit-Position: refs/heads/master@{#370518}