Bug 207137 - Crash in WebCore::IDBServer::IDBServer::createIndex
Summary: Crash in WebCore::IDBServer::IDBServer::createIndex
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-03 11:54 PST by Sihui Liu
Modified: 2020-02-03 16:31 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.69 KB, patch)
2020-02-03 11:56 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (3.10 KB, patch)
2020-02-03 15:27 PST, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch for landing (3.15 KB, patch)
2020-02-03 15:55 PST, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.