Application cache is mostly working now. We should enable the feature and run the test cases.
Created attachment 48346 [details] fix patch
The Mac EWS failed to process this patch due to bug 34717.
Should the QWebSettings::setOfflineWebApplicationCachePath(..) call be moved to in WebKitTools/DumpRenderTree/qt/main.cpp's main function instead of the WebPage() contructor ? Otherwise it looks good to me.
Created attachment 48361 [details] patch after Laszlo's comments
Comment on attachment 48361 [details] patch after Laszlo's comments LGTM; r+.
Comment on attachment 48361 [details] patch after Laszlo's comments Rejecting patch 48361 from commit-queue. Chang.Shu@nokia.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/committers.py by adding yourself to the file (no review needed). Due to bug 30084 the commit-queue will require a restart after your change. Please contact eseidel@chromium.org to request a commit-queue restart. After restart the commit-queue will correctly respect your committer rights.
This rejection is due to bug 34717. Either you'll need to change the case on your bugzilla account or committers.py for now.
Comment on attachment 48361 [details] patch after Laszlo's comments Clearing flags on attachment: 48361 Committed r54543: <http://trac.webkit.org/changeset/54543>
All reviewed patches have been landed. Closing bug.
This patch broke DRT in debug mode (32 and 64 bit) gdb backtrace: 0x00007fca7d7b9b38 in WebCore::ApplicationCacheStorage::setCacheDirectory (this=0x2563090, cacheDirectory=@0x7fff693d99a0) at ../../../WebCore/loader/appcache/ApplicationCacheStorage.cpp:354 354 ASSERT(m_cacheDirectory.isNull()); (gdb) bt #0 0x00007fca7d7b9b38 in WebCore::ApplicationCacheStorage::setCacheDirectory (this=0x2563090, cacheDirectory=@0x7fff693d99a0) at ../../../WebCore/loader/appcache/ApplicationCacheStorage.cpp:354 #1 0x00007fca7d572885 in QWebSettings::setOfflineWebApplicationCachePath (path=@0x7fff693d9ae0) at ../../../WebKit/qt/Api/qwebsettings.cpp:868 #2 0x000000000042ec32 in main (argc=2, argv=0x7fff693d9cc8) at /home/oszi/webkit/WebKitTools/DumpRenderTree/qt/main.cpp:166
Sorry guys, I had to roll it out ( http://trac.webkit.org/changeset/54573 ) because folks need working tree. You can easily roll in again after fix the bug.
(In reply to comment #11) > Sorry guys, I had to roll it out ( http://trac.webkit.org/changeset/54573 ) > because folks need working tree. You can easily roll in again after fix the > bug. Thanks a lot for doing this, indeed. I am out of town right now and will have this fixed as soon as possible.
(In reply to comment #7) > This rejection is due to bug 34717. Either you'll need to change the case on > your bugzilla account or committers.py for now. I will have to update the python file. Bugzilla failed to change my email to small case as it thought the account already exist.
http://trac.webkit.org/changeset/54757 re-landed r54543 without the change in DumpRenderTree/qt/main.cpp. Persistent storage for AppCache is already initialized in DumpRenderTreeQt.cpp.
(In reply to comment #14) > http://trac.webkit.org/changeset/54757 re-landed r54543 without the change in > DumpRenderTree/qt/main.cpp. > > Persistent storage for AppCache is already initialized in > DumpRenderTreeQt.cpp. Thanks, Laszlo, I was about to land the new patch without main.cpp change and found you already did it for me. Great!
It's still a bug that calling QWebSettings::setOfflineWebApplicationCachePath() multiple times causes a failing assertion. At least if the path is already set, then the Qt API on top of WebCore should print a qWarning() that it can't be set again instead of crashing/asserting.