Bug 175505 – Update CachedResourceLoader::requestResource() to return a WTF::Expected

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

[Chris Dumez]

Update CachedResourceLoader::requestResource() & co. to return a WTF::Expected instead of using an out parameter for the ResourceError.

[Chris Dumez]

Created attachment 318006 [details]
Patch

[Chris Dumez]

@Sam: Here is what it looks like.

[Chris Dumez]

Created attachment 318010 [details]
Patch

[youenn fablet]

Comment on attachment 318010 [details]
Patch

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

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:93
> +        return CachedResourceHandle<T> { static_cast<T*>(cachedResource.value().get()) };

In changed code, a downcast was used. Why using a static_cast here?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:729
> +                CachedResourceHandle<CachedResource> resource = createResource(type, WTFMove(request), sessionID());

Can we make createResource return a CachedResourceHandle<CachedResource> instead?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:800
> +            if (!shouldContinueAfterNotifyingLoadedFromMemoryCache(request, resource.get(), error))

Would returning an optional make it better?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:839
> +                return makeUnexpected(ResourceError { String(), 0, url, String(), ResourceError::Type::AccessControl });

AccessControl is usually the reason of immediate failures.
Would be cool to have not null resource errors in the future.

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:1258
> +        auto resourceValue = resource.value();

auto& maybe?

> Source/WebCore/loader/cache/CachedResourceLoader.h:178
> +    bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*, ResourceError&);

Pre-existing code issue but it would seem that we should pass a const CachedResource* and probably even better a const CachedResource&.

[Chris Dumez]

Created attachment 318044 [details]
Patch

[Chris Dumez]

Created attachment 318149 [details]
Patch

[Chris Dumez]

Comment on attachment 318149 [details]
Patch

Clearing flags on attachment: 318149

Committed r220760: <http://trac.webkit.org/changeset/220760>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33903768>