Bug 193361 – Regression(PSON) View becomes blank after click a cross-site download link

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

View becomes blank after click a cross-site download link when process-swap on navigation is disabled.

[Chris Dumez]

<rdar://problem/47099573>

[Chris Dumez]

Created attachment 358934 [details]
Prototype

Passes all ProcessSwap API tests in debug and fixes the cross-site download case.

[Chris Dumez]

Created attachment 358950 [details]
Prototype

[EWS Watchlist]

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


ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/TestExpectations:2912:  Duplicate or ambiguous entry lines LayoutTests/TestExpectations:130 and LayoutTests/TestExpectations:2912.  [test/expectations] [5]
Total errors found: 1 in 22 files


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

[Chris Dumez]

Created attachment 358953 [details]
Prototype

[Chris Dumez]

Created attachment 358960 [details]
Prototype (with test)

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1470:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1472:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1474:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 21 files


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

[Chris Dumez]

Created attachment 358961 [details]
Prototype (with test)

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1470:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1472:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1474:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 21 files


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

[Chris Dumez]

Created attachment 359047 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Created attachment 359071 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Created attachment 359086 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Created attachment 359088 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Created attachment 359092 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Created attachment 359119 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 25 files


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

[Chris Dumez]

Comment on attachment 359119 [details]
Patch

Nevermind, I found breakage.

[Antti Koivisto]

Comment on attachment 359119 [details]
Patch

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

> Source/WebKit/ChangeLog:77
> +        Add new ProvisionalPageProxy class to wrap the provisional load in the new process
> +        after a swap. The provisional page is owned by the WebPageProxy and we only commit
> +        the provisional page when the load is committed. Until then, the WebPageProxy keeps
> +        using the old process and displaying the current content.

Adding another proxy with message handling etc is bit unfortunate. I suppose this ends up cleaner than just expanding/renaming SuspendedPageProxy?

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:475
> -    if (parameters.isSwapFromSuspended)
> +    if (parameters.isProcessSwap)
>          freezeLayerTree(LayerTreeFreezeReason::SwapFromSuspended);

Could rename the LayerTreeFreezeReason as well.

[Chris Dumez]

Comment on attachment 359119 [details]
Patch

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

>> Source/WebKit/ChangeLog:77
>> +        using the old process and displaying the current content.
> 
> Adding another proxy with message handling etc is bit unfortunate. I suppose this ends up cleaner than just expanding/renaming SuspendedPageProxy?

There really isn't that much duplicated logic between SuspendedPageProxy and ProvisionalPageProxy though. The 2 classes almost have opposite purposes. Maybe I could introduce a base class and see what could be shared? Maybe some of the IPC related logic?

[Chris Dumez]

Comment on attachment 359119 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:475
>>          freezeLayerTree(LayerTreeFreezeReason::SwapFromSuspended);
> 
> Could rename the LayerTreeFreezeReason as well.

Oh, makes sense. Will do.

[Chris Dumez]

Created attachment 359163 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 26 files


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

[Chris Dumez]

Created attachment 359165 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 26 files


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

[Chris Dumez]

Created attachment 359166 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 26 files


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

[Chris Dumez]

Created attachment 359168 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 26 files


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

[Chris Dumez]

Created attachment 359169 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 26 files


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

[Geoffrey Garen]

Comment on attachment 359169 [details]
Patch

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

r=me

Nice and risky, just like a good old JSC patch!

> Source/WebKit/UIProcess/WebPageProxy.cpp:554
> +DrawingAreaProxy* WebPageProxy::drawingArea() const
> +{
> +    if (m_provisionalPage && m_process.ptr() == &m_provisionalPage->process() && m_provisionalPage->drawingArea())
> +        return m_provisionalPage->drawingArea();
> +    return m_drawingArea.get();
> +}

Why do we want to return the drawing area of the provisional page? My understanding is that we prefer to "lock the layer tree" and avoid drawing a page until it becomes committed.

> Source/WebKit/UIProcess/WebPageProxy.cpp:3886
> +        // The load did not fail, it merely happening in a new provisional process.

it is merely happening

[Chris Dumez]

Comment on attachment 359169 [details]
Patch

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

