[GTK] Initial view loading is slow
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 11:05:00 +0000 (11:05 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Sep 2019 11:05:00 +0000 (11:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201451

Reviewed by Sergio Villar Senin.

The problem is that now we are always calling DrawingAreaProxy::waitForBackingStoreUpdateOnNextPaint() after a
new process is launched and we used to do that only when launching a new process after a crash. This makes
m_hasReceivedFirstUpdate useless, because it's always set to true right after a process is launched. Then, we
wait up to half a second (which is usually the case for the initial load) until the first update. We only want
to do that when recovering from a crash or when swapping processes to avoid flashing effect.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::launchProcess): Add ProcessLaunchReason parameter and pass it to
finishAttachingToWebProcess instead of IsProcessSwap.
(WebKit::WebPageProxy::swapToWebProcess): Pass ProcessLaunchReason::ProcessSwap to
finishAttachingToWebProcess().
(WebKit::WebPageProxy::finishAttachingToWebProcess): Do not call
DrawingAreaProxy::waitForBackingStoreUpdateOnNextPaint() when process launch reason is ProcessLaunchReason::InitialProcess.
(WebKit::WebPageProxy::launchProcessForReload): Pass ProcessLaunchReason::Reload to launchProcess().
* UIProcess/WebPageProxy.h: Remove IsProcessSwap and add ProcessLaunchReason instead that is passed to
launchProcess and finishAttachingToWebProcess.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249953 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h

index ea1950d..9c23076 100644 (file)
@@ -1,5 +1,29 @@
 2019-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [GTK] Initial view loading is slow
+        https://bugs.webkit.org/show_bug.cgi?id=201451
+
+        Reviewed by Sergio Villar Senin.
+
+        The problem is that now we are always calling DrawingAreaProxy::waitForBackingStoreUpdateOnNextPaint() after a
+        new process is launched and we used to do that only when launching a new process after a crash. This makes
+        m_hasReceivedFirstUpdate useless, because it's always set to true right after a process is launched. Then, we
+        wait up to half a second (which is usually the case for the initial load) until the first update. We only want
+        to do that when recovering from a crash or when swapping processes to avoid flashing effect.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::launchProcess): Add ProcessLaunchReason parameter and pass it to
+        finishAttachingToWebProcess instead of IsProcessSwap.
+        (WebKit::WebPageProxy::swapToWebProcess): Pass ProcessLaunchReason::ProcessSwap to
+        finishAttachingToWebProcess().
+        (WebKit::WebPageProxy::finishAttachingToWebProcess): Do not call
+        DrawingAreaProxy::waitForBackingStoreUpdateOnNextPaint() when process launch reason is ProcessLaunchReason::InitialProcess.
+        (WebKit::WebPageProxy::launchProcessForReload): Pass ProcessLaunchReason::Reload to launchProcess().
+        * UIProcess/WebPageProxy.h: Remove IsProcessSwap and add ProcessLaunchReason instead that is passed to
+        launchProcess and finishAttachingToWebProcess.
+
+2019-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         REGRESSION(r249275): [GTK][WPE] WebPage injected bundle messages no longer work
         https://bugs.webkit.org/show_bug.cgi?id=201865
 
index 827fdb1..9dca8f7 100644 (file)
@@ -722,7 +722,7 @@ void WebPageProxy::handleSynchronousMessage(IPC::Connection& connection, const S
     });
 }
 
-void WebPageProxy::launchProcess(const RegistrableDomain& registrableDomain)
+void WebPageProxy::launchProcess(const RegistrableDomain& registrableDomain, ProcessLaunchReason reason)
 {
     ASSERT(!m_isClosed);
     ASSERT(!hasRunningProcess());
@@ -744,7 +744,7 @@ void WebPageProxy::launchProcess(const RegistrableDomain& registrableDomain)
     m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::Yes);
     m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID, *this);
 
-    finishAttachingToWebProcess(IsProcessSwap::No);
+    finishAttachingToWebProcess(reason);
 }
 
 bool WebPageProxy::suspendCurrentPageIfPossible(API::Navigation& navigation, Optional<FrameIdentifier> mainFrameID, ProcessSwapRequestedByClient processSwapRequestedByClient, ShouldDelayClosingUntilEnteringAcceleratedCompositingMode shouldDelayClosingUntilEnteringAcceleratedCompositingMode)
