Bug 118584

Summary: [Application Cache] Assert failed if ApplicationCacheStorage::setCacheDirectory is called more than once.
Product: WebKit Reporter: Donggwan Kim <donggwan.kim>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: ap, commit-queue, dw.im, japhet
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: Linux   
Attachments:
Description Flags
Patch ap: review-, ap: commit-queue-

Description Donggwan Kim 2013-07-11 23:30:22 PDT
ApplicationCacheStorage::setCacheDirectory could be called more than once.
So below assert sentence should be removed.
ASSERT(m_cacheDirectory.isNull());
Comment 1 Donggwan Kim 2013-07-11 23:36:55 PDT
Created attachment 206508 [details]
Patch
Comment 2 Alexey Proskuryakov 2013-07-12 13:36:13 PDT
Can you please explain why it can be called multiple times? The assertion checks for exactly the opposite, that this shouldn't happen.
Comment 3 Donggwan Kim 2013-07-14 20:34:46 PDT
I think following cases could be happened

1. when storeCopyOfCache is called for some reason after setCacheDirectory was called to initialize default cache directory by application
2. For test, it could be called more than once. For example, it was called two times in test_ewk_setting.cpp of Efl port.

In my humble opinion, there is no reason for limiting setCacheDirectory force to be called only one time. In other cases, (e.g. SQLDatabase, Local Storage ...) there are no limitation like this.

So i think it should be removed.

If you have any other opinion, please let me know.
Comment 4 Alexey Proskuryakov 2013-07-16 09:44:34 PDT
Comment on attachment 206508 [details]
Patch

If we are to allow changing directory on the fly, then we should ensure that on-disk database state doesn't get out of sync. So, we should either close it and open one from the new directory, or make this function assert when called in a situation where safe change is not possible.
Comment 5 Donggwan Kim 2013-07-16 18:20:56 PDT
Ok. I understood what you mean. I will close this patch as Won't fix.
Comment 6 Donggwan Kim 2013-07-16 18:22:03 PDT
I think INVALID is more reasonable to this patch