Bug 153037 – WebKit2 should have an API for eagerly querying whether the web process is responsive

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

WebKit2 should have an API for eagerly querying whether the web process is responsive

[Geoffrey Garen]

Created attachment 268805 [details]
Patch

[WebKit Commit Bot]

Attachment 268805 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:2643:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:746:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Anders Carlsson]

Comment on attachment 268805 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2647
> +        callbackFunction(webProcessIsResponsive);

You don't want to call this directly from the callback; it needs to be asynchronous. Please use RunLoop::main().dispatch.

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:558
> +    Vector<std::function<void(bool webProcessIsResponsive)>> isResponsiveCallbacks = WTFMove(m_isResponsiveCallbacks);

auto isResponsiveCallbacks?

> Source/WebKit2/UIProcess/WebProcessProxy.cpp:1004
> +            bool webProcessIsResponsive = false;
> +            callback(webProcessIsResponsive);

Same callback about being asynchronous here.

> Source/WebKit2/UIProcess/WebProcessProxy.h:258
> +    bool m_isResponsive;

m_isResponsive doesn't reflect the member variable fully. Here's my understanding:

m_isResponsive = false -> We know that the web process is unresponsive.
m_isResponsive = true -> We think that the web process is responsive.

Maybe rename it to m_isKnownToBeUnresponsive and then invert the checks?

[Geoffrey Garen]

Created attachment 268813 [details]
Patch

[WebKit Commit Bot]

Attachment 268813 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:2643:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebPageProxy.h:746:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

> https://bugs.webkit.org/attachment.cgi?id=268805&action=review
> 
> > Source/WebKit2/UIProcess/WebPageProxy.cpp:2647
> > +        callbackFunction(webProcessIsResponsive);
> 
> You don't want to call this directly from the callback; it needs to be
> asynchronous. Please use RunLoop::main().dispatch.

Fixed.

> 
> > Source/WebKit2/UIProcess/WebProcessProxy.cpp:558
> > +    Vector<std::function<void(bool webProcessIsResponsive)>> isResponsiveCallbacks = WTFMove(m_isResponsiveCallbacks);
> 
> auto isResponsiveCallbacks?

Fixed.

> 
> > Source/WebKit2/UIProcess/WebProcessProxy.cpp:1004
> > +            bool webProcessIsResponsive = false;
> > +            callback(webProcessIsResponsive);
> 
> Same callback about being asynchronous here.

Fixed.

> > Source/WebKit2/UIProcess/WebProcessProxy.h:258
> > +    bool m_isResponsive;
> 
> m_isResponsive doesn't reflect the member variable fully. Here's my
> understanding:
> 
> m_isResponsive = false -> We know that the web process is unresponsive.
> m_isResponsive = true -> We think that the web process is responsive.
> 
> Maybe rename it to m_isKnownToBeUnresponsive and then invert the checks?

I made this an enum.

I renamed the API to remove "get" from the name since Tim and Alex seemed not to like "get".

[Geoffrey Garen]

(Anders didn't like "get" either.)

[Geoffrey Garen]

Committed r194986: <http://trac.webkit.org/changeset/194986>