Pepper: Cleanup in PepperReverseInterface.

Pepper: Cleanup in PepperReverseInterface.

Some code in PepperReverseInterface is more complicated than it needs to be.
I figured I'd clean this up a bit while I was looking at the manifest handling
logic.

BUG=
R=dmichael@chromium.org

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

[dmichael (off chromium), 2014-03-05 16:54:50]

Yep, those look ripe for cleaning up, thanks...  a couple of suggestions.

https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right):

https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:172: while (true) {
Could this instead be:
while (!shutting_down_ && !op_complete)
  NaClXCondVarWait(&cv_, &mu);
NaClLog("... done!");  // <-- The "done!" log happens for either
                       // case in the old code
if (shutting_down_) {
  NaClLog("... plugin is shutting down\n");
  return false;
}

Or am I misunderstanding something?

https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:365: while (true) {
I think you could do the same kind of transformation here, unless I'm missing
something...

while (!shutting_down_ && !op_complete)
  NaClXCondVarWait(&cv_, &mu_);
if (shutting_down_)
  return false;

[teravest, 2014-03-05 17:17:24]

On Wed, Mar 5, 2014 at 9:54 AM,  <dmichael@chromium.org> wrote:
> Yep, those look ripe for cleaning up, thanks...  a couple of suggestions.

Thanks, what you suggest is better.

>
>
>
https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
> File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right):
>
>
https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
> ppapi/native_client/src/trusted/plugin/service_runtime.cc:172: while
> (true) {
> Could this instead be:
> while (!shutting_down_ && !op_complete)
>   NaClXCondVarWait(&cv_, &mu);
> NaClLog("... done!");  // <-- The "done!" log happens for either
>                        // case in the old code
> if (shutting_down_) {
>   NaClLog("... plugin is shutting down\n");
>   return false;
> }
>
> Or am I misunderstanding something?

Done.

>
>
https://codereview.chromium.org/183793007/diff/1/ppapi/native_client/src/trus...
> ppapi/native_client/src/trusted/plugin/service_runtime.cc:365: while
> (true) {
> I think you could do the same kind of transformation here, unless I'm
> missing something...
>
> while (!shutting_down_ && !op_complete)
>   NaClXCondVarWait(&cv_, &mu_);
> if (shutting_down_)
>   return false;

Done.

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

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

[dmichael (off chromium), 2014-03-05 18:08:40]

lgtm

[teravest, 2014-03-05 18:43:19]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-03-05 18:43:42]

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

[commit-bot: I haz the power, 2014-03-05 21:38:51]

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

[commit-bot: I haz the power, 2014-03-05 21:38:51]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[teravest, 2014-03-06 15:41:57]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-03-06 15:42:21]

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

[commit-bot: I haz the power, 2014-03-06 17:03:36]

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

[commit-bot: I haz the power, 2014-03-06 17:03:36]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[teravest, 2014-03-06 21:04:50]

Committed patchset #3 manually as r255425 (presubmit successful).