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

Changeset 242986 in webkit


Ignore:
Timestamp:
Mar 14, 2019, 7:24:55 PM (7 years ago)
Author:
sihui_liu@apple.com
Message:

IndexedDB: re-enable some leak tests
​https://bugs.webkit.org/show_bug.cgi?id=194806

Reviewed by Geoffrey Garen.

Source/WebCore:

Protected JSIDBCursor object when advance/continue request on IDBCursor is not finished, because after the
advance operation completes on success, we need to return the same JSIDBCursor object as before the advance,
and during the wait for advance operation to complete, we need to return error as the result.

Covered by existing tests.

  • Modules/indexeddb/IDBCursor.cpp:

(WebCore::IDBCursor::setGetResult):
(WebCore::IDBCursor::clearWrappers):

  • Modules/indexeddb/IDBCursor.h:
  • Modules/indexeddb/IDBRequest.cpp:

(WebCore::IDBRequest::stop):
(WebCore::IDBRequest::setResult):
(WebCore::IDBRequest::setResultToStructuredClone):
(WebCore::IDBRequest::setResultToUndefined):
(WebCore::IDBRequest::willIterateCursor):
(WebCore::IDBRequest::didOpenOrIterateCursor):
(WebCore::IDBRequest::clearWrappers):

  • Modules/indexeddb/IDBRequest.h:

(WebCore::IDBRequest::cursorWrapper):

  • bindings/js/JSIDBRequestCustom.cpp:

(WebCore::JSIDBRequest::visitAdditionalChildren):

  • bindings/js/JSValueInWrappedObject.h:

(WebCore::JSValueInWrappedObject::JSValueInWrappedObject):
(WebCore::JSValueInWrappedObject::operator=):
(WebCore::JSValueInWrappedObject::clear):

LayoutTests:

  • TestExpectations:
  • platform/win/TestExpectations:
  • storage/indexeddb/connection-leak-expected.txt:
  • storage/indexeddb/connection-leak-private-expected.txt:
  • storage/indexeddb/cursor-leak-expected.txt:
  • storage/indexeddb/cursor-leak-private-expected.txt:
  • storage/indexeddb/cursor-request-cycle-expected.txt:
  • storage/indexeddb/cursor-request-cycle-private-expected.txt:
  • storage/indexeddb/request-leak-expected.txt:
  • storage/indexeddb/request-leak-private-expected.txt:
  • storage/indexeddb/resources/cursor-request-cycle.js:
