Skip to content

Commit 68c74e1

Browse files
author
[email protected]
committed
WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load
https://bugs.webkit.org/show_bug.cgi?id=173384 <rdar://problem/32723779> Reviewed by Dan Bernstein. Source/WebKit2: WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load. This is because WebPageProxy::stopLoad() starts the responsiveness timer and expects a StopResponsinessTimer IPC from the WebProcess to stop the timer so we don't report the process as unresponsive. However, if WebPageProxy::close() is called before the StopResponsinessTimer IPC has been received, the page will remove itself from the message receiver map and we would no longer be able to receive the StopResponsinessTimer IPC and stop the timer, even if the WebProcess sent it to the UIProcess. To address the issue, we now send the IPC Message to the WebProcessProxy instead of the WebPageProxy, so we can stop the responsiveness timer, even after the WebPageProxy has been called. * UIProcess/WebPageProxy.cpp: * UIProcess/WebPageProxy.h: * UIProcess/WebPageProxy.messages.in: * UIProcess/WebProcessProxy.cpp: (WebKit::WebProcessProxy::stopResponsivenessTimer): * UIProcess/WebProcessProxy.h: * UIProcess/WebProcessProxy.messages.in: * WebProcess/WebPage/WebPage.cpp: (WebKit::SendStopResponsivenessTimer::~SendStopResponsivenessTimer): (WebKit::WebPage::tryClose): (WebKit::WebPage::loadRequest): (WebKit::WebPage::loadDataImpl): (WebKit::WebPage::stopLoading): (WebKit::WebPage::reload): (WebKit::WebPage::goForward): (WebKit::WebPage::goBack): (WebKit::WebPage::goToBackForwardItem): Tools: * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: * TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp: Added. Add API test coverage. * TestWebKitAPI/cocoa/UtilitiesCocoa.mm: (TestWebKitAPI::Util::sleep): Update implementation of Util::sleep() so that we actually run the run loop. Otherwise, we don't process events while sleeping. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@218295 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 22c2a4b commit 68c74e1

File tree

12 files changed

+173
-26
lines changed

12 files changed

+173
-26
lines changed

‎Source/WebKit2/ChangeLog

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,39 @@
1+
2017-06-14 Chris Dumez <[email protected]>
2+
3+
WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load
4+
https://bugs.webkit.org/show_bug.cgi?id=173384
5+
<rdar://problem/32723779>
6+
7+
Reviewed by Dan Bernstein.
8+
9+
WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load.
10+
This is because WebPageProxy::stopLoad() starts the responsiveness timer and expects a StopResponsinessTimer
11+
IPC from the WebProcess to stop the timer so we don't report the process as unresponsive. However, if
12+
WebPageProxy::close() is called before the StopResponsinessTimer IPC has been received, the page will remove
13+
itself from the message receiver map and we would no longer be able to receive the StopResponsinessTimer
14+
IPC and stop the timer, even if the WebProcess sent it to the UIProcess.
15+
16+
To address the issue, we now send the IPC Message to the WebProcessProxy instead of the WebPageProxy, so we
17+
can stop the responsiveness timer, even after the WebPageProxy has been called.
18+
19+
* UIProcess/WebPageProxy.cpp:
20+
* UIProcess/WebPageProxy.h:
21+
* UIProcess/WebPageProxy.messages.in:
22+
* UIProcess/WebProcessProxy.cpp:
23+
(WebKit::WebProcessProxy::stopResponsivenessTimer):
24+
* UIProcess/WebProcessProxy.h:
25+
* UIProcess/WebProcessProxy.messages.in:
26+
* WebProcess/WebPage/WebPage.cpp:
27+
(WebKit::SendStopResponsivenessTimer::~SendStopResponsivenessTimer):
28+
(WebKit::WebPage::tryClose):
29+
(WebKit::WebPage::loadRequest):
30+
(WebKit::WebPage::loadDataImpl):
31+
(WebKit::WebPage::stopLoading):
32+
(WebKit::WebPage::reload):
33+
(WebKit::WebPage::goForward):
34+
(WebKit::WebPage::goBack):
35+
(WebKit::WebPage::goToBackForwardItem):
36+
137
2017-06-14 Commit Queue <[email protected]>
238

