Bug 34713 - [Qt] appcache is disabled and LayoutTests/http/tests/appcache are skipped on QtLinux
Summary: [Qt] appcache is disabled and LayoutTests/http/tests/appcache are skipped on ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Chang Shu
URL:
Keywords:
Depends on:
Blocks: 34712
  Show dependency treegraph
 
Reported: 2010-02-08 11:16 PST by Chang Shu
Modified: 2010-02-15 07:00 PST (History)
6 users (show)

See Also:


Attachments
fix patch (3.60 KB, patch)
2010-02-08 11:30 PST, Chang Shu
no flags Details | Formatted Diff | Diff
patch after Laszlo's comments (3.78 KB, patch)
2010-02-08 13:26 PST, Chang Shu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chang Shu 2010-02-08 11:16:18 PST
Application cache is mostly working now. We should enable the feature and run the test cases.
Comment 1 Chang Shu 2010-02-08 11:30:30 PST
Created attachment 48346 [details]
fix patch
Comment 2 Eric Seidel (no email) 2010-02-08 12:42:06 PST
The Mac EWS failed to process this patch due to bug 34717.
Comment 3 Laszlo Gombos 2010-02-08 12:53:57 PST
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.
Comment 4 Chang Shu 2010-02-08 13:26:52 PST
Created attachment 48361 [details]
patch after Laszlo's comments
Comment 5 Laszlo Gombos 2010-02-08 14:29:19 PST
Comment on attachment 48361 [details]
patch after Laszlo's comments

LGTM; r+.
Comment 6 WebKit Commit Bot 2010-02-08 17:09:20 PST
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.
Comment 7 Eric Seidel (no email) 2010-02-08 17:28:15 PST
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 8 WebKit Commit Bot 2010-02-09 02:40:29 PST
Comment on attachment 48361 [details]
patch after Laszlo's comments

Clearing flags on attachment: 48361

Committed r54543: <http://trac.webkit.org/changeset/54543>
Comment 9 WebKit Commit Bot 2010-02-09 02:40:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Csaba Osztrogonác 2010-02-09 04:30:51 PST
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
Comment 11 Csaba Osztrogonác 2010-02-09 16:17:16 PST
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.
Comment 12 Chang Shu 2010-02-10 22:18:35 PST
(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.
Comment 13 Chang Shu 2010-02-10 22:21:12 PST
(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.
Comment 14 Laszlo Gombos 2010-02-14 08:34:58 PST
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.
Comment 15 Chang Shu 2010-02-14 17:09:29 PST
(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!
Comment 16 Simon Hausmann 2010-02-15 07:00:25 PST
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.