Pepper proxy: make the browser sender handle the proxy lock properly.

Pepper proxy: make the browser sender handle the proxy lock properly.
- unlock the pepper proxy lock when sending sync messages to the browser;
- remove the SendToBrowser() method from the PluginProxyDelegate;

TEST=None
BUG=161286
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169781

[yzshen1, 2012-11-22 00:26:12]

Hi, Brett.

Would you please take a look? Thanks!

[yzshen1, 2012-11-22 00:42:29]

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_globals.cc
File ppapi/proxy/plugin_globals.cc (right):

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_globals.cc...
ppapi/proxy/plugin_globals.cc:111: ProxyAutoUnlock unlock;
This is a little ugly, because unlocking here implies that we know about the
internal implementation of plugin_proxy_delegate_->PreCacheFont(). However, I
cannot think up a better solution:
- I cannot send ChildProcessHostMsg_PreCacheFont directly in this file,
otherwise it needs to depend on the content common target;
- I cannot create a new message, say, PpapiHostMsg_PreCacheFont and let
PpapiPluginProcessHost do the work. (It is hard to access the font cache there.)
- Letting PpapiThread call PluginGlobals::Get()->GetBrowserSender() sounds ugly
as well.


Please let me know if you have a better idea. Thanks! :)

[brettw, 2012-11-27 00:49:15]

LGTM

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_dispatcher.cc
File ppapi/proxy/plugin_dispatcher.cc (right):

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_dispatcher...
ppapi/proxy/plugin_dispatcher.cc:198: // may be accessed by multiple threads at
the same time.
Is there a bug on this?

[commit-bot: I haz the power, 2012-11-27 00:49:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yzshen@chromium.org/11299147/1

[commit-bot: I haz the power, 2012-11-27 03:29:54]

Retried try job too often for step(s) content_unittests

[commit-bot: I haz the power, 2012-11-27 22:26:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yzshen@chromium.org/11299147/1

[commit-bot: I haz the power, 2012-11-27 23:01:32]

Change committed as 169781

[yzshen1, 2012-11-30 04:29:13]

Thanks Brett!

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_dispatcher.cc
File ppapi/proxy/plugin_dispatcher.cc (right):

https://codereview.chromium.org/11299147/diff/1/ppapi/proxy/plugin_dispatcher...
ppapi/proxy/plugin_dispatcher.cc:198: // may be accessed by multiple threads at
the same time.
On 2012/11/27 00:49:15, brettw wrote:
> Is there a bug on this?

Yes. Although I haven't seen any bug reports, the Send() implementation doesn't
look safe. I will take a closer look and fix it.