⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 185404 in webkit


Ignore:
Timestamp:
Jun 9, 2015, 11:26:57 PM (11 years ago)
Author:
youenn.fablet@crf.canon.fr
Message:

DeferredWrapper should clear its JS strong references once its promise is resolved/rejected
​https://bugs.webkit.org/show_bug.cgi?id=145753

Reviewed by Darin Adler.

Clear strong references at the end of DeferredWrapper::resolve and DeferredWrapper::reject.
Added assertions to check that resolve or reject is called only once.

Removed DeferredWrapper constructor that creates internally its promise.
Reason is DeferredWrapper can be resolved synchronously and promise
will be lost when returning it at the end of the binding promise function.
Updated all custom bindings accordingly.

Covered by existing tests.

  • bindings/js/JSAudioContextCustom.cpp:

(WebCore::JSAudioContext::suspend): Updated DeferredWrapper constructor to properly return the promise.
(WebCore::JSAudioContext::resume): Ditto.
(WebCore::JSAudioContext::close): Ditto.

  • bindings/js/JSDOMPromise.cpp:

(WebCore::DeferredWrapper::resolve): Cleared strong references after promise resolution.
(WebCore::DeferredWrapper::reject): Cleared strong references after promise rejection.

  • bindings/js/JSDOMPromise.h:

(WebCore::DeferredWrapper::resolve): Added ASSERT to check promise is not yet rejected/resolved.
(WebCore::DeferredWrapper::reject): Ditto.
(WebCore::DeferredWrapper::reject<ExceptionCode>): Ditto.
(WebCore::DeferredWrapper::resolve<String>): Ditto.
(WebCore::DeferredWrapper::resolve<bool>): Ditto.
(WebCore::DeferredWrapper::resolve<JSC::JSValue>): Ditto.
(WebCore::char>>):
(WebCore::DeferredWrapper::reject<String>): Ditto.

  • bindings/js/JSMediaDevicesCustom.cpp:

(WebCore::JSMediaDevices::getUserMedia): Updated DeferredWrapper constructor to properly return the promise.

  • bindings/js/JSReadableStreamReaderCustom.cpp:

(WebCore::JSReadableStreamReader::read): Ditto.
(WebCore::JSReadableStreamReader::closed): Ditto.

  • bindings/js/JSSubtleCryptoCustom.cpp:

