Bug 26848 – [Qt] ResourceHandle::willLoadFromCache needs to be implmeneted QtWebKit.

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

[Yongjun Zhang]

When disk cache is enabled, WebCore can figure out if the next main load could be loaded from cache by calling willLoadFromCache when do form repost (FrameLoader.cpp:4530).  willLoadFromCache in QtWebKit is currently not implemented and always returns false.

[Yongjun Zhang]

Created attachment 32079 [details]
Implement willLoadFromCache for QtWebKit.

Implement ResouceHandler::willLoadFromCache in QtWebKit, add Frame* argument to generic willLoadFromCache because it is needed to figure out the right QNetworkAccessManager from QWebPage.

[Eric Seidel (no email)]

Comment on attachment 32079 [details]
Implement willLoadFromCache for QtWebKit.

Why does Qt need the frame and no other port does?

Isn't there a global "network manager"?
+    QNetworkAccessManager* manager = QWebFramePrivate::kit(frame)->page()->networkAccessManager();

[Simon Hausmann]

(In reply to comment #2)
> (From update of attachment 32079 [details] [review])
> Why does Qt need the frame and no other port does?
> 
> Isn't there a global "network manager"?
> +    QNetworkAccessManager* manager =
> QWebFramePrivate::kit(frame)->page()->networkAccessManager();
> 

I think the Qt port is the only one that allows that kind of sandboxing, where there is a per page (or page group) specific network manager and similar things. From an application developer's point of view that's very convenient.

[Holger Freyther]

Comment on attachment 32079 [details]
Implement willLoadFromCache for QtWebKit.


> -bool ResourceHandle::willLoadFromCache(ResourceRequest& request)
> +bool ResourceHandle::willLoadFromCache(ResourceRequest& request, Frame* frame)

this will break the mac build... remove the unused parameter


> +#if QT_VERSION >= 0x040400
> +    QNetworkAccessManager* manager = QWebFramePrivate::kit(frame)->page()->networkAccessManager();
> +    QAbstractNetworkCache* cache = manager->cache();


this will not work on Qt4.4... the cache got added in Qt 4.5...

please revise the patch, it looks good otherwise!

[Holger Freyther]

(In reply to comment #2)
> (From update of attachment 32079 [details])
> Why does Qt need the frame and no other port does?
> 
> Isn't there a global "network manager"?
> +    QNetworkAccessManager* manager =
> QWebFramePrivate::kit(frame)->page()->networkAccessManager();

Qt has a network manager instance per QWebPage. E.g. different parts of the app (think of the help system) have different policies for networking access... anyway this was just a quick example.

Prior changes due this included the static/synchronous XMLHttpRequest implementation and cookies. Back then we have added the Frame*/Document* to the signature with Apple's review, I think it is the same case here.

[Yongjun Zhang]

Created attachment 33100 [details]
modify the patch to include Zecke's review comments.

Incorporate Zeke's comments:

1. leave Frame* argument empty to be able to  build in mac.
2. enable the feature in QT_VERSION 4.5, where disk cache was first introduced.

[Simon Hausmann]

Comment on attachment 33100 [details]
modify the patch to include Zecke's review comments.

r=me. There's a typo in the ChangeLog and the "No new tests" line shouldn't be there, but I'll remove that when landing.

[Simon Hausmann]

Landed in r46535