Bug 34713 – [Qt] appcache is disabled and LayoutTests/http/tests/appcache are skipped on QtLinux

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Chang Shu]

Application cache is mostly working now. We should enable the feature and run the test cases.

[Chang Shu]

Created attachment 48346 [details]
fix patch

[Eric Seidel (no email)]

The Mac EWS failed to process this patch due to bug 34717.

[Laszlo Gombos]

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.

[Chang Shu]

Created attachment 48361 [details]
patch after Laszlo's comments

[Laszlo Gombos]

Comment on attachment 48361 [details]
patch after Laszlo's comments

LGTM; r+.

[WebKit Commit Bot]

Comment on attachment 48361 [details]
patch after Laszlo's comments

Rejecting patch 48361 from commit-queue.

[e-mail] 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 [e-mail] to request a commit-queue restart.  After restart the commit-queue will correctly respect your committer rights.

[Eric Seidel (no email)]

This rejection is due to bug 34717.  Either you'll need to change the case on your bugzilla account or committers.py for now.

[WebKit Commit Bot]

Comment on attachment 48361 [details]
patch after Laszlo's comments

Clearing flags on attachment: 48361

Committed r54543: <http://trac.webkit.org/changeset/54543>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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.

[Chang Shu]

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

[Chang Shu]

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

[Laszlo Gombos]

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.

[Chang Shu]

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

[Simon Hausmann]

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.