Bug 30670

Summary: [Qt] symbian buildfix for resourceloadnotifier
Product: WebKit Reporter: Janne Koskinen <koshuin>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric, zecke
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: S60 Emulator   
OS: S60 3rd edition   
Bug Depends on:    
Bug Blocks: 27065, 28003    
Attachments:
Description Flags
proposed fix for ResourceLoaderNotifier.h
none
proposed fix for ResourceLoaderNotifier.h take 2
zecke: review-
unification of ResourceRequest class/struct usage none

Description Janne Koskinen 2009-10-22 03:59:35 PDT
ResourceRequest is forward declared as class and the actual definition is class.
This will cause WINSCW linker to fail all function calls where ResourceRequest is one of the parameters
Comment 1 Janne Koskinen 2009-10-22 04:20:40 PDT
Created attachment 41653 [details]
proposed fix for ResourceLoaderNotifier.h
Comment 2 Janne Koskinen 2009-10-22 04:25:13 PDT
Created attachment 41654 [details]
proposed fix for ResourceLoaderNotifier.h take 2

Added the wrong patch. that will be coming up shortly :)
Comment 3 Holger Freyther 2009-10-22 05:02:11 PDT
Comment on attachment 41654 [details]
proposed fix for ResourceLoaderNotifier.h take 2


>  #include <wtf/Noncopyable.h>
> -

^^^ we don't add whitespace like this...


>  namespace WebCore {
>  
>  class AuthenticationChallenge;
> @@ -40,7 +39,7 @@ class Frame;
>  class ResourceError;
>  class ResourceLoader;
>  class ResourceResponse;
> -class ResourceRequest;
> +struct ResourceRequest;
>  class ScriptString;


Yes and No. CF, Chromium, CURL, SOUP refer to it as struct ResourceRequest, mac is having a proper class.

WebCore/platform/network/mac/ResourceRequest.h:41:    class ResourceRequest : public ResourceRequestBase {

I think it is time to change the other ports to class ResourceRequest as well because the base class is declared as a "class".
Comment 4 Janne Koskinen 2009-10-22 06:58:30 PDT
(In reply to comment #3)
> WebCore/platform/network/mac/ResourceRequest.h:41:    class ResourceRequest :
> public ResourceRequestBase {
> 
> I think it is time to change the other ports to class ResourceRequest as well
> because the base class is declared as a "class".

Thanks for the review, I'll prepare a patch that changes all forward declarations and the struct definition to class. This time without additional line removal :) Patch will be slightly bigger as we touch a lot of files (75 at quick glance).
Comment 5 Janne Koskinen 2009-10-29 01:36:47 PDT
Created attachment 42084 [details]
unification of ResourceRequest class/struct usage
Comment 6 Holger Freyther 2009-10-29 05:13:52 PDT
Do you mind if I change the changelog subject line to "Change ResourceRequest to be ..."? Patch looks fine, I will try to build Gtk, Qt and Mac and then land it.
Comment 7 Janne Koskinen 2009-10-29 10:32:32 PDT
(In reply to comment #6)
> Do you mind if I change the changelog subject line to "Change ResourceRequest
> to be ..."? Patch looks fine, I will try to build Gtk, Qt and Mac and then land
> it.

No, I don't mind. Go ahead and change it.
Comment 8 Holger Freyther 2009-11-07 00:14:43 PST
Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

Should be compiled tested before landing...
Comment 9 Eric Seidel (no email) 2009-11-08 10:39:52 PST
bug 28003 is yet another "dupe" of this.
Comment 10 Eric Seidel (no email) 2009-11-08 10:40:19 PST
Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

the commit-queue will test the compile before it lands.
Comment 11 WebKit Commit Bot 2009-11-08 11:02:03 PST
Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

Clearing flags on attachment: 42084

Committed r50625: <http://trac.webkit.org/changeset/50625>
Comment 12 WebKit Commit Bot 2009-11-08 11:02:09 PST
All reviewed patches have been landed.  Closing bug.