Bug 206037 – Expand _WKResourceLoadDelegate callbacks

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Expand _WKResourceLoadDelegate callbacks

[Alex Christensen]

Created attachment 387277 [details]
Patch

[Alex Christensen]

Created attachment 387280 [details]
Patch

[Alex Christensen]

Created attachment 387284 [details]
Patch

[youenn fablet]

Comment on attachment 387284 [details]
Patch

Seems fine but I have some questions.

This API is read-only.
Do we also want to modify/cancel all these requests?
Do we envision replacing InjectedBundle loading API with this API?

Do we want to see these requests before or after service worker?
Currently, this is after service worker.

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

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:104
> +    return nextIdentifier++;

Can we use an ObjectIdentifier?

> Source/WebKit/NetworkProcess/NetworkResourceLoader.h:111
> +    void didReceiveChallenge(const WebCore::AuthenticationChallenge&) override;

final

> Source/WebKit/UIProcess/API/APIResourceLoadInfo.h:40
> +    ResourceLoadInfo(WebKit::ResourceLoadInfo&& info)

explicit and private.

[Alex Christensen]

(In reply to youenn fablet from comment #4)
> Comment on attachment 387284 [details]
> Patch
> 
> Seems fine but I have some questions.
> 
> This API is read-only.
> Do we also want to modify/cancel all these requests?
That is not planned right now.  Hopefully never.
> Do we envision replacing InjectedBundle loading API with this API?
The InjectedBundle has the ability to mutate requests before sending.  This is not a replacement for that ability.
> 
> Do we want to see these requests before or after service worker?
> Currently, this is after service worker.
After.  This records what is actually sent on the network.  If we find we need service worker requests, too, we can add that.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387284&action=review
> 
> > Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:104
> > +    return nextIdentifier++;
> 
> Can we use an ObjectIdentifier?
No, because NetworkResourceLoader already has an identifier() method that returns the identifier according to the web process, but this identifier needs to be unique to the network process.
> 
> > Source/WebKit/NetworkProcess/NetworkResourceLoader.h:111
> > +    void didReceiveChallenge(const WebCore::AuthenticationChallenge&) override;
> 
> final
> 
> > Source/WebKit/UIProcess/API/APIResourceLoadInfo.h:40
> > +    ResourceLoadInfo(WebKit::ResourceLoadInfo&& info)
> 
> explicit and private.
Will do before committing.

[youenn fablet]

> > Can we use an ObjectIdentifier?
> No, because NetworkResourceLoader already has an identifier() method that
> returns the identifier according to the web process, but this identifier
> needs to be unique to the network process.

I mean, can we use an ObjectIdentifier<NetworkResourceLoadIdentifierType> instead of an integer.

[Alex Christensen]

Created attachment 387350 [details]
Patch

[Alex Christensen]

(In reply to youenn fablet from comment #6)
> I mean, can we use an ObjectIdentifier<NetworkResourceLoadIdentifierType>
> instead of an integer.
That worked, but it made it so my identifiers were not 1, 2, 3... but rather numbers with all ObjectIdentifiers mixed in.  That's probably fine, but I had to update the test too.  I shouldn't be relying on deterministic identifiers anyways.

[Alex Christensen]

http://trac.webkit.org/r254345

[Radar WebKit Bug Importer]

<rdar://problem/58482281>

[Jonathan Bedard]

New test has been crashing since it was committed:
https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ResourceLoadDelegate.BeaconAndSyncXHR

[Alex Christensen]

Looking into it...

[Alex Christensen]

http://trac.webkit.org/r254367 should fix those tests.

[Alex Christensen]

http://trac.webkit.org/r256444