Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Issue 6881012: Keep the module in scope when executing scripts. This prevents a crash when the (Closed)

Created:
9 years, 8 months ago by brettw
Modified:
9 years, 7 months ago
Reviewers:
viettrungluu
CC:
chromium-reviews, piman+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Keep the module in scope when executing scripts. This prevents a crash when the script deletes the plugin object synchronously. This in turn deletes the dispatcher which will make the code returning the out param and exception to the plugin crash. To prevent the crash, this patch adds a way for the proxy to manipulate the refcount of the plugin object so that it's still alive when as long as the scripting message is being processed. A manual test is included. This is not automatically run now. I tried to fit it into the current test infrastructure and found it very challenging, We need to revisit this to allow custom tests to more easily be written. TEST=manual with included plugin and html BUG=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81993

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+165 lines, -1 line) Patch
M ppapi/c/private/ppb_proxy_private.h View 1 chunk +6 lines, -0 lines 0 comments Download
M ppapi/proxy/host_dispatcher.h View 1 chunk +17 lines, -0 lines 0 comments Download
M ppapi/proxy/host_dispatcher.cc View 1 chunk +13 lines, -0 lines 1 comment Download
M ppapi/proxy/ppb_instance_proxy.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M ppapi/proxy/ppb_var_deprecated_proxy.cc View 1 chunk +7 lines, -0 lines 0 comments Download
A ppapi/tests/manual/delete_plugin.cc View 1 1 chunk +74 lines, -0 lines 1 comment Download
A ppapi/tests/manual/delete_plugin.html View 1 1 chunk +25 lines, -0 lines 0 comments Download
M webkit/plugins/ppapi/ppapi_plugin_instance.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/plugins/ppapi/ppb_proxy_impl.cc View 1 chunk +15 lines, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
brettw
9 years, 8 months ago (2011-04-18 18:03:10 UTC) #1
viettrungluu
9 years, 8 months ago (2011-04-18 19:53:41 UTC) #2
LGTM w/nits.

http://codereview.chromium.org/6881012/diff/5003/ppapi/proxy/host_dispatcher.cc
File ppapi/proxy/host_dispatcher.cc (right):

http://codereview.chromium.org/6881012/diff/5003/ppapi/proxy/host_dispatcher....
ppapi/proxy/host_dispatcher.cc:236: 
Nit: Is the extra blank line really necessary?

http://codereview.chromium.org/6881012/diff/5003/ppapi/tests/manual/delete_pl...
File ppapi/tests/manual/delete_plugin.cc (right):

http://codereview.chromium.org/6881012/diff/5003/ppapi/tests/manual/delete_pl...
ppapi/tests/manual/delete_plugin.cc:45: /*
Could you add a comment on why it's commented out?

Powered by Google App Engine
This is Rietveld 408576698