Bug 193361 - Regression(PSON) View becomes blank after click a cross-site download link
Summary: Regression(PSON) View becomes blank after click a cross-site download link
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 193509
Blocks:
  Show dependency treegraph
 
Reported: 2019-01-11 13:17 PST by Chris Dumez
Modified: 2019-01-16 14:20 PST (History)
14 users (show)

See Also:


Attachments
Prototype (66.32 KB, patch)
2019-01-11 13:19 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Prototype (74.19 KB, patch)
2019-01-11 14:50 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Prototype (72.51 KB, patch)
2019-01-11 14:57 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Prototype (with test) (77.17 KB, patch)
2019-01-11 16:18 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Prototype (with test) (77.20 KB, patch)
2019-01-11 16:30 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (95.43 KB, patch)
2019-01-14 09:54 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (96.00 KB, patch)
2019-01-14 13:10 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (97.53 KB, patch)
2019-01-14 15:41 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (97.74 KB, patch)
2019-01-14 15:49 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (97.66 KB, patch)
2019-01-14 16:06 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (98.71 KB, patch)
2019-01-14 19:21 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (99.89 KB, patch)
2019-01-15 08:45 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (99.97 KB, patch)
2019-01-15 09:15 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (103.93 KB, patch)
2019-01-15 09:37 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (103.56 KB, patch)
2019-01-15 09:44 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (104.28 KB, patch)
2019-01-15 09:54 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (104.27 KB, patch)
2019-01-15 10:24 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (165.01 KB, patch)
2019-01-15 22:00 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (193.23 KB, patch)
2019-01-16 09:13 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (222.28 KB, patch)
2019-01-16 10:01 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-01-11 13:17:13 PST
View becomes blank after click a cross-site download link when process-swap on navigation is disabled.
Comment 1 Chris Dumez 2019-01-11 13:17:30 PST
<rdar://problem/47099573>
Comment 2 Chris Dumez 2019-01-11 13:19:17 PST
Created attachment 358934 [details]
Prototype

Passes all ProcessSwap API tests in debug and fixes the cross-site download case.
Comment 3 Chris Dumez 2019-01-11 14:50:41 PST
Created attachment 358950 [details]
Prototype
Comment 4 EWS Watchlist 2019-01-11 14:53:38 PST
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.
Comment 5 Chris Dumez 2019-01-11 14:57:28 PST
Created attachment 358953 [details]
Prototype
Comment 6 Chris Dumez 2019-01-11 16:18:39 PST
Created attachment 358960 [details]
Prototype (with test)
Comment 7 EWS Watchlist 2019-01-11 16:21:58 PST
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.
Comment 8 Chris Dumez 2019-01-11 16:30:36 PST
Created attachment 358961 [details]
Prototype (with test)
Comment 9 EWS Watchlist 2019-01-11 16:35:09 PST
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.
Comment 10 Chris Dumez 2019-01-14 09:54:50 PST
Created attachment 359047 [details]
Patch
Comment 11 EWS Watchlist 2019-01-14 09:57:12 PST
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.
Comment 12 Chris Dumez 2019-01-14 13:10:32 PST
Created attachment 359071 [details]
Patch
Comment 13 EWS Watchlist 2019-01-14 13:12:37 PST
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.
Comment 14 Chris Dumez 2019-01-14 15:41:37 PST
Created attachment 359086 [details]
Patch
Comment 15 EWS Watchlist 2019-01-14 15:43:21 PST
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.
Comment 16 Chris Dumez 2019-01-14 15:49:42 PST
Created attachment 359088 [details]
Patch
Comment 17 EWS Watchlist 2019-01-14 15:51:58 PST
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.
Comment 18 Chris Dumez 2019-01-14 16:06:45 PST
Created attachment 359092 [details]
Patch
Comment 19 EWS Watchlist 2019-01-14 16:10:02 PST
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.
Comment 20 Chris Dumez 2019-01-14 19:21:44 PST
Created attachment 359119 [details]
Patch
Comment 21 EWS Watchlist 2019-01-14 19:24:20 PST
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.
Comment 22 Chris Dumez 2019-01-14 19:55:46 PST
Comment on attachment 359119 [details]
Patch

