PPAPI: Blocking callbacks shouldn't need a target_loop_

PPAPI: Blocking callbacks shouldn't need a target_loop_

See bug for details.

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

[Peng, 2013-09-04 22:26:46]

On 2013/09/04 22:05:53, dmichael wrote:

It fixes my issue. Thanks.

[bbudge, 2013-09-04 22:29:18]

https://codereview.chromium.org/23706006/diff/6001/ppapi/shared_impl/tracked_...
File ppapi/shared_impl/tracked_callback.cc (right):

https://codereview.chromium.org/23706006/diff/6001/ppapi/shared_impl/tracked_...
ppapi/shared_impl/tracked_callback.cc:174: } else if (!target_loop_.get()) {
It seems like the most common case is last. Do you have a feel for the
likelihood of the different cases? Did you consider switching the ifs? In other
words,

if (target_loop_.get()) {
....post task to target_loop_
} else {
  if (is_blocking()) {
.....blocking code
  } else {
.....in-process code
  }
}

[bbudge, 2013-09-04 22:31:45]

https://codereview.chromium.org/23706006/diff/10001/ppapi/shared_impl/tracked...
File ppapi/shared_impl/tracked_callback.cc (right):

https://codereview.chromium.org/23706006/diff/10001/ppapi/shared_impl/tracked...
ppapi/shared_impl/tracked_callback.cc:169:
RunWhileLocked(base::Bind(&TrackedCallback::Run, this, result)));
We should probably move this into the 'else' clause of the first 'if'.

[dmichael (off chromium), 2013-09-04 22:44:12]

I took a little bit of both comments. WDYT?

[bbudge, 2013-09-04 22:50:36]

LGTM

https://codereview.chromium.org/23706006/diff/1002/ppapi/shared_impl/tracked_...
File ppapi/shared_impl/tracked_callback.cc (right):

https://codereview.chromium.org/23706006/diff/1002/ppapi/shared_impl/tracked_...
ppapi/shared_impl/tracked_callback.cc:176: if (target_loop_) {
Do we have a conversion operator now? Should we start using it elsewhere?

[bbudge, 2013-09-04 22:52:16]

https://codereview.chromium.org/23706006/diff/1002/ppapi/shared_impl/tracked_...
File ppapi/shared_impl/tracked_callback.cc (right):

https://codereview.chromium.org/23706006/diff/1002/ppapi/shared_impl/tracked_...
ppapi/shared_impl/tracked_callback.cc:171: // don't use the locking closure.
Second part of comment needs tweaking. Maybe s/use the/run a

[commit-bot: I haz the power, 2013-09-05 16:20:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmichael@chromium.org/23706006/29001

[commit-bot: I haz the power, 2013-09-05 23:04:51]

Change committed as 221535