Summary: | Regression: WebPageProxy::exceededDatabaseQuota() needs to be serialized | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
Component: | WebCore Misc. | Assignee: | Mark Lam <mark.lam> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, ap, beidson, ggaren, sam, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mark Lam
2013-03-06 15:04:08 PST
Created attachment 191864 [details]
the patch.
Attachment 191864 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Platform/CoreIPC/HandleMessage.h', u'Source/WebKit2/UIProcess/WebPageProxy.cpp', u'Source/WebKit2/UIProcess/WebPageProxy.h', u'Source/WebKit2/UIProcess/WebPageProxy.messages.in']" exit_code: 1
Source/WebKit2/UIProcess/WebPageProxy.cpp:155: The parameter name "reply" adds no information, so it should be removed. [readability/parameter_name] [5]
Source/WebKit2/UIProcess/WebPageProxy.cpp:155: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Source/WebKit2/UIProcess/WebPageProxy.h:52: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 3 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 191864 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=191864&action=review >>> Source/WebKit2/UIProcess/WebPageProxy.cpp:155 >>> + PassRefPtr<Messages::WebPageProxy::ExceededDatabaseQuota::DelayedReply> reply); >> >> The parameter name "reply" adds no information, so it should be removed. [readability/parameter_name] [5] > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] I'll fix this indentation before landing. Comment on attachment 191864 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=191864&action=review > Source/WebKit2/ChangeLog:15 > + In webkit2, we can have multiple WebProcesses concurrently triggering a > + call to this function. While the function is waiting on feedback from a > + UI dialog, the wait loop may re-enter the function to service a second > + request to call this function from another WebProcess. This results in I'm not sure if I understand the mechanism of this. Is this a CoreIPC bug? (In reply to comment #5) > (From update of attachment 191864 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191864&action=review > > > Source/WebKit2/ChangeLog:15 > > + In webkit2, we can have multiple WebProcesses concurrently triggering a > > + call to this function. While the function is waiting on feedback from a > > + UI dialog, the wait loop may re-enter the function to service a second > > + request to call this function from another WebProcess. This results in > > I'm not sure if I understand the mechanism of this. Is this a CoreIPC bug? Alexey, to illustrate what went wrong in this bug, I've include below an excerpt of the backtrace that has the recursive calls into Connection::SyncMessageState::dispatchMessages() (and WebPageProxy::exceededDatabaseQuota()) before my fix is applied. Note: I added some blank lines and commentary in the middle to help highlight some points. Regarding whether it is a bug for the idle loop to service more messages from CoreIPC or not (while waiting for user input on a modal dialog), I'm not the best person to make that determination. Maybe Anders can chime in. === BEGIN backtrace =================== (gdb) bt ... // Cut out the higher levels of recursions for brevity. Just showing one level of recursion below. // Here's the 2nd time we invoke the "exceeded quota" modal dialog on this stack. We shouldn't be doing this. #216 0x00007fff96136c8b in -[NSApplication runModalForWindow:] () // 2nd time in exceededDatabaseQuota: #217 0x000000010dcb0665 in Safari::BrowserContentViewController::exceededDatabaseQuota (this=0x7fb6a24c17d0, originDisplayName=@0x7fff52085cf0, requestingURL=@0x7fff52085ce8, databaseIdentifier=@0x7fff52085cc8, databaseDisplayName=@0x7fff52085cc0, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserContentViewController.mm:2501 #218 0x000000010dd24b41 in Safari::BrowserPageUIClient::exceededDatabaseQuota (this=0x7fb6a244ae10, frame=@0x7fff52085dc8, origin=@0x7fff52085dc0, databaseName=@0x7fff52085db8, displayName=@0x7fff52085db0, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserPageUIClient.mm:539 #219 0x000000010e072e97 in Safari::WK::exceededDatabaseQuota (page=0x7fb6a28e3400, frame=0x7fb6a3c2a130, origin=0x7fb6a7101c40, databaseName=0x7fb6a2468020, displayName=0x7fb6a7165f10, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1, clientInfo=0x7fb6a244ae10) at /Volumes/Source/ws7/Internal/Safari/Wrappers/WK/PageUIClient.cpp:345 #220 0x00000001115b1bd4 in WebKit::WebUIClient::exceededDatabaseQuota (this=0x7fb6a28e3598, page=0x7fb6a28e3400, frame=0x7fb6a3c2a130, origin=0x7fb6a7101c40, databaseName=@0x7fff52086168, databaseDisplayName=@0x7fff52086170, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebUIClient.cpp:324 #221 0x00000001114e8155 in WebKit::WebPageProxy::exceededDatabaseQuota (this=0x7fb6a28e3400, frameID=1, originIdentifier=@0x7fff52086160, databaseName=@0x7fff52086168, displayName=@0x7fff52086170, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1, newQuota=@0x7fff52086148) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebPageProxy.cpp:3945 #222 0x0000000111539117 in CoreIPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long&), unsigned long long, WTF::String, WTF::String, WTF::String, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long> (args=@0x7fff52086158, replyArgs=@0x7fff52086148, object=0x7fb6a28e3400, function={ptr = 4585324400, ptr = 0}) at HandleMessage.h:173 #223 0x000000011153740e in CoreIPC::handleMessage<Messages::WebPageProxy::ExceededDatabaseQuota, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long&)> (decoder=@0x7fb6a7005370, replyEncoder=@0x7fb6a713bc40, object=0x7fb6a28e3400, function={ptr = 4585324400, ptr = 0}) at HandleMessage.h:345 #224 0x0000000111531044 in WebKit::WebPageProxy::didReceiveSyncMessage (this=0x7fb6a28e3400, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/WebKitBuild/Debug/DerivedSources/WebKit2/WebPageProxyMessageReceiver.cpp:816 #225 0x0000000111531aaf in non-virtual thunk to WebKit::WebPageProxy::didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, WTF::OwnPtr<CoreIPC::MessageEncoder>&) (this=0x7fb6a28e3418, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/WebKitBuild/Debug/DerivedSources/WebKit2/WebPageProxyMessageReceiver.cpp:894 #226 0x0000000111265512 in CoreIPC::MessageReceiverMap::dispatchSyncMessage (this=0x7fb6a3910b30, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/MessageReceiverMap.cpp:103 #227 0x00000001111b917f in WebKit::ChildProcessProxy::dispatchSyncMessage (this=0x7fb6a3910af0, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Shared/ChildProcessProxy.cpp:112 #228 0x000000011158e1f2 in WebKit::WebProcessProxy::didReceiveSyncMessage (this=0x7fb6a3910af0, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebProcessProxy.cpp:378 #229 0x000000011158e2ef in non-virtual thunk to WebKit::WebProcessProxy::didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, WTF::OwnPtr<CoreIPC::MessageEncoder>&) (this=0x7fb6a3910af8, connection=0x7fb6a24db870, decoder=@0x7fb6a7005370, replyEncoder=@0x7fff52086f80) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebProcessProxy.cpp:390 // 2nd time in dispatchMessages: #230 0x00000001111c1011 in CoreIPC::Connection::dispatchSyncMessage (this=0x7fb6a24db870, decoder=@0x7fb6a7005370) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:728 #231 0x00000001111bd8f0 in CoreIPC::Connection::dispatchMessage (this=0x7fb6a24db870, incomingMessage=<value temporarily unavailable, due to optimizations>) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:778 #232 0x00000001111bd7b3 in CoreIPC::Connection::SyncMessageState::dispatchMessages (this=0x7fb6a24ab910, allowedConnection=0x7fb6a24db870) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:187 #233 0x00000001111bd63e in CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection (this=0x7fb6a24ab910, connection=0x7fb6a24db870) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:204 #234 0x00000001111d27ca in WTF::FunctionWrapper<void (CoreIPC::Connection::SyncMessageState::*)(CoreIPC::Connection*)>::operator() (this=0x7fb6a2659cb0, c=0x7fb6a24ab910, p1=0x7fb6a24db870) at Functional.h:246 #235 0x00000001111d273c in WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::SyncMessageState::*)(CoreIPC::Connection*)>, void ()(CoreIPC::Connection::SyncMessageState*, WTF::RefPtr<CoreIPC::Connection>)>::operator() (this=0x7fb6a2659ca0) at Functional.h:522 #236 0x0000000113e93d6e in WTF::Function<void ()()>::operator() (this=0x7fff52087220) at Functional.h:704 #237 0x0000000113e93a1f in WebCore::RunLoop::performWork (this=0x7fb6a260a4c0) at /Volumes/Source/ws7/OpenSource/Source/WebCore/platform/RunLoop.cpp:91 #238 0x0000000113e94dde in WebCore::RunLoop::performWork (context=0x7fb6a260a4c0) at /Volumes/Source/ws7/OpenSource/Source/WebCore/platform/cf/RunLoopCF.cpp:66 // 2nd time in idle wait loop waiting for user to click on a button on the modal dialog, but another sync message is in the queue to be processed (see above): #239 0x00007fff8fae31b1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ () #240 0x00007fff8fae2b16 in __CFRunLoopDoSources0 () #241 0x00007fff8fb00b7b in __CFRunLoopRun () #242 0x00007fff8fb000d9 in CFRunLoopRunSpecific () #243 0x00007fff8ba0b7d5 in RunCurrentEventLoopInMode () #244 0x00007fff8ba0b478 in ReceiveNextEventCommon () #245 0x00007fff8bb1dc2b in _BlockUntilNextEventMatchingListInModeWithFilter () #246 0x00007fff95f0471f in _DPSNextEvent () #247 0x00007fff95f03d4e in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #248 0x000000010dc69b13 in -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x7fb6a260f780, _cmd=0x7fff967b03cf, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff7a333610, dequeue=1 '\001') at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserApplication.mm:365 #249 0x00007fff961369af in -[NSApplication _realDoModalLoop:peek:] () // 1st time invoking the "quota exceeded" modal dialog: #250 0x00007fff96136c8b in -[NSApplication runModalForWindow:] () // 1st time in exceededDatabaseQuota: #251 0x000000010dcb0665 in Safari::BrowserContentViewController::exceededDatabaseQuota (this=0x7fb6a3c12da0, originDisplayName=@0x7fff52088da0, requestingURL=@0x7fff52088d98, databaseIdentifier=@0x7fff52088d78, databaseDisplayName=@0x7fff52088d70, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserContentViewController.mm:2501 #252 0x000000010dd24b41 in Safari::BrowserPageUIClient::exceededDatabaseQuota (this=0x7fb6a3c0e920, frame=@0x7fff52088e78, origin=@0x7fff52088e70, databaseName=@0x7fff52088e68, displayName=@0x7fff52088e60, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserPageUIClient.mm:539 #253 0x000000010e072e97 in Safari::WK::exceededDatabaseQuota (page=0x7fb6a6814400, frame=0x7fb6a3c1de70, origin=0x7fb6a71281b0, databaseName=0x7fb6a7154610, displayName=0x7fb6a71307d0, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1, clientInfo=0x7fb6a3c0e920) at /Volumes/Source/ws7/Internal/Safari/Wrappers/WK/PageUIClient.cpp:345 #254 0x00000001115b1bd4 in WebKit::WebUIClient::exceededDatabaseQuota (this=0x7fb6a6814598, page=0x7fb6a6814400, frame=0x7fb6a3c1de70, origin=0x7fb6a71281b0, databaseName=@0x7fff52089218, databaseDisplayName=@0x7fff52089220, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebUIClient.cpp:324 #255 0x00000001114e8155 in WebKit::WebPageProxy::exceededDatabaseQuota (this=0x7fb6a6814400, frameID=1, originIdentifier=@0x7fff52089210, databaseName=@0x7fff52089218, displayName=@0x7fff52089220, currentQuota=5000000, currentOriginUsage=5001216, currentDatabaseUsage=0, expectedUsage=1, newQuota=@0x7fff520891f8) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebPageProxy.cpp:3945 #256 0x0000000111539117 in CoreIPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long&), unsigned long long, WTF::String, WTF::String, WTF::String, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long> (args=@0x7fff52089208, replyArgs=@0x7fff520891f8, object=0x7fb6a6814400, function={ptr = 4585324400, ptr = 0}) at HandleMessage.h:173 #257 0x000000011153740e in CoreIPC::handleMessage<Messages::WebPageProxy::ExceededDatabaseQuota, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(unsigned long long, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long long, unsigned long long, unsigned long long, unsigned long long, unsigned long long&)> (decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fb6a714c250, object=0x7fb6a6814400, function={ptr = 4585324400, ptr = 0}) at HandleMessage.h:345 #258 0x0000000111531044 in WebKit::WebPageProxy::didReceiveSyncMessage (this=0x7fb6a6814400, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/WebKitBuild/Debug/DerivedSources/WebKit2/WebPageProxyMessageReceiver.cpp:816 #259 0x0000000111531aaf in non-virtual thunk to WebKit::WebPageProxy::didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, WTF::OwnPtr<CoreIPC::MessageEncoder>&) (this=0x7fb6a6814418, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/WebKitBuild/Debug/DerivedSources/WebKit2/WebPageProxyMessageReceiver.cpp:894 #260 0x0000000111265512 in CoreIPC::MessageReceiverMap::dispatchSyncMessage (this=0x7fb6a3c10080, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/MessageReceiverMap.cpp:103 #261 0x00000001111b917f in WebKit::ChildProcessProxy::dispatchSyncMessage (this=0x7fb6a3c10040, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Shared/ChildProcessProxy.cpp:112 #262 0x000000011158e1f2 in WebKit::WebProcessProxy::didReceiveSyncMessage (this=0x7fb6a3c10040, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebProcessProxy.cpp:378 #263 0x000000011158e2ef in non-virtual thunk to WebKit::WebProcessProxy::didReceiveSyncMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&, WTF::OwnPtr<CoreIPC::MessageEncoder>&) (this=0x7fb6a3c10048, connection=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120, replyEncoder=@0x7fff5208a030) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/UIProcess/WebProcessProxy.cpp:390 // 1st time in dispatchMessages: #264 0x00000001111c1011 in CoreIPC::Connection::dispatchSyncMessage (this=0x7fb6a3a17e10, decoder=@0x7fb6a3b3a120) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:728 #265 0x00000001111bd8f0 in CoreIPC::Connection::dispatchMessage (this=0x7fb6a3a17e10, incomingMessage=<value temporarily unavailable, due to optimizations>) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:778 #266 0x00000001111bd7b3 in CoreIPC::Connection::SyncMessageState::dispatchMessages (this=0x7fb6a24ab910, allowedConnection=0x7fb6a3a17e10) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:187 #267 0x00000001111bd63e in CoreIPC::Connection::SyncMessageState::dispatchMessageAndResetDidScheduleDispatchMessagesForConnection (this=0x7fb6a24ab910, connection=0x7fb6a3a17e10) at /Volumes/Source/ws7/OpenSource/Source/WebKit2/Platform/CoreIPC/Connection.cpp:204 #268 0x00000001111d27ca in WTF::FunctionWrapper<void (CoreIPC::Connection::SyncMessageState::*)(CoreIPC::Connection*)>::operator() (this=0x7fb6a3b3a2b0, c=0x7fb6a24ab910, p1=0x7fb6a3a17e10) at Functional.h:246 #269 0x00000001111d273c in WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::SyncMessageState::*)(CoreIPC::Connection*)>, void ()(CoreIPC::Connection::SyncMessageState*, WTF::RefPtr<CoreIPC::Connection>)>::operator() (this=0x7fb6a3b3a2a0) at Functional.h:522 #270 0x0000000113e93d6e in WTF::Function<void ()()>::operator() (this=0x7fff5208a2d0) at Functional.h:704 #271 0x0000000113e93a1f in WebCore::RunLoop::performWork (this=0x7fb6a260a4c0) at /Volumes/Source/ws7/OpenSource/Source/WebCore/platform/RunLoop.cpp:91 #272 0x0000000113e94dde in WebCore::RunLoop::performWork (context=0x7fb6a260a4c0) at /Volumes/Source/ws7/OpenSource/Source/WebCore/platform/cf/RunLoopCF.cpp:66 // 1st time in idle wait loop: #273 0x00007fff8fae31b1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ () #274 0x00007fff8fae2b16 in __CFRunLoopDoSources0 () #275 0x00007fff8fb00b7b in __CFRunLoopRun () #276 0x00007fff8fb000d9 in CFRunLoopRunSpecific () #277 0x00007fff8ba0b7d5 in RunCurrentEventLoopInMode () #278 0x00007fff8ba0b576 in ReceiveNextEventCommon () #279 0x00007fff8bb1dc2b in _BlockUntilNextEventMatchingListInModeWithFilter () #280 0x00007fff95f0471f in _DPSNextEvent () #281 0x00007fff95f03d4e in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #282 0x000000010dc69b13 in -[BrowserApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x7fb6a260f780, _cmd=0x7fff967b03cf, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff7a333610, dequeue=1 '\001') at /Volumes/Source/ws7/Internal/Safari/Basics/BrowserApplication.mm:365 #283 0x00007fff95efbe8c in -[NSApplication run] () #284 0x00007fff95ea5d16 in NSApplicationMain () #285 0x000000010e0ef666 in SafariMain (argc=1, argv=0x7fff5208ba98) at /Volumes/Source/ws7/Internal/Safari/Basics/SafariMain.mm:52 #286 0x000000010db74f42 in main (argc=1, argv=0x7fff5208ba98) at /Volumes/Source/ws7/Internal/Safari/Basics/main.c:13 (gdb) === END backtrace =================== It seems wrong to me that CoreIPC messages are being handled under a modal dialog, and perhaps that's what we should fix instead. Anders would know for sure. Comment on attachment 191864 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=191864&action=review > Source/WebKit2/UIProcess/WebPageProxy.cpp:150 > + static ExceededDatabaseQuotaRecords& getRecords(); Our idiomatic name for a singleton is "shared()". Please remember that the "get" prefix is reserved for out parameters. > Source/WebKit2/UIProcess/WebPageProxy.cpp:176 > + static ExceededDatabaseQuotaRecords* records = 0; > + // FIXME: The following is vulnerable to a race between threads. Need to > + // implement a thread safe on-first-use static initializer. > + if (!records) > + records = new ExceededDatabaseQuotaRecords(); > + > + return *records; Please use DEFINE_STATIC_LOCAL. > Source/WebKit2/UIProcess/WebPageProxy.cpp:179 > +void ExceededDatabaseQuotaRecords::add(uint64_t frameID, String originIdentifier, This function should take a PassOwnPtr<Record> as its argument. > Source/WebKit2/UIProcess/WebPageProxy.cpp:198 > +ExceededDatabaseQuotaRecords::Record* ExceededDatabaseQuotaRecords::getNext() This should be named "next()". Please remember that the get prefix is reserved for out parameters. > Source/WebKit2/UIProcess/WebPageProxy.cpp:203 > + else > + m_currentRecord.clear(); I think this would be clearer if you started by unconditionally clearing m_currentRecord. Created attachment 192065 [details]
updated patch.
Addressed Geoff's comments.
Attachment 192065 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Platform/CoreIPC/HandleMessage.h', u'Source/WebKit2/UIProcess/WebPageProxy.cpp', u'Source/WebKit2/UIProcess/WebPageProxy.h', u'Source/WebKit2/UIProcess/WebPageProxy.messages.in']" exit_code: 1 Source/WebKit2/UIProcess/WebPageProxy.cpp:180: Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Source/WebKit2/UIProcess/WebPageProxy.cpp:4012: Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] Source/WebKit2/UIProcess/WebPageProxy.h:52: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 3 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style. Comment on attachment 192065 [details] updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=192065&action=review >> Source/WebKit2/UIProcess/WebPageProxy.cpp:180 >> + PassOwnPtr<Record> record = adoptPtr(new Record); > > Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] I'll change this to an OwnPtr and release it on return below. >> Source/WebKit2/UIProcess/WebPageProxy.cpp:4012 >> + PassOwnPtr<ExceededDatabaseQuotaRecords::Record> newRecord = records.createRecord(frameID, > > Local variables should never be PassOwnPtr (see http://webkit.org/coding/RefPtr.html). [readability/pass_ptr] [5] I'll change this to an OwnPtr and release it to add() below. Comment on attachment 192065 [details] updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=192065&action=review r=me with the changes you mentioned. > Source/WebKit2/UIProcess/WebPageProxy.cpp:175 > +PassOwnPtr<ExceededDatabaseQuotaRecords::Record> ExceededDatabaseQuotaRecords::createRecord( Usually we put the 'create' function inside the class. In this case, it would be Record::create. Thanks for the review. Landed in r145164: <http://trac.webkit.org/changeset/145164>. |