| Summary: | Regression(PSON): WebView is blank when navigating back cross-process on iOS | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
| Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | commit-queue, ggaren, koivisto, realdawei, ryanhaddad, simon.fraser, thorton, tsavell, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | 189587 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Chris Dumez
2018-09-10 13:33:31 PDT
Created attachment 349325 [details]
Patch
Comment on attachment 349325 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349325&action=review > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:526 > + if (auto* drawingArea = m_page.drawingArea()) I bet these aren't the only null checks we need. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:6003 > + if (m_isSuspended == suspended) > + return; Suspended just means PSON-y suspension, not e.g. normal process suspension? (In reply to Tim Horton from comment #3) > Comment on attachment 349325 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349325&action=review > > > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:526 > > + if (auto* drawingArea = m_page.drawingArea()) > > I bet these aren't the only null checks we need. Chances are you are right. Those are the only ones I hit on on Mac and iOS so far though. > > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:6003 > > + if (m_isSuspended == suspended) > > + return; > > Suspended just means PSON-y suspension, not e.g. normal process suspension? PSON-y suspension. (In reply to Chris Dumez from comment #4) > (In reply to Tim Horton from comment #3) > > Comment on attachment 349325 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=349325&action=review > > > > > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:526 > > > + if (auto* drawingArea = m_page.drawingArea()) > > > > I bet these aren't the only null checks we need. > > Chances are you are right. Those are the only ones I hit on on Mac and iOS > so far though. > > > > > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:6003 > > > + if (m_isSuspended == suspended) > > > + return; > > > > Suspended just means PSON-y suspension, not e.g. normal process suspension? > > PSON-y suspension. Very mysterious, we should probably think about naming these things more independently. (In reply to Tim Horton from comment #5) > (In reply to Chris Dumez from comment #4) > > (In reply to Tim Horton from comment #3) > > > Comment on attachment 349325 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=349325&action=review > > > > > > > Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:526 > > > > + if (auto* drawingArea = m_page.drawingArea()) > > > > > > I bet these aren't the only null checks we need. > > > > Chances are you are right. Those are the only ones I hit on on Mac and iOS > > so far though. > > > > > > > > > Source/WebKit/WebProcess/WebPage/WebPage.cpp:6003 > > > > + if (m_isSuspended == suspended) > > > > + return; > > > > > > Suspended just means PSON-y suspension, not e.g. normal process suspension? > > > > PSON-y suspension. > > Very mysterious, we should probably think about naming these things more > independently. I agree, Brady added this and I am not quite sure what he had in mind at the time. I'll clarify the naming in a follow-up. Comment on attachment 349325 [details] Patch Clearing flags on attachment: 349325 Committed r235867: <https://trac.webkit.org/changeset/235867> All reviewed patches have been landed. Closing bug. (In reply to WebKit Commit Bot from comment #7) > Comment on attachment 349325 [details] > Patch > > Clearing flags on attachment: 349325 > > Committed r235867: <https://trac.webkit.org/changeset/235867> Seeing API failures on macOS and iOS after this revision sample output: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/7888/steps/run-api-tests/logs/stdio TestWebKitAPI.ProcessSwap.ProcessReuse Received data during response processing, queuing it. WARNING: The option to always keep swapped web processes alive is active. This is meant for debugging and testing only. Received data during response processing, queuing it. Received data during response processing, queuing it. /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1375 Expected equality of these values: 2u Which is: 2 seenPIDs.size() Which is: 3 /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1378 Expected equality of these values: pid1 Which is: 76466 pid3 Which is: 76470 TestWebKitAPI.ProcessSwap.ProcessReuseeTLDPlus2 Received data during response processing, queuing it. WARNING: The option to always keep swapped web processes alive is active. This is meant for debugging and testing only. Received data during response processing, queuing it. Received data during response processing, queuing it. /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1422 Expected equality of these values: 2u Which is: 2 seenPIDs.size() Which is: 3 /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:1425 Expected equality of these values: pid1 Which is: 76472 pid3 (In reply to Dawei Fenton (:realdawei) from comment #9) > (In reply to WebKit Commit Bot from comment #7) > > Comment on attachment 349325 [details] > > Patch > > > > Clearing flags on attachment: 349325 > > > > Committed r235867: <https://trac.webkit.org/changeset/235867> > > Seeing API failures on macOS and iOS after this revision > > sample output: > https://build.webkit.org/builders/ > Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/7888/steps/run-api- > tests/logs/stdio > > TestWebKitAPI.ProcessSwap.ProcessReuse > Received data during response processing, queuing it. > WARNING: The option to always keep swapped web processes alive is > active. This is meant for debugging and testing only. > Received data during response processing, queuing it. > Received data during response processing, queuing it. > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1375 > Expected equality of these values: > 2u > Which is: 2 > seenPIDs.size() > Which is: 3 > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1378 > Expected equality of these values: > pid1 > Which is: 76466 > pid3 > Which is: 76470 > > > TestWebKitAPI.ProcessSwap.ProcessReuseeTLDPlus2 > Received data during response processing, queuing it. > WARNING: The option to always keep swapped web processes alive is > active. This is meant for debugging and testing only. > Received data during response processing, queuing it. > Received data during response processing, queuing it. > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1422 > Expected equality of these values: > 2u > Which is: 2 > seenPIDs.size() > Which is: 3 > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1425 > Expected equality of these values: > pid1 > Which is: 76472 > pid3 Odd, I will investigate shortly. (In reply to Dawei Fenton (:realdawei) from comment #9) > (In reply to WebKit Commit Bot from comment #7) > > Comment on attachment 349325 [details] > > Patch > > > > Clearing flags on attachment: 349325 > > > > Committed r235867: <https://trac.webkit.org/changeset/235867> > > Seeing API failures on macOS and iOS after this revision > > sample output: > https://build.webkit.org/builders/ > Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/7888/steps/run-api- > tests/logs/stdio > > TestWebKitAPI.ProcessSwap.ProcessReuse > Received data during response processing, queuing it. > WARNING: The option to always keep swapped web processes alive is > active. This is meant for debugging and testing only. > Received data during response processing, queuing it. > Received data during response processing, queuing it. > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1375 > Expected equality of these values: > 2u > Which is: 2 > seenPIDs.size() > Which is: 3 > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1378 > Expected equality of these values: > pid1 > Which is: 76466 > pid3 > Which is: 76470 > > > TestWebKitAPI.ProcessSwap.ProcessReuseeTLDPlus2 > Received data during response processing, queuing it. > WARNING: The option to always keep swapped web processes alive is > active. This is meant for debugging and testing only. > Received data during response processing, queuing it. > Received data during response processing, queuing it. > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1422 > Expected equality of these values: > 2u > Which is: 2 > seenPIDs.size() > Which is: 3 > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/ProcessSwapOnNavigation.mm:1425 > Expected equality of these values: > pid1 > Which is: 76472 > pid3 Follow-up fix in <https://trac.webkit.org/changeset/235880>. (In reply to Chris Dumez from comment #11) > (In reply to Dawei Fenton (:realdawei) from comment #9) > > (In reply to WebKit Commit Bot from comment #7) > > > Comment on attachment 349325 [details] > > > Patch > > > > > > Clearing flags on attachment: 349325 > > > > > > Committed r235867: <https://trac.webkit.org/changeset/235867> > > > > Seeing API failures on macOS and iOS after this revision > > > > sample output: > > https://build.webkit.org/builders/ > > Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/7888/steps/run-api- > > tests/logs/stdio > > > > TestWebKitAPI.ProcessSwap.ProcessReuse > > Received data during response processing, queuing it. > > WARNING: The option to always keep swapped web processes alive is > > active. This is meant for debugging and testing only. > > Received data during response processing, queuing it. > > Received data during response processing, queuing it. > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > WebKitCocoa/ProcessSwapOnNavigation.mm:1375 > > Expected equality of these values: > > 2u > > Which is: 2 > > seenPIDs.size() > > Which is: 3 > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > WebKitCocoa/ProcessSwapOnNavigation.mm:1378 > > Expected equality of these values: > > pid1 > > Which is: 76466 > > pid3 > > Which is: 76470 > > > > > > TestWebKitAPI.ProcessSwap.ProcessReuseeTLDPlus2 > > Received data during response processing, queuing it. > > WARNING: The option to always keep swapped web processes alive is > > active. This is meant for debugging and testing only. > > Received data during response processing, queuing it. > > Received data during response processing, queuing it. > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > WebKitCocoa/ProcessSwapOnNavigation.mm:1422 > > Expected equality of these values: > > 2u > > Which is: 2 > > seenPIDs.size() > > Which is: 3 > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > WebKitCocoa/ProcessSwapOnNavigation.mm:1425 > > Expected equality of these values: > > pid1 > > Which is: 76472 > > pid3 > > Follow-up fix in <https://trac.webkit.org/changeset/235880>. Dawei, do you still see failures after the fix? I see that you just cc'd a few more people. (In reply to Chris Dumez from comment #12) > (In reply to Chris Dumez from comment #11) > > (In reply to Dawei Fenton (:realdawei) from comment #9) > > > (In reply to WebKit Commit Bot from comment #7) > > > > Comment on attachment 349325 [details] > > > > Patch > > > > > > > > Clearing flags on attachment: 349325 > > > > > > > > Committed r235867: <https://trac.webkit.org/changeset/235867> > > > > > > Seeing API failures on macOS and iOS after this revision > > > > > > sample output: > > > https://build.webkit.org/builders/ > > > Apple%20High%20Sierra%20Release%20WK1%20(Tests)/builds/7888/steps/run-api- > > > tests/logs/stdio > > > > > > TestWebKitAPI.ProcessSwap.ProcessReuse > > > Received data during response processing, queuing it. > > > WARNING: The option to always keep swapped web processes alive is > > > active. This is meant for debugging and testing only. > > > Received data during response processing, queuing it. > > > Received data during response processing, queuing it. > > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > > WebKitCocoa/ProcessSwapOnNavigation.mm:1375 > > > Expected equality of these values: > > > 2u > > > Which is: 2 > > > seenPIDs.size() > > > Which is: 3 > > > > > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > > WebKitCocoa/ProcessSwapOnNavigation.mm:1378 > > > Expected equality of these values: > > > pid1 > > > Which is: 76466 > > > pid3 > > > Which is: 76470 > > > > > > > > > TestWebKitAPI.ProcessSwap.ProcessReuseeTLDPlus2 > > > Received data during response processing, queuing it. > > > WARNING: The option to always keep swapped web processes alive is > > > active. This is meant for debugging and testing only. > > > Received data during response processing, queuing it. > > > Received data during response processing, queuing it. > > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > > WebKitCocoa/ProcessSwapOnNavigation.mm:1422 > > > Expected equality of these values: > > > 2u > > > Which is: 2 > > > seenPIDs.size() > > > Which is: 3 > > > > > > > > > > > > /Volumes/Data/slave/highsierra-release/build/Tools/TestWebKitAPI/Tests/ > > > WebKitCocoa/ProcessSwapOnNavigation.mm:1425 > > > Expected equality of these values: > > > pid1 > > > Which is: 76472 > > > pid3 > > > > Follow-up fix in <https://trac.webkit.org/changeset/235880>. > > Dawei, do you still see failures after the fix? I see that you just cc'd a > few more people. Sorry about that, we generally cc each other to facilitate tracking...I had forgotten to cc earlier :) |