Location:
trunk
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r242979 r242986  
     12019-03-14  Sihui Liu  <sihui_liu@apple.com>
     2
     3        IndexedDB: re-enable some leak tests
     4        https://bugs.webkit.org/show_bug.cgi?id=194806
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * TestExpectations:
     9        * platform/win/TestExpectations:
     10        * storage/indexeddb/connection-leak-expected.txt:
     11        * storage/indexeddb/connection-leak-private-expected.txt:
     12        * storage/indexeddb/cursor-leak-expected.txt:
     13        * storage/indexeddb/cursor-leak-private-expected.txt:
     14        * storage/indexeddb/cursor-request-cycle-expected.txt:
     15        * storage/indexeddb/cursor-request-cycle-private-expected.txt:
     16        * storage/indexeddb/request-leak-expected.txt:
     17        * storage/indexeddb/request-leak-private-expected.txt:
     18        * storage/indexeddb/resources/cursor-request-cycle.js:
     19
    1202019-03-14  Simon Fraser  <simon.fraser@apple.com>
    221

  • trunk/LayoutTests/TestExpectations

    r242714 r242986  
    15311531# With Modern IDB and the in-memory backing store, that should change.
    15321532storage/indexeddb/open-db-private-browsing.html [ Failure ]
    1533 
    1534 # Relies on internals.observeGC
    1535 storage/indexeddb/connection-leak-private.html [ Skip ]
    1536 storage/indexeddb/connection-leak.html [ Skip ]
    1537 storage/indexeddb/cursor-leak-private.html [ Failure ]
    1538 storage/indexeddb/cursor-leak.html [ Skip ]
    1539 storage/indexeddb/cursor-request-cycle-private.html [ Failure ]
    1540 storage/indexeddb/cursor-request-cycle.html [ Skip ]
    1541 storage/indexeddb/delete-closed-database-object-private.html [ Skip ]
    1542 storage/indexeddb/delete-closed-database-object.html [ Skip ]
    1543 storage/indexeddb/request-leak-private.html [ Failure ]
    1544 storage/indexeddb/request-leak.html [ Failure ]
    15451533
    15461534webkit.org/b/154619 storage/indexeddb/odd-strings.html [ Skip ]

  • trunk/LayoutTests/platform/win/TestExpectations

    r242911 r242986  
    42794279storage/indexeddb/result-request-cycle.html [ Skip ]
    42804280storage/indexeddb/value-cursor-cycle.html [ Skip ]
     4281storage/indexeddb/connection-leak-private.html [ Skip ]
     4282storage/indexeddb/connection-leak.html [ Skip ]
     4283storage/indexeddb/cursor-leak-private.html [ Skip ]
     4284storage/indexeddb/cursor-leak.html [ Skip ]
     4285storage/indexeddb/cursor-request-cycle-private.html [ Skip ]
     4286storage/indexeddb/cursor-request-cycle.html [ Skip ]
     4287storage/indexeddb/delete-closed-database-object-private.html [ Skip ]
     4288storage/indexeddb/delete-closed-database-object.html [ Skip ]
     4289storage/indexeddb/request-leak-private.html [ Skip ]
     4290storage/indexeddb/request-leak.html [ Skip ]
    42814291
    42824292webkit.org/b/194711 fast/replaced/encrypted-pdf-as-object-and-embed.html [ Failure ]

  • trunk/LayoutTests/storage/indexeddb/connection-leak-expected.txt

    r163963 r242986  
    44
    55
    6 dbname = "connection-leak.html"
    76
    87doFirstOpen():

  • trunk/LayoutTests/storage/indexeddb/connection-leak-private-expected.txt

    r195394 r242986  
    44
    55
    6 dbname = "connection-leak.html"
    76
    87doFirstOpen():

  • trunk/LayoutTests/storage/indexeddb/cursor-leak-expected.txt

    r163963 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "cursor-leak.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)

  • trunk/LayoutTests/storage/indexeddb/cursor-leak-private-expected.txt

    r195394 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "cursor-leak.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)

  • trunk/LayoutTests/storage/indexeddb/cursor-request-cycle-expected.txt

    r163963 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "cursor-request-cycle.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)
    … …  
    4140gc()
    4241PASS cursorObservation.wasCollected is false
     42PASS cursorRequestObservation.wasCollected is false
    4343finalRequest = store.get(0)
    4444

  • trunk/LayoutTests/storage/indexeddb/cursor-request-cycle-private-expected.txt

    r195394 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "cursor-request-cycle.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)
    … …  
    4140gc()
    4241PASS cursorObservation.wasCollected is false
     42PASS cursorRequestObservation.wasCollected is false
    4343finalRequest = store.get(0)
    4444

  • trunk/LayoutTests/storage/indexeddb/request-leak-expected.txt

    r163963 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "request-leak.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)

  • trunk/LayoutTests/storage/indexeddb/request-leak-private-expected.txt

    r195394 r242986  
    66indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
    77
    8 dbname = "request-leak.html"
    98indexedDB.deleteDatabase(dbname)
    109indexedDB.open(dbname)

  • trunk/LayoutTests/storage/indexeddb/resources/cursor-request-cycle.js

    r195299 r242986  
    6767        evalAndLog("gc()");
    6868        shouldBeFalse("cursorObservation.wasCollected");
     69        shouldBeFalse("cursorRequestObservation.wasCollected");
    6970
    7071        evalAndLog("finalRequest = store.get(0)");

  • trunk/Source/WebCore/ChangeLog

    r242985 r242986  
     12019-03-14  Sihui Liu  <sihui_liu@apple.com>
     2
     3        IndexedDB: re-enable some leak tests
     4        https://bugs.webkit.org/show_bug.cgi?id=194806
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Protected JSIDBCursor object when advance/continue request on IDBCursor is not finished, because after the
     9        advance operation completes on success, we need to return the same JSIDBCursor object as before the advance,
     10        and during the wait for advance operation to complete, we need to return error as the result.
     11 
     12        Covered by existing tests.
     13
     14        * Modules/indexeddb/IDBCursor.cpp:
     15        (WebCore::IDBCursor::setGetResult):
     16        (WebCore::IDBCursor::clearWrappers):
     17        * Modules/indexeddb/IDBCursor.h:
     18        * Modules/indexeddb/IDBRequest.cpp:
     19        (WebCore::IDBRequest::stop):
     20        (WebCore::IDBRequest::setResult):
     21        (WebCore::IDBRequest::setResultToStructuredClone):
     22        (WebCore::IDBRequest::setResultToUndefined):
     23        (WebCore::IDBRequest::willIterateCursor):
     24        (WebCore::IDBRequest::didOpenOrIterateCursor):
     25        (WebCore::IDBRequest::clearWrappers):
     26        * Modules/indexeddb/IDBRequest.h:
     27        (WebCore::IDBRequest::cursorWrapper):
     28        * bindings/js/JSIDBRequestCustom.cpp:
     29        (WebCore::JSIDBRequest::visitAdditionalChildren):
     30        * bindings/js/JSValueInWrappedObject.h:
     31        (WebCore::JSValueInWrappedObject::JSValueInWrappedObject):
     32        (WebCore::JSValueInWrappedObject::operator=):
     33        (WebCore::JSValueInWrappedObject::clear):
     34
    1352019-03-14  Shawn Roberts  <sroberts@apple.com>
    236

  • trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp

    r241196 r242986  
    310310}
    311311
    312 void IDBCursor::setGetResult(IDBRequest&, const IDBGetResult& getResult)
     312bool IDBCursor::setGetResult(IDBRequest& request, const IDBGetResult& getResult)
    313313{
    314314    LOG(IndexedDB, "IDBCursor::setGetResult - current key %s", getResult.keyData().loggingString().substring(0, 100).utf8().data());
    315315    ASSERT(&effectiveObjectStore().transaction().database().originThread() == &Thread::current());
     316
     317    auto* context = request.scriptExecutionContext();
     318    if (!context)
     319        return false;
     320
     321    VM& vm = context->vm();
     322    JSLockHolder lock(vm);
    316323
    317324    m_keyWrapper = { };
    … …  
    327334
    328335        m_gotValue = false;
    329         return;
     336        return false;
    330337    }
    331338
    … …  
    339346
    340347    m_gotValue = true;
     348    return true;
     349}
     350
     351void IDBCursor::clearWrappers()
     352{
     353    m_keyWrapper.clear();
     354    m_primaryKeyWrapper.clear();
     355    m_valueWrapper.clear();
    341356}
    342357

  • trunk/Source/WebCore/Modules/indexeddb/IDBCursor.h

    r241196 r242986  
    7575    void setRequest(IDBRequest& request) { m_request = makeWeakPtr(&request); }
    7676    void clearRequest() { m_request.clear(); }
     77    void clearWrappers();
    7778    IDBRequest* request() { return m_request.get(); }
    7879
    79     void setGetResult(IDBRequest&, const IDBGetResult&);
     80    bool setGetResult(IDBRequest&, const IDBGetResult&);
    8081
    8182    virtual bool isKeyCursorWithValue() const { return false; }

  • trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp

    r242818 r242986  
    278278    removeAllEventListeners();
    279279
     280    clearWrappers();
     281
    280282    m_contextStopped = true;
    281283}
    … …  
    369371        return;
    370372
    371     auto* state = context->execState();
    372     if (!state)
    373         return;
    374 
    375373    VM& vm = context->vm();
    376374    JSLockHolder lock(vm);
    … …  
    387385        return;
    388386
    389     auto* state = context->execState();
    390     if (!state)
    391         return;
    392 
    393387    VM& vm = context->vm();
    394388    JSLockHolder lock(vm);
    … …  
    405399        return;
    406400
    407     auto* state = context->execState();
    408     if (!state)
    409         return;
    410 
    411401    VM& vm = context->vm();
    412402    JSLockHolder lock(vm);
    … …  
    423413        return;
    424414
     415    VM& vm = context->vm();
     416    JSLockHolder lock(vm);
    425417    m_result = number;
    426418    m_resultWrapper = { };
    … …  
    437429        return;
    438430
    439     auto* state = context->execState();
    440     if (!state)
    441         return;
    442 
    443431    VM& vm = context->vm();
    444432    JSLockHolder lock(vm);
    … …  
    451439    ASSERT(&originThread() == &Thread::current());
    452440
     441    auto* context = scriptExecutionContext();
     442    if (!context)
     443        return;
     444   
     445    VM& vm = context->vm();
     446    JSLockHolder lock(vm);
    453447    m_result = NullResultType::Undefined;
    454448    m_resultWrapper = { };
    … …  
    477471    m_hasPendingActivity = true;
    478472    m_result = NullResultType::Empty;
     473
     474    auto* context = scriptExecutionContext();
     475    if (!context)
     476        return;
     477
     478    VM& vm = context->vm();
     479    JSLockHolder lock(vm);
     480
     481    if (m_resultWrapper)
     482        m_cursorWrapper = m_resultWrapper;
    479483    m_resultWrapper = { };
    480484    m_readyState = ReadyState::Pending;
    … …  
    488492    ASSERT(m_pendingCursor);
    489493
     494    auto* context = scriptExecutionContext();
     495    if (!context)
     496        return;
     497
     498    VM& vm = context->vm();
     499    JSLockHolder lock(vm);
     500
    490501    m_result = NullResultType::Empty;
    491502    m_resultWrapper = { };
    492503
    493504    if (resultData.type() == IDBResultType::IterateCursorSuccess || resultData.type() == IDBResultType::OpenCursorSuccess) {
    494         m_pendingCursor->setGetResult(*this, resultData.getResult());
     505        if (m_pendingCursor->setGetResult(*this, resultData.getResult()) && m_cursorWrapper)
     506            m_resultWrapper = m_cursorWrapper;
    495507        if (resultData.getResult().isDefined())
    496508            m_result = m_pendingCursor;
    … …  
    537549    ASSERT(&originThread() == &Thread::current());
    538550
     551    auto* context = scriptExecutionContext();
     552    if (!context)
     553        return;
     554
     555    VM& vm = context->vm();
     556    JSLockHolder lock(vm);
     557
    539558    m_result = RefPtr<IDBDatabase> { WTFMove(database) };
    540559    m_resultWrapper = { };
    541560}
    542561
     562void IDBRequest::clearWrappers()
     563{
     564    auto* context = scriptExecutionContext();
     565    if (!context)
     566        return;
     567    VM& vm = context->vm();
     568    JSLockHolder lock(vm);
     569   
     570    m_resultWrapper.clear();
     571    m_cursorWrapper.clear();
     572   
     573    WTF::switchOn(m_result,
     574        [] (RefPtr<IDBCursor>& cursor) { cursor->clearWrappers(); },
     575        [] (const auto&) { }
     576    );
     577}
     578
     579
    543580} // namespace WebCore
    544581

  • trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h

    r242818 r242986  
    7979    ExceptionOr<Result> result() const;
    8080    JSValueInWrappedObject& resultWrapper() { return m_resultWrapper; }
     81    JSValueInWrappedObject& cursorWrapper() { return m_cursorWrapper; }
    8182
    8283    using Source = Variant<RefPtr<IDBObjectStore>, RefPtr<IDBIndex>, RefPtr<IDBCursor>>;
    … …  
    166167    void onSuccess();
    167168
     169    void clearWrappers();
     170
    168171    IDBCursor* resultCursor();
    169172
    … …  
    172175
    173176    JSValueInWrappedObject m_resultWrapper;
     177    JSValueInWrappedObject m_cursorWrapper;
    174178    Result m_result;
    175179    Optional<Source> m_source;

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

    r241196 r242986  
    7676    auto& request = wrapped();
    7777    request.resultWrapper().visit(visitor);
     78    request.cursorWrapper().visit(visitor);
    7879}
    7980

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

    r228260 r242986  
    3737public:
    3838    JSValueInWrappedObject(JSC::JSValue = { });
     39    JSValueInWrappedObject(const JSValueInWrappedObject&);
    3940    operator JSC::JSValue() const;
    4041    explicit operator bool() const;
     42    JSValueInWrappedObject& operator=(const JSValueInWrappedObject& other);
    4143    void visit(JSC::SlotVisitor&) const;
     44    void clear();
    4245
    4346private:
    … …  
    6770
    6871inline JSValueInWrappedObject::JSValueInWrappedObject(JSC::JSValue value)
     72    : m_value(makeValue(JSC::JSValue(value)))
     73{
     74}
     75
     76inline JSValueInWrappedObject::JSValueInWrappedObject(const JSValueInWrappedObject& value)
    6977    : m_value(makeValue(value))
    7078{
    … …  
    8593}
    8694
     95inline JSValueInWrappedObject& JSValueInWrappedObject::operator=(const JSValueInWrappedObject& other)
     96{
     97    m_value = makeValue(JSC::JSValue(other));
     98    return *this;
     99}
     100
    87101inline void JSValueInWrappedObject::visit(JSC::SlotVisitor& visitor) const
    88102{
    … …  
    92106        visitor.append(value);
    93107    });
     108}
     109
     110inline void JSValueInWrappedObject::clear()
     111{
     112    WTF::switchOn(m_value, [] (Weak& value) {
     113        value.clear();
     114    }, [] (auto&) { });
    94115}
    95116
Note: See TracChangeset for help on using the changeset viewer.