Remove the proxy callback tracker.

Remove the proxy callback tracker.

This doesn't properly delete callbacks when the corresponding resource goes
away. This can lead to leaks or crashes in the plugin when the callback is
triggered unexpectedly.

BUG=http://crbug.com/86279

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

[viettrungluu, 2011-10-12 21:52:50]

You should probably look at the test failures in your try jobs.

[brettw, 2011-10-14 22:10:29]

On 2011/10/12 21:52:50, viettrungluu wrote:
> You should probably look at the test failures in your try jobs.

Can you please review this? I don't think any errors are very core to this
patch, and most don't look like my fault at all.

[viettrungluu, 2011-10-17 17:11:03]

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/enter_proxy.h
File ppapi/proxy/enter_proxy.h (right):

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/enter_proxy.h#ne...
ppapi/proxy/enter_proxy.h:197: RunCallback(PP_ERROR_BADRESOURCE);
PP_ERROR_BADARGUMENT?

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/enter_proxy.h#ne...
ppapi/proxy/enter_proxy.h:211: RunCallback(PP_ERROR_BADRESOURCE);
"

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
File ppapi/proxy/ppb_file_ref_proxy.cc (right):

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
ppapi/proxy/ppb_file_ref_proxy.cc:65: int next_callback_id_;
Seems bad that things will fail when this wraps, which isn't that inconceivable.

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
ppapi/proxy/ppb_file_ref_proxy.cc:150: return;
Is this really an expected case? Should it ever be reached?

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_url_loader_p...
File ppapi/proxy/ppb_url_loader_proxy.cc (right):

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_url_loader_p...
ppapi/proxy/ppb_url_loader_proxy.cc:138: // one thing (open, follow redirect,
read, etc.) out-standing at once.
s/-//

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_url_loader_p...
ppapi/proxy/ppb_url_loader_proxy.cc:139: PP_CompletionCallback
current_callback_;
Hrm, now the callbacks for different functions are no longer independent. Either
the error code reported is wrong (e.g., if you call FinishStreamingToFile with
an Open pending, you now get a PP_ERROR_INPROGRESS) or our semantics are
unclear/wrong (I haven't thought about whether it ever makes sense for several
things to be outstanding), or both.

[brettw, 2011-10-17 19:49:20]

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
File ppapi/proxy/ppb_file_ref_proxy.cc (right):

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
ppapi/proxy/ppb_file_ref_proxy.cc:65: int next_callback_id_;
I added a loop to check for duplicates.

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_file_ref_pro...
ppapi/proxy/ppb_file_ref_proxy.cc:150: return;
I think this happens a bunch, I added a comment.

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_url_loader_p...
File ppapi/proxy/ppb_url_loader_proxy.cc (right):

http://codereview.chromium.org/8226009/diff/4001/ppapi/proxy/ppb_url_loader_p...
ppapi/proxy/ppb_url_loader_proxy.cc:139: PP_CompletionCallback
current_callback_;
I considered this but the impl does the same thing. It's not possible to wait
for more than one thing at once with this class.

[viettrungluu, 2011-10-20 23:29:46]

LGTM. Sorry for the delay.

http://codereview.chromium.org/8226009/diff/8019/ppapi/proxy/ppb_instance_pro...
File ppapi/proxy/ppb_instance_proxy.h (right):

http://codereview.chromium.org/8226009/diff/8019/ppapi/proxy/ppb_instance_pro...
ppapi/proxy/ppb_instance_proxy.h:21: #undef PostMessage
This makes this header incompatible with using Windows's PostMessage, which is
unfortunate. Probably we can make this file compile properly on Windows by
renaming ppapi::thunk::PPB_Instance_FunctionAPI's PostMessage method.

Users of ppb_messaging.h are potentially more screwed. Maybe we should do
something like:

#ifdef PostMessage
#error "crap"
#endif

(I'd suggest "#warning", but that's apparently nonstandard.)

Let's deal with this in a separate CL.

http://codereview.chromium.org/8226009/diff/8019/ppapi/thunk/ppb_instance_api.h
File ppapi/thunk/ppb_instance_api.h (right):

http://codereview.chromium.org/8226009/diff/8019/ppapi/thunk/ppb_instance_api...
ppapi/thunk/ppb_instance_api.h:18: #undef PostMessage
"