Bug 158313 – Modernize how many platform/network classes do isolatedCopy()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Modernize how many platform/network classes do isolatedCopy()

First part of this work was in https://bugs.webkit.org/show_bug.cgi?id=158293

Now let's get rid of these "CrossThread*Data" objects altogether.

[Brady Eidson]

Created attachment 280359 [details]
Patch

[Alex Christensen]

Comment on attachment 280359 [details]
Patch

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

> Source/WebCore/platform/network/ResourceRequestBase.cpp:87
> +    setAllowCookies(m_allowCookies);

other.m_allowCookies

> Source/WebCore/platform/network/ResourceRequestBase.cpp:89
> +    const_cast<ResourceRequest&>(asResourceRequest()).doPlatformSetAsIsolatedCopy(other);

Would it work to make a non-const asResourceRequest() instead of a const_cast here?

[Brady Eidson]

(In reply to comment #2)
> Comment on attachment 280359 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=280359&action=review
> 
> > Source/WebCore/platform/network/ResourceRequestBase.cpp:87
> > +    setAllowCookies(m_allowCookies);
> 
> other.m_allowCookies

Yup yikes.

> > Source/WebCore/platform/network/ResourceRequestBase.cpp:89
> > +    const_cast<ResourceRequest&>(asResourceRequest()).doPlatformSetAsIsolatedCopy(other);
> 
> Would it work to make a non-const asResourceRequest() instead of a
> const_cast here?

const_cast'ing the const ResourceRequest& is an already established pattern in the class.


The relationship between ResourceRequestBase and ResourceRequest is already so gnarly that I didn't feel like tackling that here.

Would be a good followup cleanup.

[Brady Eidson]

The linux build errors:

In file included from ../../Source/WebCore/platform/network/ResourceErrorBase.cpp:28:0:
../../Source/WebCore/platform/network/soup/ResourceError.h: In member function 'WebCore::ResourceError WebCore::ResourceErrorBase::isolatedCopy() const':
../../Source/WebCore/platform/network/soup/ResourceError.h:70:10: error: 'void WebCore::ResourceError::doPlatformIsolatedCopy(const WebCore::ResourceError&)' is private
../../Source/WebCore/platform/network/ResourceErrorBase.cpp:52:55: error: within this context

Funny, that method is private in the CF/Mac version as well, but it builds on Clang.

Anyways, yah I'll make it public.

(Normally I'd virtualize it, but we try to keep these classes non-virtual for some historic reason I forgot)

[Brady Eidson]

(In reply to comment #4)
> The linux build errors:
> 
> In file included from
> ../../Source/WebCore/platform/network/ResourceErrorBase.cpp:28:0:
> ../../Source/WebCore/platform/network/soup/ResourceError.h: In member
> function 'WebCore::ResourceError WebCore::ResourceErrorBase::isolatedCopy()
> const':
> ../../Source/WebCore/platform/network/soup/ResourceError.h:70:10: error:
> 'void WebCore::ResourceError::doPlatformIsolatedCopy(const
> WebCore::ResourceError&)' is private
> ../../Source/WebCore/platform/network/ResourceErrorBase.cpp:52:55: error:
> within this context
> 
> Funny, that method is private in the CF/Mac version as well, but it builds
> on Clang.
> 
> Anyways, yah I'll make it public.
> 
> (Normally I'd virtualize it, but we try to keep these classes non-virtual
> for some historic reason I forgot)

Oh duh:
    friend class ResourceErrorBase;


Will add to the other headers.

[Brady Eidson]

Last 500 characters of output:
p.o.d -o Source/WebCore/CMakeFiles/WebCore.dir/platform/network/FormData.cpp.o -c ../../Source/WebCore/platform/network/FormData.cpp
../../Source/WebCore/platform/network/FormData.cpp: In member function 'WebCore::FormDataElement WebCore::FormDataElement::isolatedCopy() const':
../../Source/WebCore/platform/network/FormData.cpp:139:1: error: control reaches end of non-void function [-Werror=return-type]

That's just GCC not recognizing that all 3 of the possible cases are handled. *sigh*

[Brady Eidson]

Created attachment 280367 [details]
Patch

[Brady Eidson]

Created attachment 280368 [details]
Patch

[Brady Eidson]

For an EWS run. Will cq+ once linux bots are happy

[WebKit Commit Bot]

Comment on attachment 280368 [details]
Patch

Clearing flags on attachment: 280368

Committed r201623: <http://trac.webkit.org/changeset/201623>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.