Add CHECKs to ensure that PPAPI resources are created on the right thread

Add CHECKs to ensure that PPAPI resources are created on the right thread

These CHECKs could be converted to DCHECKs at some point. But we should get a
clear idea of how often this is happening and what issues it is responsible
for first.

BUG=146415, 92909
TEST=Ran test case in http://code.google.com/p/chromium/issues/detail?id=143183
and checked that the CHECK fired.
NOTRY=true


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155257

[dmichael (off chromium), 2012-09-06 02:57:26]

lgtm

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
File ppapi/shared_impl/resource_tracker.cc (right):

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
ppapi/shared_impl/resource_tracker.cc:200: void
ResourceTracker::LastPluginRefWasDeleted(Resource* object) {
Is there any reason not to check here too? Poking around, it does look like it's
only called from other ResourceTracker functions. Maybe you could make it
private, and then it would be more obvious that you don't have to check here?

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
File ppapi/shared_impl/resource_tracker.h (right):

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
ppapi/shared_impl/resource_tracker.h:102: // allowed off of the main thread.
I would rephrase that to "We won't need to do thread checks" :-)

If we're lucky, maybe it will fix a few crashes when it goes in.

[raymes, 2012-09-06 16:23:59]

Thanks

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
File ppapi/shared_impl/resource_tracker.cc (right):

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
ppapi/shared_impl/resource_tracker.cc:200: void
ResourceTracker::LastPluginRefWasDeleted(Resource* object) {
On 2012/09/06 02:57:26, dmichael wrote:
> Is there any reason not to check here too? Poking around, it does look like
it's
> only called from other ResourceTracker functions. Maybe you could make it
> private, and then it would be more obvious that you don't have to check here?

Done.

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
File ppapi/shared_impl/resource_tracker.h (right):

https://chromiumcodereview.appspot.com/10912086/diff/8001/ppapi/shared_impl/r...
ppapi/shared_impl/resource_tracker.h:102: // allowed off of the main thread.
On 2012/09/06 02:57:26, dmichael wrote:
> I would rephrase that to "We won't need to do thread checks" :-)
> 
> If we're lucky, maybe it will fix a few crashes when it goes in.

Done.

[commit-bot: I haz the power, 2012-09-06 16:24:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/10912086/10003

[commit-bot: I haz the power, 2012-09-06 18:22:01]

Try job failure for 10912086-10003 (retry) on linux_rel for step
"interactive_ui_tests".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-09-06 18:32:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/10912086/10003

[commit-bot: I haz the power, 2012-09-06 21:37:29]

Try job failure for 10912086-10003 (retry) on win_rel for step
"sync_integration_tests".
It's a second try, previously, step "sync_integration_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-09-06 21:40:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/10912086/10003

[commit-bot: I haz the power, 2012-09-06 22:03:28]

Change committed as 155257