Changeset 228100 in webkit


Ignore:
Timestamp:
Feb 5, 2018 10:03:58 AM (6 years ago)
Author:
dbates@webkit.org
Message:

Disallow evaluating JavaScript from NPP_Destroy() in WebKit
​https://bugs.webkit.org/show_bug.cgi?id=181889
<rdar://problem/36674701>

Reviewed by Brent Fulgham.

Source/WebKit:

Make the behavior of WebKit match the behavior of WebKitLegacy on Mac.

  • Shared/Plugins/NPObjectMessageReceiver.cpp:

(WebKit::NPObjectMessageReceiver::hasMethod):
(WebKit::NPObjectMessageReceiver::invoke):
(WebKit::NPObjectMessageReceiver::invokeDefault):
(WebKit::NPObjectMessageReceiver::hasProperty):
(WebKit::NPObjectMessageReceiver::getProperty):
(WebKit::NPObjectMessageReceiver::setProperty):
(WebKit::NPObjectMessageReceiver::removeProperty):
(WebKit::NPObjectMessageReceiver::enumerate):
(WebKit::NPObjectMessageReceiver::construct):
Bail out if the plugin is executing NPP_Destroy().

  • WebProcess/Plugins/Plugin.cpp:

(WebKit::Plugin::destroyPlugin):

  • WebProcess/Plugins/Plugin.h:

(WebKit::Plugin::isBeingDestroyed const):
Move bookkeeping of whether the plugin is being destroyed from PluginView
to here. This makes it straightforward for NPObjectMessageReceiver to query
this information.

  • WebProcess/Plugins/PluginView.cpp:

(WebKit::PluginView::~PluginView):
(WebKit::PluginView::destroyPluginAndReset):
(WebKit::PluginView::recreateAndInitialize):
(WebKit::PluginView::protectPluginFromDestruction):
(WebKit::PluginView::unprotectPluginFromDestruction):
Move bookkeeping of whether the plugin is being destroyed from here
to Plugin.

  • WebProcess/Plugins/PluginView.h:

(WebKit::PluginView::isBeingDestroyed const): Turn around and ask the plugin if it
is being destroyed, if we have one.

LayoutTests:

