Skip to content

Commit 13ddea1

Browse files
szewaiszewai
committed
Webpages flash when getting closed
https://bugs.webkit.org/show_bug.cgi?id=216131 <rdar://problem/62264106> Reviewed by Chris Dumez. Closing a page in web process will detach root CA layer and clears content in view immediately. If this happens in web process before transaction, which contains view hierachy change or layer change, is committed in UI process, we would see a white flash. To try fixing this issue, we explicitly delay sending the close message to next runloop cycle. * UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::close): (WebKit::WebPageProxy::receivedNavigationPolicyDecision): * UIProcess/WebProcessProxy.cpp: (WebKit::m_shutdownPreventingScopeCounter): (WebKit::WebProcessProxy::canTerminateAuxiliaryProcess): * UIProcess/WebProcessProxy.h: remove class ScopePreventingShutdown and use RefCounter to prevent process shutdown. (WebKit::WebProcessProxy::shutdownPreventingScope): (WebKit::WebProcessProxy::ScopePreventingShutdown::ScopePreventingShutdown): Deleted. (WebKit::WebProcessProxy::ScopePreventingShutdown::~ScopePreventingShutdown): Deleted. (WebKit::WebProcessProxy::makeScopePreventingShutdown): Deleted. Canonical link: https://commits.webkit.org/229509@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267250 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 6804ca0 commit 13ddea1

4 files changed

Lines changed: 38 additions & 24 deletions

File tree

‎Source/WebKit/ChangeLog‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
2020-09-18 Sihui Liu <sihui_liu@apple.com>
2+
3+
Webpages flash when getting closed
4+
https://bugs.webkit.org/show_bug.cgi?id=216131
5+
<rdar://problem/62264106>
6+
7+
Reviewed by Chris Dumez.
8+
9+
Closing a page in web process will detach root CA layer and clears content in view immediately. If this happens
10+
in web process before transaction, which contains view hierachy change or layer change, is committed in UI
11+
process, we would see a white flash. To try fixing this issue, we explicitly delay sending the close message
12+
to next runloop cycle.
13+
14+
* UIProcess/WebPageProxy.cpp:
15+
(WebKit::WebPageProxy::close):
16+
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
17+
* UIProcess/WebProcessProxy.cpp:
18+
(WebKit::m_shutdownPreventingScopeCounter):
19+
(WebKit::WebProcessProxy::canTerminateAuxiliaryProcess):
20+
* UIProcess/WebProcessProxy.h: remove class ScopePreventingShutdown and use RefCounter to prevent process
21+
shutdown.
22+
(WebKit::WebProcessProxy::shutdownPreventingScope):
23+
(WebKit::WebProcessProxy::ScopePreventingShutdown::ScopePreventingShutdown): Deleted.
24+
(WebKit::WebProcessProxy::ScopePreventingShutdown::~ScopePreventingShutdown): Deleted.
25+
(WebKit::WebProcessProxy::makeScopePreventingShutdown): Deleted.
26+
127
2020-09-18 Youenn Fablet <youenn@apple.com>
228

329
Add internal flag to enable/disable H264 hardware encoder

‎Source/WebKit/UIProcess/WebPageProxy.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,7 +1145,9 @@ void WebPageProxy::close()
11451145

11461146
m_process->processPool().backForwardCache().removeEntriesForPage(*this);
11471147

