Bug 207137

Summary: Crash in WebCore::IDBServer::IDBServer::createIndex
Product: WebKit Reporter: Sihui Liu <sihui_liu>
Component: New BugsAssignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, beidson, cdumez, commit-queue, darin, ews-watchlist, jsbell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Sihui Liu 2020-02-03 11:54:14 PST
0   com.apple.WebCore       WTF::StringImpl::ref() + 12 (StringImpl.h:1098)
1   com.apple.WebCore       void WTF::refIfNotNull<WTF::StringImpl>(WTF::StringImpl*) + 46 (RefPtr.h:39)
2   com.apple.WebCore       WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::RefPtr(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> > const&) + 46 (RefPtr.h:59)
3   com.apple.WebCore       WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::RefPtr(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> > const&) + 29 (RefPtr.h:59)
4   com.apple.WebCore       WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::operator=(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> > const&) + 37
5   com.apple.WebCore       WTF::String::operator=(WTF::String const&) + 36 (WTFString.h:134)
6   com.apple.WebCore       void WTF::__copy_assign_op_table<WTF::Variant<WTF::String, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> >, WTF::__index_sequence<0l, 1l> >::__copy_assign_func<0l>(WTF::Variant<WTF::String, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> >*, WTF::Variant<WTF::String, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> > const&) + 50 (Variant.h:800)
7   com.apple.WebCore       WTF::Variant<WTF::String, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> >::operator=(WTF::Variant<WTF::String, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> > const&) + 125 (Variant.h:1633)
8   com.apple.WebCore       WebCore::IDBIndexInfo::operator=(WebCore::IDBIndexInfo const&) + 111 (IDBIndexInfo.h:35)
9   com.apple.WebCore       void WTF::GenericHashTraits<WebCore::IDBIndexInfo>::assignToEmpty<WebCore::IDBIndexInfo, WebCore::IDBIndexInfo const&>(WebCore::IDBIndexInfo&, WebCore::IDBIndexInfo const&) + 37 (HashTraits.h:73)
10  com.apple.WebCore       void WTF::HashMapTranslator<WTF::HashMap<unsigned long long, WebCore::IDBIndexInfo, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebCore::IDBIndexInfo> >::KeyValuePairTraits, WTF::IntHash<unsigned long long> >::translate<WTF::KeyValuePair<unsigned long long, WebCore::IDBIndexInfo>, unsigned long long, WebCore::IDBIndexInfo const&>(WTF::KeyValuePair<unsigned long long, WebCore::IDBIndexInfo>&, unsigned long long&&, WebCore::IDBIndexInfo const&) + 87 (HashMap.h:210)
...
16  com.apple.WebCore       WebCore::IDBServer::SQLiteIDBBackingStore::createIndex(WebCore::IDBResourceIdentifier const&, WebCore::IDBIndexInfo const&) + 3659 (SQLiteIDBBackingStore.cpp:1385)
17  com.apple.WebCore       WebCore::IDBServer::UniqueIDBDatabase::createIndex(WebCore::IDBServer::UniqueIDBDatabaseTransaction&, WebCore::IDBIndexInfo const&, WTF::Function<void (WebCore::IDBError const&)>) + 491 (UniqueIDBDatabase.cpp:631)
18  com.apple.WebCore       WebCore::IDBServer::UniqueIDBDatabaseTransaction::createIndex(WebCore::IDBRequestData const&, WebCore::IDBIndexInfo const&) + 523 (UniqueIDBDatabaseTransaction.cpp:202)
19  com.apple.WebCore       WebCore::IDBServer::IDBServer::createIndex(WebCore::IDBRequestData const&, WebCore::IDBIndexInfo const&) + 429 (IDBServer.cpp:268)
20  com.apple.WebKitLegacy  InProcessIDBServer::createIndex(WebCore::IDBRequestData const&, WebCore::IDBIndexInfo const&)::$_30::operator()() const + 116 (InProcessIDBServer.cpp:328)
21  com.apple.WebKitLegacy  WTF::Detail::CallableWrapper<InProcessIDBServer::createIndex(WebCore::IDBRequestData const&, WebCore::IDBIndexInfo const&)::$_30, void>::call() + 30 (Function.h:52)
Comment 1 Sihui Liu 2020-02-03 11:54:55 PST
<rdar://problem/59096231>
Comment 2 Sihui Liu 2020-02-03 11:56:35 PST
Created attachment 389544 [details]
Patch
Comment 3 Chris Dumez 2020-02-03 12:48:57 PST
That's a lot of red bubbles you got there :)
Comment 4 Darin Adler 2020-02-03 14:29:37 PST
Comment on attachment 389544 [details]
Patch

Need to add WEBCORE_EXPORT to the function.
Comment 5 Sihui Liu 2020-02-03 15:27:38 PST
Created attachment 389581 [details]
Patch
Comment 6 Darin Adler 2020-02-03 15:47:08 PST
Comment on attachment 389581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=389581&action=review

> Source/WebCore/ChangeLog:11
> +        No behavior change.
> +
> +        * Modules/indexeddb/shared/IDBIndexInfo.h:

This is not a great way to write the change log. Ideally the comment on the file would say something like "Export IDBIndexInfo::isolatedCopy so it can be used in WebKitLegacy framework code."

Replacing the line about testing with "No behavior change" is also not great. Just deleting it is better. After all, the patch does create a behavior change, although it’s the part of the change in WebKitLegacy rather than the part in WebCore, so it’s not accurate.

> Source/WebKitLegacy/ChangeLog:9
> +        Create an isolated copy of IDBIndexInfo before passing it to IDB thread.

Seems like this comment should be on the function line rather than up here.
Comment 7 Sihui Liu 2020-02-03 15:52:24 PST
(In reply to Darin Adler from comment #6)
> Comment on attachment 389581 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389581&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        No behavior change.
> > +
> > +        * Modules/indexeddb/shared/IDBIndexInfo.h:
> 
> This is not a great way to write the change log. Ideally the comment on the
> file would say something like "Export IDBIndexInfo::isolatedCopy so it can
> be used in WebKitLegacy framework code."
> 
> Replacing the line about testing with "No behavior change" is also not
> great. Just deleting it is better. After all, the patch does create a
> behavior change, although it’s the part of the change in WebKitLegacy rather
> than the part in WebCore, so it’s not accurate.
> 
> > Source/WebKitLegacy/ChangeLog:9
> > +        Create an isolated copy of IDBIndexInfo before passing it to IDB thread.
> 
> Seems like this comment should be on the function line rather than up here.

Will change. Thanks for the review!
Comment 8 Sihui Liu 2020-02-03 15:55:00 PST
Created attachment 389585 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2020-02-03 16:31:07 PST
Comment on attachment 389585 [details]
Patch for landing

Clearing flags on attachment: 389585

Committed r255615: <https://trac.webkit.org/changeset/255615>
Comment 10 WebKit Commit Bot 2020-02-03 16:31:09 PST
All reviewed patches have been landed.  Closing bug.