Bug 146710 – Move PingLoaders to the NetworkingProcess

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Move PingLoaders to the NetworkingProcess

<rdar://problem/18860263>

[Brady Eidson]

Created attachment 256395 [details]
Patch v1

[Brady Eidson]

Created attachment 256403 [details]
Patch v2

[Tim Horton]

Comment on attachment 256403 [details]
Patch v2

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

> Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:262
> +    WebFrame* webFrame = webFrameLoaderClient ? webFrameLoaderClient->webFrame() : 0;

nullptr!

[Brady Eidson]

Created attachment 256407 [details]
Patch for landing.

[Daniel Bates]

Comment on attachment 256407 [details]
Patch for landing.

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

> Source/WebCore/platform/network/PingHandle.h:65
> +    virtual bool usesAsyncCallbacks() { return m_usesAsyncCallbacks == UsesAsyncCallbacks::Yes; }

This needs to be marked override or it will cause a compiler error since we compile with -Winconsistent-missing-override.

[Daniel Bates]

Created attachment 256408 [details]
Patch for landing

Added keyword override to the declaration of PingHandle::usesAsyncCallbacks.

[Brady Eidson]

Created attachment 256409 [details]
Patch for landing.

[Alexey Proskuryakov]

Comment on attachment 256408 [details]
Patch for landing

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

Finally! :)

> Source/WebCore/ChangeLog:9
> +        There's no current solution for communicating back to a page that a ping load has reached its target.

We could have the ping target change some global state (usually it's a file on disk), and then another document could poll the state.

[Daniel Bates]

<rdar://problem/18860263>

[Brady Eidson]

(In reply to comment #8)
> Comment on attachment 256408 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256408&action=review
> 
> Finally! :)
> 
> > Source/WebCore/ChangeLog:9
> > +        There's no current solution for communicating back to a page that a ping load has reached its target.
> 
> We could have the ping target change some global state (usually it's a file
> on disk), and then another document could poll the state.

That was my first idea, but then I realized that:
- Tests that rely on files on disk on the httpd side are often fragile
- Tests that rely on polling are... gross

I actually played around with running two php scripts and IPC'ing between them, and I think that will actually work, but there's a steep learning curve incompatible with getting this frick'in bug fix in!

I will revisit it soon - it's on my white board :)

[Brady Eidson]

(In reply to comment #9)
> <rdar://problem/18860263>

That was in comment 1 ;)

[Brady Eidson]

*** Bug 138688 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 256409 [details]
Patch for landing.

Clearing flags on attachment: 256409

Committed r186530: <http://trac.webkit.org/changeset/186530>

[Alexey Proskuryakov]

> - Tests that rely on files on disk on the httpd side are often fragile

They actually aren't any more, as long as written carefully. We now make sure to have unique names (usually by using a sanitized test path name as temporary file name), and that works pretty reliably, even in most stress testing scenarios.

Some CGI scripts get the name implicitly (from Referer header field), others get it explicitly.

[Brady Eidson]

(In reply to comment #14)
> > - Tests that rely on files on disk on the httpd side are often fragile
> 
> They actually aren't any more, as long as written carefully. We now make
> sure to have unique names (usually by using a sanitized test path name as
> temporary file name), and that works pretty reliably, even in most stress
> testing scenarios.
> 
> Some CGI scripts get the name implicitly (from Referer header field), others
> get it explicitly.

Good to know.

I could be able to use that plus a script that polls for the file before responding. That way the test itself won't have to poll the server - it can just wait for the response.

But as long as I'm polling for a file in php to do adhoc IPC between scripts, I could just use one of the "real" PHP IPC mechanisms.

Will revisit soon.