(WebCore::JSSubtleCrypto::encrypt): Ditto.
(WebCore::JSSubtleCrypto::decrypt): Ditto.
(WebCore::JSSubtleCrypto::sign): Ditto.
(WebCore::JSSubtleCrypto::verify): Ditto.
(WebCore::JSSubtleCrypto::digest): Ditto.
(WebCore::JSSubtleCrypto::generateKey): Ditto.
(WebCore::JSSubtleCrypto::importKey): Ditto.
(WebCore::JSSubtleCrypto::exportKey): Ditto.
(WebCore::JSSubtleCrypto::wrapKey): Ditto.
(WebCore::JSSubtleCrypto::unwrapKey): Ditto.

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r185403 r185404  
     12015-06-09  Youenn Fablet  <youenn.fablet@crf.canon.fr>
     2
     3        DeferredWrapper should clear its JS strong references once its promise is resolved/rejected
     4        https://bugs.webkit.org/show_bug.cgi?id=145753
     5
     6        Reviewed by Darin Adler.
     7
     8        Clear strong references at the end of DeferredWrapper::resolve and DeferredWrapper::reject.
     9        Added assertions to check that resolve or reject is called only once.
     10
     11        Removed DeferredWrapper constructor that creates internally its promise.
     12        Reason is DeferredWrapper can be resolved synchronously and promise
     13        will be lost when returning it at the end of the binding promise function.
     14        Updated all custom bindings accordingly.
     15
     16        Covered by existing tests.
     17
     18        * bindings/js/JSAudioContextCustom.cpp:
     19        (WebCore::JSAudioContext::suspend): Updated DeferredWrapper constructor to properly return the promise.
     20        (WebCore::JSAudioContext::resume): Ditto.
     21        (WebCore::JSAudioContext::close): Ditto.
     22        * bindings/js/JSDOMPromise.cpp:
     23        (WebCore::DeferredWrapper::resolve): Cleared strong references after promise resolution.
     24        (WebCore::DeferredWrapper::reject): Cleared strong references after promise rejection.
     25        * bindings/js/JSDOMPromise.h:
     26        (WebCore::DeferredWrapper::resolve): Added ASSERT to check promise is not yet rejected/resolved.
     27        (WebCore::DeferredWrapper::reject): Ditto.
     28        (WebCore::DeferredWrapper::reject<ExceptionCode>): Ditto.
     29        (WebCore::DeferredWrapper::resolve<String>): Ditto.
     30        (WebCore::DeferredWrapper::resolve<bool>): Ditto.
     31        (WebCore::DeferredWrapper::resolve<JSC::JSValue>): Ditto.
     32        (WebCore::char>>):
     33        (WebCore::DeferredWrapper::reject<String>): Ditto.
     34        * bindings/js/JSMediaDevicesCustom.cpp:
     35        (WebCore::JSMediaDevices::getUserMedia): Updated DeferredWrapper constructor to properly return the promise.
     36        * bindings/js/JSReadableStreamReaderCustom.cpp:
     37        (WebCore::JSReadableStreamReader::read): Ditto.
     38        (WebCore::JSReadableStreamReader::closed): Ditto.
     39        * bindings/js/JSSubtleCryptoCustom.cpp:
     40        (WebCore::JSSubtleCrypto::encrypt): Ditto.
     41        (WebCore::JSSubtleCrypto::decrypt): Ditto.
     42        (WebCore::JSSubtleCrypto::sign): Ditto.
     43        (WebCore::JSSubtleCrypto::verify): Ditto.
     44        (WebCore::JSSubtleCrypto::digest): Ditto.
     45        (WebCore::JSSubtleCrypto::generateKey): Ditto.
     46        (WebCore::JSSubtleCrypto::importKey): Ditto.
     47        (WebCore::JSSubtleCrypto::exportKey): Ditto.
     48        (WebCore::JSSubtleCrypto::wrapKey): Ditto.
     49        (WebCore::JSSubtleCrypto::unwrapKey): Ditto.
     50
    1512015-06-09  Zalan Bujtas  <zalan@apple.com>
    252

  • trunk/Source/WebCore/bindings/js/JSAudioContextCustom.cpp

    r184651 r185404  
    114114JSValue JSAudioContext::suspend(ExecState* exec)
    115115{
    116     DeferredWrapper wrapper(exec, globalObject());
     116    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     117    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    117118    auto successCallback = [wrapper]() mutable {
    118119        wrapper.resolve(nullptr);
    … …  
    124125    impl().suspendContext(WTF::move(successCallback), WTF::move(failureCallback));
    125126
    126     return wrapper.promise();
     127    return promiseDeferred->promise();
    127128}
    128129
    129130JSValue JSAudioContext::resume(ExecState* exec)
    130131{
    131     DeferredWrapper wrapper(exec, globalObject());
     132    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     133    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    132134    auto successCallback = [wrapper]() mutable {
    133135        wrapper.resolve(nullptr);
    … …  
    139141    impl().resumeContext(WTF::move(successCallback), WTF::move(failureCallback));
    140142
    141     return wrapper.promise();
     143    return promiseDeferred->promise();
    142144}
    143145
    144146JSValue JSAudioContext::close(ExecState* exec)
    145147{
    146     DeferredWrapper wrapper(exec, globalObject());
     148    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     149    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    147150    auto successCallback = [wrapper]() mutable {
    148151        wrapper.resolve(nullptr);
    … …  
    154157    impl().closeContext(WTF::move(successCallback), WTF::move(failureCallback));
    155158
    156     return wrapper.promise();
     159    return promiseDeferred->promise();
    157160}
    158161

  • trunk/Source/WebCore/bindings/js/JSDOMPromise.cpp

    r180559 r185404  
    3333namespace WebCore {
    3434
    35 DeferredWrapper::DeferredWrapper(ExecState* exec, JSDOMGlobalObject* globalObject)
    36     : m_globalObject(exec->vm(), globalObject)
    37     , m_deferred(exec->vm(), JSPromiseDeferred::create(exec, globalObject))
    38 {
    39 }
    40 
    4135DeferredWrapper::DeferredWrapper(ExecState* exec, JSDOMGlobalObject* globalObject, JSPromiseDeferred* promiseDeferred)
    4236    : m_globalObject(exec->vm(), globalObject)
    … …  
    4741JSObject* DeferredWrapper::promise() const
    4842{
     43    // FIXME: Remove this accessor once ReadableStreamReader custom binding does not need it.
     44    ASSERT(m_deferred);
    4945    return m_deferred->promise();
    5046}
    … …  
    6258
    6359    call(exec, deferredResolve, resolveCallType, resolveCallData, jsUndefined(), arguments);
     60
     61    m_globalObject.clear();
     62    m_deferred.clear();
    6463}
    6564
    … …  
    7675
    7776    call(exec, deferredReject, rejectCallType, rejectCallData, jsUndefined(), arguments);
     77
     78    m_globalObject.clear();
     79    m_deferred.clear();
    7880}
    7981

  • trunk/Source/WebCore/bindings/js/JSDOMPromise.h

    r184651 r185404  
    3939class DeferredWrapper {
    4040public:
    41     DeferredWrapper(JSC::ExecState*, JSDOMGlobalObject*);
    4241    DeferredWrapper(JSC::ExecState*, JSDOMGlobalObject*, JSC::JSPromiseDeferred*);
    4342
    … …  
    6160inline void DeferredWrapper::resolve(const ResolveResultType& result)
    6261{
     62    ASSERT(m_deferred);
     63    ASSERT(m_globalObject);
    6364    JSC::ExecState* exec = m_globalObject->globalExec();
    6465    JSC::JSLockHolder locker(exec);
    … …  
    6970inline void DeferredWrapper::reject(const RejectResultType& result)
    7071{
     72    ASSERT(m_deferred);
     73    ASSERT(m_globalObject);
    7174    JSC::ExecState* exec = m_globalObject->globalExec();
    7275    JSC::JSLockHolder locker(exec);
    … …  
    7780inline void DeferredWrapper::reject(const std::nullptr_t&)
    7881{
     82    ASSERT(m_deferred);
     83    ASSERT(m_globalObject);
    7984    JSC::ExecState* exec = m_globalObject->globalExec();
    8085    JSC::JSLockHolder locker(exec);
    … …  
    8590inline void DeferredWrapper::reject(const JSC::JSValue& value)
    8691{
     92    ASSERT(m_deferred);
     93    ASSERT(m_globalObject);
    8794    JSC::ExecState* exec = m_globalObject->globalExec();
    8895    JSC::JSLockHolder locker(exec);
    … …  
    93100inline void DeferredWrapper::reject<ExceptionCode>(const ExceptionCode& ec)
    94101{
     102    ASSERT(m_deferred);
     103    ASSERT(m_globalObject);
    95104    JSC::ExecState* exec = m_globalObject->globalExec();
    96105    JSC::JSLockHolder locker(exec);
    … …  
    101110inline void DeferredWrapper::resolve<String>(const String& result)
    102111{
     112    ASSERT(m_deferred);
     113    ASSERT(m_globalObject);
    103114    JSC::ExecState* exec = m_globalObject->globalExec();
    104115    JSC::JSLockHolder locker(exec);
    … …  
    109120inline void DeferredWrapper::resolve<bool>(const bool& result)
    110121{
     122    ASSERT(m_deferred);
     123    ASSERT(m_globalObject);
    111124    JSC::ExecState* exec = m_globalObject->globalExec();
    112125    JSC::JSLockHolder locker(exec);
    … …  
    117130inline void DeferredWrapper::resolve<JSC::JSValue>(const JSC::JSValue& value)
    118131{
     132    ASSERT(m_deferred);
     133    ASSERT(m_globalObject);
    119134    JSC::ExecState* exec = m_globalObject->globalExec();
    120135    JSC::JSLockHolder locker(exec);
    … …  
    124139inline void DeferredWrapper::resolve<Vector<unsigned char>>(const Vector<unsigned char>& result)
    125140{
     141    ASSERT(m_deferred);
     142    ASSERT(m_globalObject);
    126143    JSC::ExecState* exec = m_globalObject->globalExec();
    127144    JSC::JSLockHolder locker(exec);
    … …  
    133150inline void DeferredWrapper::resolve(const std::nullptr_t&)
    134151{
     152    ASSERT(m_deferred);
     153    ASSERT(m_globalObject);
    135154    JSC::ExecState* exec = m_globalObject->globalExec();
    136155    JSC::JSLockHolder locker(exec);
    … …  
    141160inline void DeferredWrapper::reject<String>(const String& result)
    142161{
     162    ASSERT(m_deferred);
     163    ASSERT(m_globalObject);
    143164    JSC::ExecState* exec = m_globalObject->globalExec();
    144165    JSC::JSLockHolder locker(exec);

  • trunk/Source/WebCore/bindings/js/JSMediaDevicesCustom.cpp

    r185259 r185404  
    4848JSValue JSMediaDevices::getUserMedia(ExecState* exec)
    4949{
    50     DeferredWrapper wrapper(exec, globalObject());
     50    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     51    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    5152
    5253    Dictionary options(exec, exec->argument(0));
    … …  
    5556        exec->clearException();
    5657        wrapper.reject(exception->value());
    57         return wrapper.promise();
     58        return promiseDeferred->promise();
    5859    }
    5960
    … …  
    6162        JSValue error = createTypeError(exec, "First argument of getUserMedia must be a valid Dictionary");
    6263        wrapper.reject(error);
    63         return wrapper.promise();
     64        return promiseDeferred->promise();
    6465    }
    6566
    … …  
    7677        wrapper.reject(ec);
    7778
    78     return wrapper.promise();
     79    return promiseDeferred->promise();
    7980}
    8081

  • trunk/Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp

    r185149 r185404  
    4747JSValue JSReadableStreamReader::read(ExecState* exec)
    4848{
    49     DeferredWrapper wrapper(exec, globalObject());
     49    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     50    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
     51
    5052    auto successCallback = [wrapper](JSValue value) mutable {
    5153        JSValue result = createIteratorResultObject(wrapper.promise()->globalObject()->globalExec(), value, false);
    … …  
    6264    impl().read(WTF::move(successCallback), WTF::move(endCallback), WTF::move(failureCallback));
    6365
    64     return wrapper.promise();
     66    return promiseDeferred->promise();
    6567}
    6668
    … …  
    8284    impl().closed(WTF::move(successCallback), WTF::move(failureCallback));
    8385
    84     return wrapper.promise();
     86    return m_closedPromiseDeferred->promise();
    8587}
    8688

  • trunk/Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp

    r180717 r185404  
    163163
    164164   
    165     DeferredWrapper wrapper(exec, globalObject());
     165    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     166    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    166167    auto successCallback = [wrapper](const Vector<uint8_t>& result) mutable {
    167168        wrapper.resolve(result);
    … …  
    178179    }
    179180
    180     return wrapper.promise();
     181    return promiseDeferred->promise();
    181182}
    182183
    … …  
    214215    }
    215216
    216     DeferredWrapper wrapper(exec, globalObject());
     217    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     218    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    217219    auto successCallback = [wrapper](const Vector<uint8_t>& result) mutable {
    218220        wrapper.resolve(result);
    … …  
    229231    }
    230232
    231     return wrapper.promise();
     233    return promiseDeferred->promise();
    232234}
    233235
    … …  
    265267    }
    266268
    267     DeferredWrapper wrapper(exec, globalObject());
     269    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     270    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    268271    auto successCallback = [wrapper](const Vector<uint8_t>& result) mutable {
    269272        wrapper.resolve(result);
    … …  
    280283    }
    281284
    282     return wrapper.promise();
     285    return promiseDeferred->promise();
    283286}
    284287
    … …  
    322325    }
    323326
    324     DeferredWrapper wrapper(exec, globalObject());
     327    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     328    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    325329    auto successCallback = [wrapper](bool result) mutable {
    326330        wrapper.resolve(result);
    … …  
    337341    }
    338342
    339     return wrapper.promise();
     343    return promiseDeferred->promise();
    340344}
    341345
    … …  
    363367    }
    364368
    365     DeferredWrapper wrapper(exec, globalObject());
     369    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     370    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    366371    auto successCallback = [wrapper](const Vector<uint8_t>& result) mutable {
    367372        wrapper.resolve(result);
    … …  
    378383    }
    379384
    380     return wrapper.promise();
     385    return promiseDeferred->promise();
    381386}
    382387
    … …  
    413418    }
    414419
    415     DeferredWrapper wrapper(exec, globalObject());
     420    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     421    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    416422    auto successCallback = [wrapper](CryptoKey* key, CryptoKeyPair* keyPair) mutable {
    417423        ASSERT(key || keyPair);
    … …  
    433439    }
    434440
    435     return wrapper.promise();
     441    return promiseDeferred->promise();
    436442}
    437443
    … …  
    540546    }
    541547
    542     DeferredWrapper wrapper(exec, globalObject());
     548    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     549    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    543550    auto successCallback = [wrapper](CryptoKey& result) mutable {
    544551        wrapper.resolve(&result);
    … …  
    552559        return jsUndefined();
    553560
    554     return wrapper.promise();
     561    return promiseDeferred->promise();
    555562}
    556563
    … …  
    602609        return throwTypeError(exec);
    603610
    604     DeferredWrapper wrapper(exec, globalObject());
     611    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     612    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    605613    auto successCallback = [wrapper](const Vector<uint8_t>& result) mutable {
    606614        wrapper.resolve(result);
    … …  
    614622        return jsUndefined();
    615623
    616     return wrapper.promise();
     624    return promiseDeferred->promise();
    617625}
    618626
    … …  
    654662    }
    655663
    656     DeferredWrapper wrapper(exec, globalObject());
     664    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     665    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    657666
    658667    CryptoAlgorithm* algorithmPtr = algorithm.release();
    … …  
    693702    }
    694703
    695     return wrapper.promise();
     704    return promiseDeferred->promise();
    696705}
    697706
    … …  
    766775    }
    767776
    768     DeferredWrapper wrapper(exec, globalObject());
     777    JSPromiseDeferred* promiseDeferred = JSPromiseDeferred::create(exec, globalObject());
     778    DeferredWrapper wrapper(exec, globalObject(), promiseDeferred);
    769779    Strong<JSDOMGlobalObject> domGlobalObject(exec->vm(), globalObject());
    770780
    … …  
    803813    }
    804814
    805     return wrapper.promise();
     815    return promiseDeferred->promise();
    806816}
    807817
Note: See TracChangeset for help on using the changeset viewer.