Changeset 64487 in webkit


Ignore:
Timestamp:
Aug 2, 2010 1:02:43 PM (14 years ago)
Author:
andersca@apple.com
Message:

Protect the plug-in from being destroyed while in plug-in code
​https://bugs.webkit.org/show_bug.cgi?id=43370

Reviewed by Sam Weinig.

Add a PluginProtector to NPRuntimeObjectMap and use it in JSNPObject.

  • WebProcess/Plugins/JSNPObject.cpp:

(WebKit::JSNPObject::callMethod):
(WebKit::JSNPObject::callObject):
(WebKit::JSNPObject::callConstructor):
(WebKit::JSNPObject::put):
(WebKit::JSNPObject::getOwnPropertyNames):
(WebKit::JSNPObject::propertyGetter):
Add PluginProtector declarations.

  • WebProcess/Plugins/NPRuntimeObjectMap.cpp:

(WebKit::NPRuntimeObjectMap::PluginProtector::PluginProtector):
Ref the plug-in view (unless it's being destroyed).

(WebKit::NPRuntimeObjectMap::PluginProtector::~PluginProtector):

  • WebProcess/Plugins/NPRuntimeObjectMap.h:
  • WebProcess/Plugins/PluginView.cpp:

(WebKit::PluginView::PluginView):
Initialize m_isBeingDestroyed.

(WebKit::PluginView::~PluginView):
Set m_isBeingDestroyed to true.

(WebKit::PluginView::scriptObject):
Don't crash if the plug-in failed to initialize.

(WebKit::PluginView::evaluate):
Remove comment.

  • WebProcess/Plugins/PluginView.h:

(WebKit::PluginView::isBeingDestroyed):

Location:
trunk/WebKit2
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKit2/ChangeLog

    r64485 r64487  
    5858        (WebKit::WebBackForwardList::forwardListAsImmutableArrayWithLimit):
    5959        Use a simpler and correct expression to determine "last".
     60
     612010-08-02  Anders Carlsson  <andersca@apple.com>
     62
     63        Reviewed by Sam Weinig.
     64
     65        Protect the plug-in from being destroyed while in plug-in code
     66        https://bugs.webkit.org/show_bug.cgi?id=43370
     67
     68        Add a PluginProtector to NPRuntimeObjectMap and use it in JSNPObject.
     69       
     70        * WebProcess/Plugins/JSNPObject.cpp:
     71        (WebKit::JSNPObject::callMethod):
     72        (WebKit::JSNPObject::callObject):
     73        (WebKit::JSNPObject::callConstructor):
     74        (WebKit::JSNPObject::put):
     75        (WebKit::JSNPObject::getOwnPropertyNames):
     76        (WebKit::JSNPObject::propertyGetter):
     77        Add PluginProtector declarations.
     78
     79        * WebProcess/Plugins/NPRuntimeObjectMap.cpp:
     80        (WebKit::NPRuntimeObjectMap::PluginProtector::PluginProtector):
     81        Ref the plug-in view (unless it's being destroyed).
     82
     83        (WebKit::NPRuntimeObjectMap::PluginProtector::~PluginProtector):
     84        * WebProcess/Plugins/NPRuntimeObjectMap.h:
     85
     86        * WebProcess/Plugins/PluginView.cpp:
     87        (WebKit::PluginView::PluginView):
     88        Initialize m_isBeingDestroyed.
     89
     90        (WebKit::PluginView::~PluginView):
     91        Set m_isBeingDestroyed to true.
     92
     93        (WebKit::PluginView::scriptObject):
     94        Don't crash if the plug-in failed to initialize.
     95
     96        (WebKit::PluginView::evaluate):
     97        Remove comment.
     98
     99        * WebProcess/Plugins/PluginView.h:
     100        (WebKit::PluginView::isBeingDestroyed):
    60101
    611022010-08-02  Anders Carlsson  <andersca@apple.com>

  • trunk/WebKit2/WebProcess/Plugins/JSNPObject.cpp

    r64479 r64487  
    8989        m_objectMap->convertJSValueToNPVariant(exec, exec->argument(i), arguments[i]);
    9090
     91    // Calling NPClass::invoke will call into plug-in code, and there's no telling what the plug-in can do.
     92    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     93    // the call has finished.
     94    NPRuntimeObjectMap::PluginProtector protector(m_objectMap);
     95
    9196    bool returnValue;
    9297    NPVariant result;
    … …  
    96101        JSLock::DropAllLocks dropAllLocks(SilenceAssertionsOnly);
    97102        returnValue = m_npObject->_class->invoke(m_npObject, methodName, arguments.data(), argumentCount, &result);
    98 
    99         // FIXME: Handle invoke setting an exception.
    100         // FIXME: Find out what happens if calling invoke causes the plug-in to go away.
     103        NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
    101104    }
    102105
    … …  
    124127    for (size_t i = 0; i < argumentCount; ++i)
    125128        m_objectMap->convertJSValueToNPVariant(exec, exec->argument(i), arguments[i]);
     129
     130    // Calling NPClass::invokeDefault will call into plug-in code, and there's no telling what the plug-in can do.
     131    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     132    // the call has finished.
     133    NPRuntimeObjectMap::PluginProtector protector(m_objectMap);
    126134   
    127135    bool returnValue;
    … …  
    132140        JSLock::DropAllLocks dropAllLocks(SilenceAssertionsOnly);
    133141        returnValue = m_npObject->_class->invokeDefault(m_npObject, arguments.data(), argumentCount, &result);
    134        
    135         // FIXME: Handle invokeDefault setting an exception.
    136         // FIXME: Find out what happens if calling invokeDefault causes the plug-in to go away.
     142        NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
    137143    }
    138144
    … …  
    161167        m_objectMap->convertJSValueToNPVariant(exec, exec->argument(i), arguments[i]);
    162168
     169    // Calling NPClass::construct will call into plug-in code, and there's no telling what the plug-in can do.
     170    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     171    // the call has finished.
     172    NPRuntimeObjectMap::PluginProtector protector(m_objectMap);
     173   
    163174    bool returnValue;
    164175    NPVariant result;
    … …  
    169180        returnValue = m_npObject->_class->construct(m_npObject, arguments.data(), argumentCount, &result);
    170181        NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
    171        
    172         // FIXME: Find out what happens if calling construct causes the plug-in to go away.
    173182    }
    174183
    … …  
    286295    NPVariant variant;
    287296    m_objectMap->convertJSValueToNPVariant(exec, value, variant);
     297
     298    // Calling NPClass::setProperty will call into plug-in code, and there's no telling what the plug-in can do.
     299    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     300    // the call has finished.
     301    NPRuntimeObjectMap::PluginProtector protector(m_objectMap);
     302
    288303    {
    289304        JSLock::DropAllLocks dropAllLocks(SilenceAssertionsOnly);
    … …  
    292307        NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
    293308
    294         // FIXME: Find out what happens if calling setProperty causes the plug-in to go away.
    295309        // FIXME: Should we throw an exception if setProperty returns false?
    296310    }
    … …  
    312326    uint32_t identifierCount = 0;
    313327   
    314     {
    315         JSLock::DropAllLocks dropAllLocks(SilenceAssertionsOnly);
    316 
    317         // FIXME: Find out what happens if calling enumerate causes the plug-in to go away.
     328    // Calling NPClass::enumerate will call into plug-in code, and there's no telling what the plug-in can do.
     329    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     330    // the call has finished.
     331    NPRuntimeObjectMap::PluginProtector protector(m_objectMap);
     332   
     333    {
     334        JSLock::DropAllLocks dropAllLocks(SilenceAssertionsOnly);
     335
    318336        // FIXME: Should we throw an exception if enumerate returns false?
    319337        if (!m_npObject->_class->enumerate(m_npObject, &identifiers, &identifierCount))
    … …  
    355373    VOID_TO_NPVARIANT(result);
    356374
     375    // Calling NPClass::getProperty will call into plug-in code, and there's no telling what the plug-in can do.
     376    // (including destroying the plug-in). Because of this, we make sure to keep the plug-in alive until
     377    // the call has finished.
     378    NPRuntimeObjectMap::PluginProtector protector(thisObj->m_objectMap);
     379   
    357380    bool returnValue;
    358381    {
    … …  
    362385       
    363386        NPRuntimeObjectMap::moveGlobalExceptionToExecState(exec);
    364 
    365         // FIXME: Find out what happens if calling getProperty causes the plug-in to go away.
    366387    }
    367388

  • trunk/WebKit2/WebProcess/Plugins/NPRuntimeObjectMap.cpp

    r64479 r64487  
    4747}
    4848
     49NPRuntimeObjectMap::PluginProtector::PluginProtector(NPRuntimeObjectMap* npRuntimeObjectMap)
     50{
     51    // If we're already in the plug-in view destructor, we shouldn't try to keep it alive.
     52    if (!npRuntimeObjectMap->m_pluginView->isBeingDestroyed())
     53        m_pluginView = npRuntimeObjectMap->m_pluginView;
     54}
     55
     56NPRuntimeObjectMap::PluginProtector::~PluginProtector()
     57{
     58}
     59
    4960NPObject* NPRuntimeObjectMap::getOrCreateNPObject(JSObject* jsObject)
    5061{

  • trunk/WebKit2/WebProcess/Plugins/NPRuntimeObjectMap.h

    r64479 r64487  
    5454    explicit NPRuntimeObjectMap(PluginView*);
    5555
     56    class PluginProtector {
     57    public:
     58        explicit PluginProtector(NPRuntimeObjectMap* npRuntimeObjectMap);
     59        ~PluginProtector();
     60       
     61    private:
     62        RefPtr<PluginView> m_pluginView;
     63    };
     64
    5665    // Returns an NPObject that wraps the given JSObject object. If there is already an NPObject that wraps this JSObject, it will
    5766    // retain it and return it.

  • trunk/WebKit2/WebProcess/Plugins/PluginView.cpp

    r64377 r64487  
    218218    , m_isInitialized(false)
    219219    , m_isWaitingUntilMediaCanStart(false)
     220    , m_isBeingDestroyed(false)
    220221    , m_pendingURLRequestsTimer(RunLoop::main(), this, &PluginView::pendingURLRequestsTimerFired)
    221222    , m_npRuntimeObjectMap(this)
    … …  
    225226PluginView::~PluginView()
    226227{
     228    ASSERT(!m_isBeingDestroyed);
     229
    227230    if (m_isWaitingUntilMediaCanStart)
    228231        m_pluginElement->document()->removeMediaCanStartListener(this);
    … …  
    233236        it->first->setLoadListener(0);
    234237
    235     if (m_plugin && m_isInitialized)
     238    if (m_plugin && m_isInitialized) {
     239        m_isBeingDestroyed = true;
    236240        m_plugin->destroy();
     241        m_isBeingDestroyed = false;
     242    }
    237243
    238244    // Invalidate the object map.
    … …  
    285291JSObject* PluginView::scriptObject(JSGlobalObject* globalObject)
    286292{
     293    // The plug-in can be null here if it failed to initialize.
     294    if (!m_plugin)
     295        return 0;
     296   
    287297    NPObject* scriptableNPObject = m_plugin->pluginScriptableNPObject();
    288298    if (!scriptableNPObject)
    … …  
    645655    frame()->script()->setAllowPopupsFromPlugin(allowPopups);
    646656
    647     // FIXME: What happens if calling evaluate will cause the plug-in view to go away.
    648657    bool returnValue = m_npRuntimeObjectMap.evaluate(npObject, scriptString, result);
    649658

  • trunk/WebKit2/WebProcess/Plugins/PluginView.h

    r64377 r64487  
    5454
    5555    WebCore::Frame* frame();
     56
     57    bool isBeingDestroyed() const { return m_isBeingDestroyed; }
    5658
    5759private:
    … …  
    117119    bool m_isInitialized;
    118120    bool m_isWaitingUntilMediaCanStart;
     121    bool m_isBeingDestroyed;
    119122
    120123    // Pending URLRequests that the plug-in has made.
Note: See TracChangeset for help on using the changeset viewer.