1148-
send(Messages::WebPage::Close());
1148+
RunLoop::current().dispatch([destinationID = messageSenderDestinationID(), protectedProcess = m_process.copyRef(), preventProcessShutdownScope = m_process->shutdownPreventingScope()] {
1149+
protectedProcess->send(Messages::WebPage::Close(), destinationID);
1150+
});
11491151
m_process->removeWebPage(*this, WebProcessProxy::EndsUsingDataStore::Yes);
11501152
m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
11511153
m_process->processPool().supplement<WebNotificationManagerProxy>()->clearNotifications(this);
@@ -3305,7 +3307,7 @@ void WebPageProxy::receivedNavigationPolicyDecision(PolicyAction policyAction, A
33053307
Optional<SandboxExtension::Handle> optionalHandle;
33063308
if (shouldProcessSwap) {
33073309
// Make sure the process to be used for the navigation does not get shutDown now due to destroying SuspendedPageProxy or ProvisionalPageProxy objects.
3308-
auto preventNavigationProcessShutdown = processForNavigation->makeScopePreventingShutdown();
3310+
auto preventNavigationProcessShutdown = processForNavigation->shutdownPreventingScope();
33093311

33103312
ASSERT(!destinationSuspendedPage || navigation->targetItem());
33113313
auto suspendedPage = destinationSuspendedPage ? backForwardCache().takeSuspendedPage(*navigation->targetItem()) : nullptr;

‎Source/WebKit/UIProcess/WebProcessProxy.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ WebProcessProxy::WebProcessProxy(WebProcessPool& processPool, WebsiteDataStore*
201201
, m_userMediaCaptureManagerProxy(makeUnique<UserMediaCaptureManagerProxy>(makeUniqueRef<UIProxyForCapture>(*this)))
202202
#endif
203203
, m_isPrewarmed(isPrewarmed == IsPrewarmed::Yes)
204+
, m_shutdownPreventingScopeCounter([this](RefCounterEvent event) { if (event == RefCounterEvent::Decrement) maybeShutDown(); })
204205
{
205206
RELEASE_ASSERT(isMainThreadOrCheckDisabled());
206207

@@ -1091,7 +1092,7 @@ void WebProcessProxy::maybeShutDown()
10911092

10921093
bool WebProcessProxy::canTerminateAuxiliaryProcess()
10931094
{
1094-
if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache || m_shutdownPreventingScopeCount)
1095+
if (!m_pageMap.isEmpty() || m_suspendedPageCount || !m_provisionalPages.isEmpty() || m_isInProcessCache || m_shutdownPreventingScopeCounter.value())
10951096
return false;
10961097

10971098
if (isRunningServiceWorkers())

‎Source/WebKit/UIProcess/WebProcessProxy.h‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -294,26 +294,9 @@ class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer
294294
// Will potentially cause the WebProcessProxy object to be freed.
295295
void shutDown();
296296

297-
class ScopePreventingShutdown {
298-
public:
299-
explicit ScopePreventingShutdown(WebProcessProxy& process)
300-
: m_process(process)
301-
{
302-
++(m_process->m_shutdownPreventingScopeCount);
303-
}
304-
305-
~ScopePreventingShutdown()
306-
{
307-
ASSERT(m_process->m_shutdownPreventingScopeCount);
308-
if (!--(m_process->m_shutdownPreventingScopeCount))
309-
m_process->maybeShutDown();
310-
}
311-
312-
private:
313-
Ref<WebProcessProxy> m_process;
314-
};
315-
316-
ScopePreventingShutdown makeScopePreventingShutdown() { return ScopePreventingShutdown { *this }; }
297+
enum ShutdownPreventingScopeType { };
298+
using ShutdownPreventingScopeCounter = RefCounter<ShutdownPreventingScopeType>;
299+
ShutdownPreventingScopeCounter::Token shutdownPreventingScope() { return m_shutdownPreventingScopeCounter.count(); }
317300

318301
void didStartProvisionalLoadForMainFrame(const URL&);
319302

@@ -586,7 +569,7 @@ class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer
586569
#endif
587570

588571
unsigned m_suspendedPageCount { 0 };
589-
unsigned m_shutdownPreventingScopeCount { 0 };
572+
590573
bool m_hasCommittedAnyProvisionalLoads { false };
591574
bool m_isPrewarmed;
592575
#if ENABLE(ATTACHMENT_ELEMENT) && PLATFORM(IOS_FAMILY)
@@ -625,6 +608,8 @@ class WebProcessProxy : public AuxiliaryProcessProxy, public ResponsivenessTimer
625608
WebProcessWithAudibleMediaToken token;
626609
};
627610
Optional<AudibleMediaActivity> m_audibleMediaActivity;
611+
612+
ShutdownPreventingScopeCounter m_shutdownPreventingScopeCounter;
628613
};
629614

630615
} // namespace WebKit

0 commit comments

Comments
 (0)