Consolidate all the plugin tests that evaluate JavaScript from NPP_Destroy()
and mark them as Wont Fix. In a subsequent change we will look to replace
these tests with tests that ensure that we do not evaluate JavaScript from
NPP_Destroy().

  • platform/mac/TestExpectations:
  • platform/wk2/TestExpectations:
Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r228097 r228100  
     12018-02-05  Daniel Bates  <dabates@apple.com>
     2
     3        Disallow evaluating JavaScript from NPP_Destroy() in WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=181889
     5        <rdar://problem/36674701>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Consolidate all the plugin tests that evaluate JavaScript from NPP_Destroy()
     10        and mark them as Wont Fix. In a subsequent change we will look to replace
     11        these tests with tests that ensure that we do not evaluate JavaScript from
     12        NPP_Destroy().
     13
     14        * platform/mac/TestExpectations:
     15        * platform/wk2/TestExpectations:
     16
    1172018-02-05  Antoine Quint  <graouts@apple.com>
    218

  • trunk/LayoutTests/platform/mac/TestExpectations

    r228097 r228100  
    190190fast/images/animated-webp-expected.html
    191191
    192 # Times out because plugins aren't allowed to execute JS after NPP_Destroy has been called in WebKit1's OOP plugins implementation
    193 webkit.org/b/48929 plugins/evaluate-js-after-removing-plugin-element.html
    194192
    195193# DRT does not support toggling caret browsing on / off
    … …  
    436434
    437435# --- Plugins ---
    438 # WebKit1 OOP plug-ins: Can't evaluate JavaScript from NPP_Destroy.
    439 plugins/document-open.html
    440 plugins/geturlnotify-during-document-teardown.html
    441 plugins/nested-plugin-objects.html
    442 plugins/netscape-destroy-plugin-script-objects.html
    443 plugins/open-and-close-window-with-plugin.html
     436# Out-of-process plug-ins are disallowed from evaluating JavaScript from NPP_Destroy().
     437plugins/attach-during-destroy.html [ WontFix ]
     438plugins/destroy-reentry.html [ WontFix ]
     439plugins/document-open.html [ WontFix ]
     440webkit.org/b/48929 plugins/evaluate-js-after-removing-plugin-element.html [ WontFix ]
     441plugins/geturlnotify-during-document-teardown.html [ WontFix ]
     442plugins/js-from-destroy.html [ WontFix ]
     443plugins/nested-plugin-objects.html [ WontFix ]
     444plugins/netscape-destroy-plugin-script-objects.html [ WontFix ]
     445plugins/open-and-close-window-with-plugin.html [ WontFix ]
    444446
    445447# WebKit1 OOP plug-ins: No support for getting the form value.

  • trunk/LayoutTests/platform/wk2/TestExpectations

    r227986 r228100  
    126126transitions/default-timing-function.html
    127127
    128 # WebKitTestRunner needs testRunner.setCallCloseOnWebViews
    129 # http://webkit.org/b/46714
    130 plugins/geturlnotify-during-document-teardown.html
    131 plugins/open-and-close-window-with-plugin.html
    132 
    133128# Sometimes fails
    134129# http://webkit.org/b/58990
    … …  
    513508########################################
    514509### START OF (4) Features that are not supported in WebKit2 and likely never will be
     510
     511# Plug-ins are disallowed from evaluating JavaScript from NPP_Destroy().
     512plugins/attach-during-destroy.html [ WontFix ]
     513plugins/destroy-reentry.html [ WontFix ]
     514plugins/document-open.html [ WontFix ]
     515webkit.org/b/48929 plugins/evaluate-js-after-removing-plugin-element.html [ WontFix ]
     516plugins/geturlnotify-during-document-teardown.html [ WontFix ]
     517plugins/js-from-destroy.html [ WontFix ]
     518plugins/nested-plugin-objects.html [ WontFix ]
     519plugins/netscape-destroy-plugin-script-objects.html [ WontFix ]
     520plugins/open-and-close-window-with-plugin.html [ WontFix ]
    515521
    516522# Internals.registerDefaultPortForProtocol() does not affect NetworkProcess. We should

  • trunk/Source/WebKit/ChangeLog

    r228087 r228100  
     12018-02-05  Daniel Bates  <dabates@apple.com>
     2
     3        Disallow evaluating JavaScript from NPP_Destroy() in WebKit
     4        https://bugs.webkit.org/show_bug.cgi?id=181889
     5        <rdar://problem/36674701>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Make the behavior of WebKit match the behavior of WebKitLegacy on Mac.
     10
     11        * Shared/Plugins/NPObjectMessageReceiver.cpp:
     12        (WebKit::NPObjectMessageReceiver::hasMethod):
     13        (WebKit::NPObjectMessageReceiver::invoke):
     14        (WebKit::NPObjectMessageReceiver::invokeDefault):
     15        (WebKit::NPObjectMessageReceiver::hasProperty):
     16        (WebKit::NPObjectMessageReceiver::getProperty):
     17        (WebKit::NPObjectMessageReceiver::setProperty):
     18        (WebKit::NPObjectMessageReceiver::removeProperty):
     19        (WebKit::NPObjectMessageReceiver::enumerate):
     20        (WebKit::NPObjectMessageReceiver::construct):
     21        Bail out if the plugin is executing NPP_Destroy().
     22
     23        * WebProcess/Plugins/Plugin.cpp:
     24        (WebKit::Plugin::destroyPlugin):
     25        * WebProcess/Plugins/Plugin.h:
     26        (WebKit::Plugin::isBeingDestroyed const):
     27        Move bookkeeping of whether the plugin is being destroyed from PluginView
     28        to here. This makes it straightforward for NPObjectMessageReceiver to query
     29        this information.
     30
     31        * WebProcess/Plugins/PluginView.cpp:
     32        (WebKit::PluginView::~PluginView):
     33        (WebKit::PluginView::destroyPluginAndReset):
     34        (WebKit::PluginView::recreateAndInitialize):
     35        (WebKit::PluginView::protectPluginFromDestruction):
     36        (WebKit::PluginView::unprotectPluginFromDestruction):
     37        Move bookkeeping of whether the plugin is being destroyed from here
     38        to Plugin.
     39
     40        * WebProcess/Plugins/PluginView.h:
     41        (WebKit::PluginView::isBeingDestroyed const): Turn around and ask the plugin if it
     42        is being destroyed, if we have one.
     43
    1442018-02-05  Carlos Garcia Campos  <cgarcia@igalia.com>
    245

  • trunk/Source/WebKit/Shared/Plugins/NPObjectMessageReceiver.cpp

    r181864 r228100  
    6161void NPObjectMessageReceiver::hasMethod(const NPIdentifierData& methodNameData, bool& returnValue)
    6262{
    63     if (!m_npObject->_class->hasMethod) {
     63    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->hasMethod) {
    6464        returnValue = false;
    6565        return;
    … …  
    7171void NPObjectMessageReceiver::invoke(const NPIdentifierData& methodNameData, const Vector<NPVariantData>& argumentsData, bool& returnValue, NPVariantData& resultData)
    7272{
    73     if (!m_npObject->_class->invoke) {
     73    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->invoke) {
    7474        returnValue = false;
    7575        return;
    … …  
    101101void NPObjectMessageReceiver::invokeDefault(const Vector<NPVariantData>& argumentsData, bool& returnValue, NPVariantData& resultData)
    102102{
    103     if (!m_npObject->_class->invokeDefault) {
     103    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->invokeDefault) {
    104104        returnValue = false;
    105105        return;
    … …  
    131131void NPObjectMessageReceiver::hasProperty(const NPIdentifierData& propertyNameData, bool& returnValue)
    132132{
    133     if (!m_npObject->_class->hasProperty) {
     133    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->hasProperty) {
    134134        returnValue = false;
    135135        return;
    … …  
    141141void NPObjectMessageReceiver::getProperty(const NPIdentifierData& propertyNameData, bool& returnValue, NPVariantData& resultData)
    142142{
    143     if (!m_npObject->_class->getProperty) {
     143    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->getProperty) {
    144144        returnValue = false;
    145145        return;
    … …  
    163163void NPObjectMessageReceiver::setProperty(const NPIdentifierData& propertyNameData, const NPVariantData& propertyValueData, bool& returnValue)
    164164{
    165     if (!m_npObject->_class->setProperty) {
     165    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->setProperty) {
    166166        returnValue = false;
    167167        return;
    … …  
    179179void NPObjectMessageReceiver::removeProperty(const NPIdentifierData& propertyNameData, bool& returnValue)
    180180{
    181     if (!m_npObject->_class->removeProperty) {
     181    if (m_plugin->isBeingDestroyed() || !m_npObject->_class->removeProperty) {
    182182        returnValue = false;
    183183        return;
    … …  
    189189void NPObjectMessageReceiver::enumerate(bool& returnValue, Vector<NPIdentifierData>& identifiersData)
    190190{
    191     if (!NP_CLASS_STRUCT_VERSION_HAS_ENUM(m_npObject->_class) || !m_npObject->_class->enumerate) {
     191    if (m_plugin->isBeingDestroyed() || !NP_CLASS_STRUCT_VERSION_HAS_ENUM(m_npObject->_class) || !m_npObject->_class->enumerate) {
    192192        returnValue = false;
    193193        return;
    … …  
    209209void NPObjectMessageReceiver::construct(const Vector<NPVariantData>& argumentsData, bool& returnValue, NPVariantData& resultData)
    210210{
    211     if (!NP_CLASS_STRUCT_VERSION_HAS_CTOR(m_npObject->_class) || !m_npObject->_class->construct) {
     211    if (m_plugin->isBeingDestroyed() || !NP_CLASS_STRUCT_VERSION_HAS_CTOR(m_npObject->_class) || !m_npObject->_class->construct) {
    212212        returnValue = false;
    213213        return;

  • trunk/Source/WebKit/WebProcess/Plugins/Plugin.cpp

    r204668 r228100  
    2929#include "WebCoreArgumentCoders.h"
    3030#include <WebCore/IntPoint.h>
     31#include <wtf/SetForScope.h>
    3132
    3233using namespace WebCore;
    … …  
    99100void Plugin::destroyPlugin()
    100101{
     102    ASSERT(!m_isBeingDestroyed);
     103    SetForScope<bool> scope { m_isBeingDestroyed, true };
     104
    101105    destroy();
    102106
    103     m_pluginController = 0;
     107    m_pluginController = nullptr;
    104108}
    105109

  • trunk/Source/WebKit/WebProcess/Plugins/Plugin.h

    r227214 r228100  
    105105    void destroyPlugin();
    106106
     107    bool isBeingDestroyed() const { return m_isBeingDestroyed; }
     108
    107109    // Returns the plug-in controller for this plug-in.
    108110    PluginController* controller() { return m_pluginController; }
    … …  
    310312    PluginType m_type;
    311313
     314    bool m_isBeingDestroyed { false };
     315
    312316private:
    313317    PluginController* m_pluginController;

  • trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp

    r227214 r228100  
    320320        m_webPage->removePluginView(this);
    321321
    322     ASSERT(!m_isBeingDestroyed);
     322    ASSERT(!m_plugin || !m_plugin->isBeingDestroyed());
    323323
    324324    if (m_isWaitingUntilMediaCanStart)
    … …  
    341341
    342342    if (m_plugin) {
    343         m_isBeingDestroyed = true;
    344343        m_plugin->destroyPlugin();
    345         m_isBeingDestroyed = false;
    346344
    347345        m_pendingURLRequests.clear();
    … …  
    374372    m_isWaitingForSynchronousInitialization = false;
    375373    m_isWaitingUntilMediaCanStart = false;
    376     m_isBeingDestroyed = false;
    377374    m_manualStreamState = ManualStreamState::Initial;
    378375    m_transientPaintingSnapshot = nullptr;
    … …  
    16421639void PluginView::protectPluginFromDestruction()
    16431640{
    1644     if (!m_isBeingDestroyed)
     1641    if (m_plugin && !m_plugin->isBeingDestroyed())
    16451642        ref();
    16461643}
    … …  
    16481645void PluginView::unprotectPluginFromDestruction()
    16491646{
    1650     if (m_isBeingDestroyed)
     1647    if (!m_plugin || m_plugin->isBeingDestroyed())
    16511648        return;
    16521649

  • trunk/Source/WebKit/WebProcess/Plugins/PluginView.h

    r227214 r228100  
    7171    WebCore::Frame* frame() const;
    7272
    73     bool isBeingDestroyed() const { return m_isBeingDestroyed; }
     73    bool isBeingDestroyed() const { return !m_plugin || m_plugin->isBeingDestroyed(); }
    7474
    7575    void manualLoadDidReceiveResponse(const WebCore::ResourceResponse&);
    … …  
    249249    bool m_isWaitingForSynchronousInitialization { false };
    250250    bool m_isWaitingUntilMediaCanStart { false };
    251     bool m_isBeingDestroyed { false };
    252251    bool m_pluginProcessHasCrashed { false };
    253252
Note: See TracChangeset for help on using the changeset viewer.