Bug 30670 – [Qt] symbian buildfix for resourceloadnotifier

• Component: New Bugs
• Status: RESOLVED FIXED

[Janne Koskinen]

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

[Janne Koskinen]

Created attachment 41653 [details]
proposed fix for ResourceLoaderNotifier.h

[Janne Koskinen]

Created attachment 41654 [details]
proposed fix for ResourceLoaderNotifier.h take 2

Added the wrong patch. that will be coming up shortly :)

[Holger Freyther]

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

[Janne Koskinen]

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

[Janne Koskinen]

Created attachment 42084 [details]
unification of ResourceRequest class/struct usage

[Holger Freyther]

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.

[Janne Koskinen]

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

[Holger Freyther]

Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

Should be compiled tested before landing...

[Eric Seidel (no email)]

bug 28003 is yet another "dupe" of this.

[Eric Seidel (no email)]

Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

the commit-queue will test the compile before it lands.

[WebKit Commit Bot]

Comment on attachment 42084 [details]
unification of ResourceRequest class/struct usage

Clearing flags on attachment: 42084

Committed r50625: <http://trac.webkit.org/changeset/50625>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.