Bug 111631 – Regression: WebPageProxy::exceededDatabaseQuota() needs to be serialized

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mark Lam]

In webkit1, WebPageProxy::exceededDatabaseQuota() will only be called in a serial fashion.  Hence, if we have more than one webpage needing to put up the "quota exceeded" dialog, then only one of them will be put up at a time..  Subsequent ones will wait till the first one returns before proceeding.

With webkit2, there is now nothing to serialize the requests for putting up the "quota exceeded" dialog.  This patch will fix that.

[Mark Lam]

<rdar://problem/13354445>.

[Mark Lam]

Created attachment 191864 [details]
the patch.

[WebKit Review Bot]

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.

[Mark Lam]

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.

[Alexey Proskuryakov]

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?

[Mark Lam]

(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 ===================

[Alexey Proskuryakov]

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.

[Geoffrey Garen]

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.

[Mark Lam]

Created attachment 192065 [details]
updated patch.

Addressed Geoff's comments.

[WebKit Review Bot]

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.

[Mark Lam]

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.

[Geoffrey Garen]

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.

[Mark Lam]

Thanks for the review.  Landed in r145164: <http://trac.webkit.org/changeset/145164>.