Nevermind, I found breakage.
Comment 23 Antti Koivisto 2019-01-15 02:25:57 PST
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.
Comment 24 Chris Dumez 2019-01-15 08:41:37 PST
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?
Comment 25 Chris Dumez 2019-01-15 08:42:50 PST
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.
Comment 26 Chris Dumez 2019-01-15 08:45:27 PST
Created attachment 359163 [details]
Patch
Comment 27 EWS Watchlist 2019-01-15 08:47:30 PST
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.
Comment 28 Chris Dumez 2019-01-15 09:15:36 PST
Created attachment 359165 [details]
Patch
Comment 29 EWS Watchlist 2019-01-15 09:17:39 PST
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.
Comment 30 Chris Dumez 2019-01-15 09:37:12 PST
Created attachment 359166 [details]
Patch
Comment 31 EWS Watchlist 2019-01-15 09:40:30 PST
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.
Comment 32 Chris Dumez 2019-01-15 09:44:50 PST
Created attachment 359168 [details]
Patch
Comment 33 EWS Watchlist 2019-01-15 09:47:38 PST
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.
Comment 34 Chris Dumez 2019-01-15 09:54:02 PST
Created attachment 359169 [details]
Patch
Comment 35 EWS Watchlist 2019-01-15 09:57:42 PST
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.
Comment 36 Geoffrey Garen 2019-01-15 10:18:24 PST
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
Comment 37 Chris Dumez 2019-01-15 10:21:37 PST
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.
Comment 38 Chris Dumez 2019-01-15 10:24:23 PST
Created attachment 359176 [details]
Patch
Comment 39 EWS Watchlist 2019-01-15 10:27:00 PST
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.
Comment 40 WebKit Commit Bot 2019-01-15 11:01:01 PST
The commit-queue encountered the following flaky tests while processing attachment 359176 [details]:

fetch/fetch-worker-crash.html bug 187257 (author: youennf@gmail.com)
The commit-queue is continuing to process your patch.
Comment 41 WebKit Commit Bot 2019-01-15 11:01:50 PST
Comment on attachment 359176 [details]
Patch

Clearing flags on attachment: 359176

Committed r239993: <https://trac.webkit.org/changeset/239993>
Comment 42 WebKit Commit Bot 2019-01-15 11:01:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 43 Chris Dumez 2019-01-15 12:05:01 PST
Follow-up iOS build fix: <https://trac.webkit.org/changeset/239995>
Comment 44 Alex Christensen 2019-01-15 12:30:16 PST
Another follow-up build fix for iOS:

http://trac.webkit.org/r239997
Comment 45 Chris Dumez 2019-01-15 15:48:37 PST
Reverted r239993, r239995, r239997, and r239999 for reason:

Caused assertions under ViewGestureController::disconnectFromProcess()

Committed r240015: <https://trac.webkit.org/changeset/240015>
Comment 46 Chris Dumez 2019-01-15 15:48:39 PST
Reverted r239993, r239995, r239997, and r239999 for reason:

Caused assertions under ViewGestureController::disconnectFromProcess()

Committed r240015: <https://trac.webkit.org/changeset/240015>
Comment 47 Chris Dumez 2019-01-15 22:00:15 PST
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.
Comment 48 EWS Watchlist 2019-01-15 22:02:29 PST
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.
Comment 49 Chris Dumez 2019-01-16 09:13:45 PST
Created attachment 359269 [details]
WIP Patch
Comment 50 EWS Watchlist 2019-01-16 09:16:19 PST
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
Comment 51 EWS Watchlist 2019-01-16 09:16:56 PST
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.
Comment 52 Chris Dumez 2019-01-16 10:01:17 PST
Created attachment 359274 [details]
Patch
Comment 53 EWS Watchlist 2019-01-16 10:03:39 PST
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.
Comment 54 Geoffrey Garen 2019-01-16 12:02:26 PST
Comment on attachment 359274 [details]
Patch

r=me
Comment 55 Chris Dumez 2019-01-16 12:40:10 PST
Comment on attachment 359274 [details]
Patch

Clearing flags on attachment: 359274

Committed r240046: <https://trac.webkit.org/changeset/240046>
Comment 56 Chris Dumez 2019-01-16 12:40:13 PST
All reviewed patches have been landed.  Closing bug.
Comment 57 Chris Dumez 2019-01-16 13:45:38 PST
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.
Comment 58 Chris Dumez 2019-01-16 14:20:50 PST
(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.