339
Unreviewed, rolling out r218263, r218265, and r218266.

‎Source/WebKit2/UIProcess/WebPageProxy.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4996,11 +4996,6 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
49964996
}
49974997
}
49984998

4999-
void WebPageProxy::stopResponsivenessTimer()
5000-
{
5001-
m_process->responsivenessTimer().stop();
5002-
}
5003-
50044999
void WebPageProxy::voidCallback(uint64_t callbackID)
50055000
{
50065001
auto callback = m_callbacks.take<VoidCallback>(callbackID);

‎Source/WebKit2/UIProcess/WebPageProxy.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,6 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
14401440
void setCursorHiddenUntilMouseMoves(bool);
14411441

14421442
void didReceiveEvent(uint32_t opaqueType, bool handled);
1443-
void stopResponsivenessTimer();
14441443

14451444
void voidCallback(uint64_t);
14461445
void dataCallback(const IPC::DataReference&, uint64_t);

‎Source/WebKit2/UIProcess/WebPageProxy.messages.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ messages -> WebPageProxy {
3939
#endif // ENABLE(WEBGL)
4040
DidChangeViewportProperties(struct WebCore::ViewportAttributes attributes)
4141
DidReceiveEvent(uint32_t type, bool handled)
42-
StopResponsivenessTimer()
4342
#if !PLATFORM(IOS)
4443
SetCursor(WebCore::Cursor cursor)
4544
SetCursorHiddenUntilMouseMoves(bool hiddenUntilMouseMoves)

‎Source/WebKit2/UIProcess/WebProcessProxy.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,11 @@ void WebProcessProxy::requestTermination(ProcessTerminationReason reason)
939939
pages[i]->processDidTerminate(reason);
940940
}
941941

942+
void WebProcessProxy::stopResponsivenessTimer()
943+
{
944+
responsivenessTimer().stop();
945+
}
946+
942947
void WebProcessProxy::enableSuddenTermination()
943948
{
944949
if (state() != State::Running)

‎Source/WebKit2/UIProcess/WebProcessProxy.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ class WebProcessProxy : public ChildProcessProxy, public ResponsivenessTimer::Cl
151151

152152
void requestTermination(ProcessTerminationReason);
153153

154+
void stopResponsivenessTimer();
155+
154156
RefPtr<API::Object> transformHandlesToObjects(API::Object*);
155157
static RefPtr<API::Object> transformObjectsToHandles(API::Object*);
156158

‎Source/WebKit2/UIProcess/WebProcessProxy.messages.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ messages -> WebProcessProxy LegacyReceiver {
4949
DidExceedInactiveMemoryLimit()
5050
DidExceedCPULimit()
5151

52+
StopResponsivenessTimer()
53+
5254
RetainIconForPageURL(String pageURL)
5355
ReleaseIconForPageURL(String pageURL)
5456

‎Source/WebKit2/WebProcess/WebPage/WebPage.cpp

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -265,18 +265,10 @@ static const Seconds maximumLayerVolatilityTimerInterval { 2_s };
265265

266266
class SendStopResponsivenessTimer {
267267
public:
268-
SendStopResponsivenessTimer(WebPage* page)
269-
: m_page(page)
270-
{
271-
}
272-
273268
~SendStopResponsivenessTimer()
274269
{
275-
m_page->send(Messages::WebPageProxy::StopResponsivenessTimer());
270+
WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
276271
}
277-
278-
private:
279-
WebPage* m_page;
280272
};
281273

282274
class DeferredPageDestructor {
@@ -1177,10 +1169,10 @@ void WebPage::close()
11771169

11781170
void WebPage::tryClose()
11791171
{
1180-
SendStopResponsivenessTimer stopper(this);
1172+
SendStopResponsivenessTimer stopper;
11811173

11821174
if (!corePage()->userInputBridge().tryClosePage()) {
1183-
send(Messages::WebPageProxy::StopResponsivenessTimer());
1175+
WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::StopResponsivenessTimer(), 0);
11841176
return;
11851177
}
11861178

@@ -1209,7 +1201,7 @@ void WebPage::platformDidReceiveLoadParameters(const LoadParameters& loadParamet
12091201

12101202
void WebPage::loadRequest(const LoadParameters& loadParameters)
12111203
{
1212-
SendStopResponsivenessTimer stopper(this);
1204+
SendStopResponsivenessTimer stopper;
12131205

12141206
m_pendingNavigationID = loadParameters.navigationID;
12151207

@@ -1233,7 +1225,7 @@ void WebPage::loadRequest(const LoadParameters& loadParameters)
12331225

12341226
void WebPage::loadDataImpl(uint64_t navigationID, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
12351227
{
1236-
SendStopResponsivenessTimer stopper(this);
1228+
SendStopResponsivenessTimer stopper;
12371229

12381230
m_pendingNavigationID = navigationID;
12391231

@@ -1317,7 +1309,7 @@ void WebPage::stopLoadingFrame(uint64_t frameID)
13171309

13181310
void WebPage::stopLoading()
13191311
{
1320-
SendStopResponsivenessTimer stopper(this);
1312+
SendStopResponsivenessTimer stopper;
13211313

13221314
corePage()->userInputBridge().stopLoadingFrame(m_mainFrame->coreFrame());
13231315
}
@@ -1334,7 +1326,7 @@ void WebPage::setDefersLoading(bool defersLoading)
13341326

13351327
void WebPage::reload(uint64_t navigationID, uint32_t reloadOptions, const SandboxExtension::Handle& sandboxExtensionHandle)
13361328
{
1337-
SendStopResponsivenessTimer stopper(this);
1329+
SendStopResponsivenessTimer stopper;
13381330

13391331
ASSERT(!m_mainFrame->coreFrame()->loader().frameHasLoaded() || !m_pendingNavigationID);
13401332
m_pendingNavigationID = navigationID;
@@ -1345,7 +1337,7 @@ void WebPage::reload(uint64_t navigationID, uint32_t reloadOptions, const Sandbo
13451337

13461338
void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
13471339
{
1348-
SendStopResponsivenessTimer stopper(this);
1340+
SendStopResponsivenessTimer stopper;
13491341

13501342
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
13511343
ASSERT(item);
@@ -1360,7 +1352,7 @@ void WebPage::goForward(uint64_t navigationID, uint64_t backForwardItemID)
13601352

13611353
void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
13621354
{
1363-
SendStopResponsivenessTimer stopper(this);
1355+
SendStopResponsivenessTimer stopper;
13641356

13651357
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
13661358
ASSERT(item);
@@ -1375,7 +1367,7 @@ void WebPage::goBack(uint64_t navigationID, uint64_t backForwardItemID)
13751367

13761368
void WebPage::goToBackForwardItem(uint64_t navigationID, uint64_t backForwardItemID)
13771369
{
1378-
SendStopResponsivenessTimer stopper(this);
1370+
SendStopResponsivenessTimer stopper;
13791371

13801372
HistoryItem* item = WebBackForwardListProxy::itemForID(backForwardItemID);
13811373
ASSERT(item);

‎Tools/ChangeLog

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,20 @@
1+
2017-06-14 Chris Dumez <[email protected]>
2+
3+
WebKit falsely reports that a web process is unresponsive if you close a page shortly after stopping a load
4+
https://bugs.webkit.org/show_bug.cgi?id=173384
5+
<rdar://problem/32723779>
6+
7+
Reviewed by Dan Bernstein.
8+
9+
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
10+
* TestWebKitAPI/Tests/WebKit2/ResponsivenessTimer.cpp: Added.
11+
Add API test coverage.
12+
13+
* TestWebKitAPI/cocoa/UtilitiesCocoa.mm:
14+
(TestWebKitAPI::Util::sleep):
15+
Update implementation of Util::sleep() so that we actually run the run loop.
16+
Otherwise, we don't process events while sleeping.
17+
118
2017-06-14 Alex Christensen <[email protected]>
219

320
Add SPI for immediate injection of user scripts

‎Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@
484484
8361F1781E610B4E00759B25 /* link-with-download-attribute-with-slashes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 8361F1771E610B2100759B25 /* link-with-download-attribute-with-slashes.html */; };
485485
837A35F11D9A1E7D00663C57 /* DownloadRequestBlobURL.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */; };
486486
83B6DE6F1EE75221001E792F /* RestoreSessionState.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */; };
487+
83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */; };
487488
8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8E4A85361E1D1AA100F53B0F /* GridPosition.cpp */; };
488489
930AD402150698D00067970F /* lots-of-text.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 930AD401150698B30067970F /* lots-of-text.html */; };
489490
9329AA291DE3F81E003ABD07 /* TextBreakIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9329AA281DE3F81E003ABD07 /* TextBreakIterator.cpp */; };
@@ -1293,6 +1294,7 @@
12931294
837A35F01D9A1E6400663C57 /* DownloadRequestBlobURL.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = DownloadRequestBlobURL.html; sourceTree = "<group>"; };
12941295
83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RestoreSessionState.cpp; sourceTree = "<group>"; };
12951296
83B88A331C80056D00BB2418 /* HTMLParserIdioms.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HTMLParserIdioms.cpp; sourceTree = "<group>"; };
1297+
83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResponsivenessTimer.cpp; sourceTree = "<group>"; };
12961298
86BD19971A2DB05B006DCF0A /* RefCounter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RefCounter.cpp; sourceTree = "<group>"; };
12971299
8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ResizeWindowAfterCrash.cpp; sourceTree = "<group>"; };
12981300
8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ReloadPageAfterCrash.cpp; sourceTree = "<group>"; };
@@ -2189,6 +2191,7 @@
21892191
8A3AF93A16C9ED2700D248C1 /* ReloadPageAfterCrash.cpp */,
21902192
2DD7D3A9178205D00026E1E3 /* ResizeReversePaginatedWebView.cpp */,
21912193
8A2C750D16CED9550024F352 /* ResizeWindowAfterCrash.cpp */,
2194+
83DE134C1EF1C4FE00C1B355 /* ResponsivenessTimer.cpp */,
21922195
C0BD669C131D3CF700E18F2A /* ResponsivenessTimerDoesntFireEarly.cpp */,
21932196
C0BD669E131D3CFF00E18F2A /* ResponsivenessTimerDoesntFireEarly_Bundle.cpp */,
21942197
83B6DE6E1EE7520F001E792F /* RestoreSessionState.cpp */,
@@ -2983,6 +2986,7 @@
29832986
7A95BDE11E9BEC5F00865498 /* InjectedBundleAppleEvent.cpp in Sources */,
29842987
510477781D29923B009747EB /* IDBDeleteRecovery.mm in Sources */,
29852988
5110FCFA1E01CDB8006F8D0B /* IDBIndexUpgradeToV2.mm in Sources */,
2989+
83DE134D1EF1C50800C1B355 /* ResponsivenessTimer.cpp in Sources */,
29862990
51A587861D273AA9004BA9AF /* IndexedDBDatabaseProcessKill.mm in Sources */,
29872991
7C83E0BE1D0A651300FEBCF3 /* IndexedDBMultiProcess.mm in Sources */,
29882992
7C83E0BF1D0A652200FEBCF3 /* IndexedDBPersistence.mm in Sources */,

0 commit comments

Comments
 (0)