>> Source/WebKit/UIProcess/WebPageProxy.cpp:554
>> +}
> 
> Why do we want to return the drawing area of the provisional page? My understanding is that we prefer to "lock the layer tree" and avoid drawing a page until it becomes committed.

I had to make this change for the swipe gesture snapshot logic. A drawing area is associated to a particular process and it is important for WebPageProxy::drawingArea() and WebPageProxy::process() to be in sync to the ViewGestureController logic to work.

[Chris Dumez]

Created attachment 359176 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 26 files


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

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 359176 [details]:

fetch/fetch-worker-crash.html bug 187257 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 359176 [details]
Patch

Clearing flags on attachment: 359176

Committed r239993: <https://trac.webkit.org/changeset/239993>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Follow-up iOS build fix: <https://trac.webkit.org/changeset/239995>

[Alex Christensen]

Another follow-up build fix for iOS:

http://trac.webkit.org/r239997

[Chris Dumez]

Reverted r239993, r239995, r239997, and r239999 for reason:

Caused assertions under ViewGestureController::disconnectFromProcess()

Committed r240015: <https://trac.webkit.org/changeset/240015>

[Chris Dumez]

Reverted r239993, r239995, r239997, and r239999 for reason:

Caused assertions under ViewGestureController::disconnectFromProcess()

Committed r240015: <https://trac.webkit.org/changeset/240015>

[Chris Dumez]

Created attachment 359256 [details]
WIP Patch

Working on a more robust iteration which does not require temporarily overriding WebPageProxy::m_process in various places.
Still needs more testing though.

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 47 files


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

[Chris Dumez]

Created attachment 359269 [details]
WIP Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 52 files


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

[Chris Dumez]

Created attachment 359274 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1476:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1478:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1480:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Source/WebKit/UIProcess/WebProcessProxy.cpp:707:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 4 in 57 files


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

[Geoffrey Garen]

Comment on attachment 359274 [details]
Patch

r=me

[Chris Dumez]

Comment on attachment 359274 [details]
Patch

Clearing flags on attachment: 359274

Committed r240046: <https://trac.webkit.org/changeset/240046>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

Caused some non-PSON API tests failures:
Test suite failed

Crashed

    TestWebKitAPI.WebKit.ResizeWindowAfterCrash
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        libc++abi.dylib: Pure virtual function called!

    TestWebKitAPI.WebKit.LoadPageAfterCrash
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        libc++abi.dylib: Pure virtual function called!

    TestWebKitAPI.WebKit.EnumerateDevices
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        libc++abi.dylib: Pure virtual function called!

    TestWebKitAPI.WKNavigation.FailureToStartWebProcessAfterCrashRecovery
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        libc++abi.dylib: Pure virtual function called!

    TestWebKitAPI.WebKit.ReloadPageAfterCrash
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        libc++abi.dylib: Pure virtual function called!

Timeout

    TestWebKitAPI.WKNavigation.FailureToStartWebProcessRecovery
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.

Investigating to see if I can quickly fix it or if I need to roll out.

[Chris Dumez]

(In reply to Chris Dumez from comment #57)
> Caused some non-PSON API tests failures:
> Test suite failed
> 
> Crashed
> 
>     TestWebKitAPI.WebKit.ResizeWindowAfterCrash
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         libc++abi.dylib: Pure virtual function called!
> 
>     TestWebKitAPI.WebKit.LoadPageAfterCrash
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         libc++abi.dylib: Pure virtual function called!
> 
>     TestWebKitAPI.WebKit.EnumerateDevices
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         libc++abi.dylib: Pure virtual function called!
> 
>     TestWebKitAPI.WKNavigation.FailureToStartWebProcessAfterCrashRecovery
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         libc++abi.dylib: Pure virtual function called!
> 
>     TestWebKitAPI.WebKit.ReloadPageAfterCrash
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
>         libc++abi.dylib: Pure virtual function called!
> 
> Timeout
> 
>     TestWebKitAPI.WKNavigation.FailureToStartWebProcessRecovery
>         _RegisterApplication(), FAILED TO establish the default connection
> to the WindowServer, _CGSDefaultConnection() is NULL.
> 
> Investigating to see if I can quickly fix it or if I need to roll out.

Fixing those crashes / timeout via https://bugs.webkit.org/show_bug.cgi?id=193509.