Bug 35671 – [Qt] Even if QNetworkRequest::AlwaysCache is set on a request, a page will be loaded from the network the first time

• Component: New Bugs
• Status: RESOLVED FIXED

[Tor Arne Vestbø]

This bug report originated from issue QTBUG-4839
http://bugreports.qt.nokia.com/browse/QTBUG-4839

--- Description ---

The following example (whole example in the attachment) will load the page from the network even if QNetworkRequest::AlwaysCache is set. the documentation says "only load from cache, indicating error if the item was not cached (i.e., off-line mode)":

<div class="code panel" style="border-width: 1px;"><div class="codeContent panelContent">
<pre class="code-java">QWebPage *webPage = ui-&gt;webView-&gt;page();
QNetworkAccessManager *netAccessMgr = <span class="code-keyword">new</span> QNetworkAccessManager(<span class="code-keyword">this</span>);
QNetworkDiskCache *dc = <span class="code-keyword">new</span> QNetworkDiskCache(<span class="code-keyword">this</span>);
dc-&gt;setCacheDirectory(<span class="code-quote">"cache"</span>);
netAccessMgr-&gt;setCache(dc);
webPage-&gt;setNetworkAccessManager(netAccessMgr);

QNetworkRequest *request = <span class="code-keyword">new</span> QNetworkRequest(QUrl(ui-&gt;lineEdit-&gt;text()));
request-&gt;setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysCache);
ui-&gt;webView-&gt;load(*request);</pre>
</div></div>

--- Comments ---

Do we have an AlwaysCache autotest?

Yes, we do have a test. This is not a QNetworkAccessManager issue. It's a QWebFrame issue, that doesn't honour the attributes set in the request.

PS: your sample code leaks the QNetworkRequest. Don't use "new".

[Kent Hansen]

Reproduced with r55658.

[Jan Erik Hanssen]

Created attachment 75649 [details]
Patch

[Antonio Gomes]

Comment on attachment 75649 [details]
Patch

lgtm

[Eric Seidel (no email)]

Comment on attachment 75649 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75649&action=review

> WebKit/qt/Api/qwebframe.cpp:867
> +            switch (cacheLoadValue) {
> +            case QNetworkRequest::AlwaysNetwork:
> +                request.setCachePolicy(WebCore::ReloadIgnoringCacheData);
> +                break;
> +            case QNetworkRequest::PreferCache:
> +                request.setCachePolicy(WebCore::ReturnCacheDataElseLoad);
> +                break;
> +            case QNetworkRequest::AlwaysCache:
> +                request.setCachePolicy(WebCore::ReturnCacheDataDontLoad);
> +                break;
> +            case QNetworkRequest::PreferNetwork:
> +                request.setCachePolicy(WebCore::UseProtocolCachePolicy);
> +                break;

This would be much better as a helper function to convert the enum.  Then you only need one request.setCachePolicy call and this function gets much cleaner.

[Jan Erik Hanssen]

Created attachment 76261 [details]
Patch

[Andreas Kling]

Comment on attachment 76261 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=76261&action=review

LGTM, two small things:

> WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:3075
> +        : QNetworkAccessManager(parent), m_lastCacheLoad(QNetworkRequest::PreferNetwork)

Style, initializers should be on separate lines.

> WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:3083
> +            m_lastCacheLoad = static_cast<QNetworkRequest::CacheLoadControl>(cacheLoad.toInt());

You're using toInt() here and toUInt() in QWebFrame::load().

[Jan Erik Hanssen]

Created attachment 76305 [details]
Patch

[Jan Erik Hanssen]

(In reply to comment #6)
> (From update of attachment 76261 [details])
> > WebKit/qt/tests/qwebframe/tst_qwebframe.cpp:3083
> > +            m_lastCacheLoad = static_cast<QNetworkRequest::CacheLoadControl>(cacheLoad.toInt());
> 
> You're using toInt() here and toUInt() in QWebFrame::load().

Indeed, nice catch. I should have spotted that :)

[WebKit Review Bot]

Comment on attachment 76305 [details]
Patch

Clearing flags on attachment: 76305

Committed r73865: <http://trac.webkit.org/changeset/73865>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.