@@ -818,10 +818,10 @@ void WebPageProxy::swapToWebProcess(Ref<WebProcessProxy>&& process, PageIdentifi
     m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::No);
     m_process->addMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID, *this);
 
-    finishAttachingToWebProcess(IsProcessSwap::Yes);
+    finishAttachingToWebProcess(ProcessLaunchReason::ProcessSwap);
 }
 
-void WebPageProxy::finishAttachingToWebProcess(IsProcessSwap isProcessSwap)
+void WebPageProxy::finishAttachingToWebProcess(ProcessLaunchReason reason)
 {
     ASSERT(m_process->state() != AuxiliaryProcessProxy::State::Terminated);
 
@@ -831,7 +831,7 @@ void WebPageProxy::finishAttachingToWebProcess(IsProcessSwap isProcessSwap)
     didAttachToRunningProcess();
 
     // In the process-swap case, the ProvisionalPageProxy already took care of initializing the WebPage in the WebProcess.
-    if (isProcessSwap != IsProcessSwap::Yes)
+    if (reason != ProcessLaunchReason::ProcessSwap)
         initializeWebPage();
 
     m_inspector->updateForNewPageProcess(this);
@@ -845,7 +845,8 @@ void WebPageProxy::finishAttachingToWebProcess(IsProcessSwap isProcessSwap)
 
     pageClient().didRelaunchProcess();
     m_pageLoadState.didSwapWebProcesses();
-    m_drawingArea->waitForBackingStoreUpdateOnNextPaint();
+    if (reason != ProcessLaunchReason::InitialProcess)
+        m_drawingArea->waitForBackingStoreUpdateOnNextPaint();
 }
 
 void WebPageProxy::didAttachToRunningProcess()
@@ -895,7 +896,7 @@ RefPtr<API::Navigation> WebPageProxy::launchProcessForReload()
     
     ASSERT(!hasRunningProcess());
     auto registrableDomain = m_backForwardList->currentItem() ? RegistrableDomain { URL(URL(), m_backForwardList->currentItem()->url()) } : RegistrableDomain { };
-    launchProcess(registrableDomain);
+    launchProcess(registrableDomain, ProcessLaunchReason::Crash);
 
     if (!m_backForwardList->currentItem()) {
         RELEASE_LOG_IF_ALLOWED(Loading, "launchProcessForReload: no current item to reload");
@@ -927,7 +928,7 @@ RefPtr<API::Navigation> WebPageProxy::launchProcessWithItem(WebBackForwardListIt
     }
 
     ASSERT(!hasRunningProcess());
-    launchProcess(RegistrableDomain { URL(URL(), item.url()) });
+    launchProcess(RegistrableDomain { URL(URL(), item.url()) }, ProcessLaunchReason::InitialProcess);
 
     if (&item != m_backForwardList->currentItem())
         m_backForwardList->goToItem(item);
@@ -1128,7 +1129,7 @@ void WebPageProxy::addPlatformLoadParameters(LoadParameters&)
 WebProcessProxy& WebPageProxy::ensureRunningProcess()
 {
     if (!hasRunningProcess())
-        launchProcess({ });
+        launchProcess({ }, ProcessLaunchReason::InitialProcess);
 
     return m_process;
 }
@@ -1141,7 +1142,7 @@ RefPtr<API::Navigation> WebPageProxy::loadRequest(ResourceRequest&& request, Sho
     RELEASE_LOG_IF_ALLOWED(Loading, "loadRequest:");
 
     if (!hasRunningProcess())
-        launchProcess(RegistrableDomain { request.url() });
+        launchProcess(RegistrableDomain { request.url() }, ProcessLaunchReason::InitialProcess);
 
     auto navigation = m_navigationState->createLoadRequestNavigation(ResourceRequest(request), m_backForwardList->currentItem());
     loadRequestWithNavigationShared(m_process.copyRef(), m_webPageID, navigation.get(), WTFMove(request), shouldOpenExternalURLsPolicy, userData, ShouldTreatAsContinuingLoad::No);
@@ -1204,7 +1205,7 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons
     }
 
     if (!hasRunningProcess())
-        launchProcess({ });
+        launchProcess({ }, ProcessLaunchReason::InitialProcess);
 
     URL fileURL = URL(URL(), fileURLString);
     if (!fileURL.isLocalFile()) {
@@ -1267,7 +1268,7 @@ RefPtr<API::Navigation> WebPageProxy::loadData(const IPC::DataReference& data, c
     }
 
     if (!hasRunningProcess())
-        launchProcess({ });
+        launchProcess({ }, ProcessLaunchReason::InitialProcess);
 
     auto navigation = m_navigationState->createLoadDataNavigation(makeUnique<API::SubstituteData>(data.vector(), MIMEType, encoding, baseURL, userData));
     loadDataWithNavigationShared(m_process.copyRef(), m_webPageID, navigation, data, MIMEType, encoding, baseURL, userData, ShouldTreatAsContinuingLoad::No, WTF::nullopt, shouldOpenExternalURLsPolicy);
@@ -1317,7 +1318,7 @@ void WebPageProxy::loadAlternateHTML(const IPC::DataReference& htmlData, const S
         m_isLoadingAlternateHTMLStringForFailingProvisionalLoad = true;
 
     if (!hasRunningProcess())
-        launchProcess(RegistrableDomain { baseURL });
+        launchProcess(RegistrableDomain { baseURL }, ProcessLaunchReason::InitialProcess);
 
     auto transaction = m_pageLoadState.transaction();
 
@@ -1354,7 +1355,7 @@ void WebPageProxy::loadWebArchiveData(API::Data* webArchiveData, API::Object* us
     }
 
     if (!hasRunningProcess())
-        launchProcess({ });
+        launchProcess({ }, ProcessLaunchReason::InitialProcess);
 
     auto transaction = m_pageLoadState.transaction();
     m_pageLoadState.setPendingAPIRequest(transaction, { 0, WTF::blankURL().string() });
@@ -1384,7 +1385,7 @@ void WebPageProxy::navigateToPDFLinkWithSimulatedClick(const String& urlString,
         return;
 
     if (!hasRunningProcess())
-        launchProcess(RegistrableDomain { URL(URL(), urlString) });
+        launchProcess(RegistrableDomain { URL(URL(), urlString) }, ProcessLaunchReason::InitialProcess);
 
     m_process->send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(urlString, documentPoint, screenPoint), m_webPageID);
     m_process->responsivenessTimer().start();
@@ -3688,7 +3689,7 @@ void WebPageProxy::replaceMatches(Vector<uint32_t>&& matchIndices, const String&
 void WebPageProxy::launchInitialProcessIfNecessary()
 {
     if (&process() == process().processPool().dummyProcessProxy())
-        launchProcess({ });
+        launchProcess({ }, ProcessLaunchReason::InitialProcess);
 }
 
 void WebPageProxy::runJavaScriptInMainFrame(const String& script, bool forceUserGesture, WTF::Function<void (API::SerializedScriptValue*, bool hadException, const ExceptionDetails&, CallbackBase::Error)>&& callbackFunction)
index 200dcef..71d0a1c 100644 (file)
@@ -1747,13 +1747,17 @@ private:
 #endif // ENABLE(NETSCAPE_PLUGIN_API)
     void setCanShortCircuitHorizontalWheelEvents(bool canShortCircuitHorizontalWheelEvents) { m_canShortCircuitHorizontalWheelEvents = canShortCircuitHorizontalWheelEvents; }
 
-    void launchProcess(const WebCore::RegistrableDomain&);
+    enum class ProcessLaunchReason {
+        InitialProcess,
+        ProcessSwap,
+        Crash
+    };
+
+    void launchProcess(const WebCore::RegistrableDomain&, ProcessLaunchReason);
     void swapToWebProcess(Ref<WebProcessProxy>&&, WebCore::PageIdentifier, std::unique_ptr<DrawingAreaProxy>&&, RefPtr<WebFrameProxy>&& mainFrame);
     void didFailToSuspendAfterProcessSwap();
     void didSuspendAfterProcessSwap();
-
-    enum class IsProcessSwap { No, Yes };
-    void finishAttachingToWebProcess(IsProcessSwap);
+    void finishAttachingToWebProcess(ProcessLaunchReason);
 
     RefPtr<API::Navigation> launchProcessForReload();
     RefPtr<API::Navigation> launchProcessWithItem(WebBackForwardListItem&);