Changeset 239182 in webkit
- Timestamp:
- Dec 13, 2018 3:17:44 PM (5 years ago)
- Location:
- trunk/Source
- Files:
-
- 22 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r239181 r239182 1 2018-12-13 Chris Dumez <cdumez@apple.com> 2 3 [PSON] We should not need to navigate to 'about:blank' to suspend pages 4 https://bugs.webkit.org/show_bug.cgi?id=192668 5 <rdar://problem/46701466> 6 7 Reviewed by Alex Christensen. 8 9 * history/PageCache.cpp: 10 (WebCore::PageCache::addIfCacheable): 11 * history/PageCache.h: 12 * loader/DocumentLoader.cpp: 13 (WebCore::DocumentLoader::redirectReceived): 14 (WebCore::DocumentLoader::willSendRequest): 15 (WebCore::DocumentLoader::startLoadingMainResource): 16 * loader/DocumentLoader.h: 17 * loader/FrameLoader.cpp: 18 (WebCore::FrameLoader::init): 19 (WebCore::FrameLoader::stopAllLoaders): 20 (WebCore::FrameLoader::setDocumentLoader): 21 (WebCore::FrameLoader::commitProvisionalLoad): 22 (WebCore::FrameLoader::continueLoadAfterNavigationPolicy): 23 (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy): 24 * loader/FrameLoaderTypes.h: 25 * loader/PolicyChecker.cpp: 26 (WebCore::PolicyChecker::checkNavigationPolicy): 27 * loader/PolicyChecker.h: 28 1 29 2018-12-13 Per Arne Vollan <pvollan@apple.com> 2 30 -
trunk/Source/WebCore/history/PageCache.cpp
r237446 r239182 426 426 } 427 427 428 voidPageCache::addIfCacheable(HistoryItem& item, Page* page)428 bool PageCache::addIfCacheable(HistoryItem& item, Page* page) 429 429 { 430 430 if (item.isInPageCache()) 431 return ;431 return false; 432 432 433 433 if (!page || !canCache(*page)) 434 return ;434 return false; 435 435 436 436 ASSERT_WITH_MESSAGE(!page->isUtilityPage(), "Utility pages such as SVGImage pages should never go into PageCache"); … … 450 450 if (!canCache(*page)) { 451 451 setPageCacheState(*page, Document::NotInPageCache); 452 return ;452 return false; 453 453 } 454 454 … … 466 466 } 467 467 prune(PruningReason::ReachedMaxSize); 468 return true; 468 469 } 469 470 -
trunk/Source/WebCore/history/PageCache.h
r232098 r239182 52 52 unsigned maxSize() const { return m_maxSize; } 53 53 54 voidaddIfCacheable(HistoryItem&, Page*); // Prunes if maxSize() is exceeded.54 WEBCORE_EXPORT bool addIfCacheable(HistoryItem&, Page*); // Prunes if maxSize() is exceeded. 55 55 WEBCORE_EXPORT void remove(HistoryItem&); 56 56 CachedPage* get(HistoryItem&, Page*); -
trunk/Source/WebCore/loader/DocumentLoader.cpp
r238771 r239182 516 516 #if ENABLE(SERVICE_WORKER) 517 517 bool isRedirectionFromServiceWorker = redirectResponse.source() == ResourceResponse::Source::ServiceWorker; 518 willSendRequest(WTFMove(request), redirectResponse, ShouldContinue::Yes,[isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable {518 willSendRequest(WTFMove(request), redirectResponse, [isRedirectionFromServiceWorker, completionHandler = WTFMove(completionHandler), protectedThis = makeRef(*this), this] (auto&& request) mutable { 519 519 ASSERT(!m_substituteData.isValid()); 520 520 if (request.isNull() || !m_mainDocumentError.isNull() || !m_frame) { … … 549 549 }); 550 550 #else 551 willSendRequest(WTFMove(request), redirectResponse, ShouldContinue::Yes,WTFMove(completionHandler));552 #endif 553 } 554 555 void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, ShouldContinue shouldContinue,CompletionHandler<void(ResourceRequest&&)>&& completionHandler)551 willSendRequest(WTFMove(request), redirectResponse, WTFMove(completionHandler)); 552 #endif 553 } 554 555 void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const ResourceResponse& redirectResponse, CompletionHandler<void(ResourceRequest&&)>&& completionHandler) 556 556 { 557 557 // Note that there are no asserts here as there are for the other callbacks. This is due to the … … 560 560 // callbacks is meant to prevent. 561 561 ASSERT(!newRequest.isNull()); 562 563 ASSERT(shouldContinue != ShouldContinue::No);564 562 565 563 bool didReceiveRedirectResponse = !redirectResponse.isNull(); … … 629 627 setRequest(newRequest); 630 628 631 if (!didReceiveRedirectResponse && shouldContinue != ShouldContinue::ForSuspension)629 if (!didReceiveRedirectResponse) 632 630 return completionHandler(WTFMove(newRequest)); 633 631 … … 635 633 m_waitingForNavigationPolicy = false; 636 634 switch (shouldContinue) { 637 case ShouldContinue::ForSuspension:638 // We handle suspension by navigating forward to about:blank, which leaves us setup to navigate back to resume.639 request = { WTF::blankURL() };640 break;641 635 case ShouldContinue::No: 642 636 stopLoadingForPolicyChange(); … … 651 645 ASSERT(!m_waitingForNavigationPolicy); 652 646 m_waitingForNavigationPolicy = true; 653 654 if (shouldContinue == ShouldContinue::ForSuspension) {655 navigationPolicyCompletionHandler(WTFMove(newRequest), nullptr, shouldContinue);656 return;657 }658 647 659 648 frameLoader()->policyChecker().checkNavigationPolicy(WTFMove(newRequest), redirectResponse, WTFMove(navigationPolicyCompletionHandler)); … … 1694 1683 } 1695 1684 1696 void DocumentLoader::startLoadingMainResource(ShouldContinue shouldContinue) 1697 { 1698 ASSERT(shouldContinue != ShouldContinue::No); 1699 1685 void DocumentLoader::startLoadingMainResource() 1686 { 1700 1687 m_mainDocumentError = ResourceError(); 1701 1688 timing().markStartTimeAndFetchStart(); … … 1723 1710 ASSERT(timing().fetchStart()); 1724 1711 1725 willSendRequest(ResourceRequest(m_request), ResourceResponse(), shouldContinue,[this, protectedThis = WTFMove(protectedThis)] (ResourceRequest&& request) mutable {1712 willSendRequest(ResourceRequest(m_request), ResourceResponse(), [this, protectedThis = WTFMove(protectedThis)] (ResourceRequest&& request) mutable { 1726 1713 m_request = request; 1727 1714 -
trunk/Source/WebCore/loader/DocumentLoader.h
r239046 r239182 248 248 void setMainResourceDataBufferingPolicy(DataBufferingPolicy); 249 249 250 void startLoadingMainResource( ShouldContinue);250 void startLoadingMainResource(); 251 251 WEBCORE_EXPORT void cancelMainResourceLoad(const ResourceError&); 252 252 void willContinueMainResourceLoadAfterRedirect(const ResourceRequest&); … … 368 368 #endif 369 369 370 void willSendRequest(ResourceRequest&&, const ResourceResponse&, ShouldContinue,CompletionHandler<void(ResourceRequest&&)>&&);370 void willSendRequest(ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&); 371 371 void finishedLoading(); 372 372 void mainReceivedError(const ResourceError&); -
trunk/Source/WebCore/loader/FrameLoader.cpp
r239046 r239182 313 313 setPolicyDocumentLoader(m_client.createDocumentLoader(ResourceRequest(URL({ }, emptyString())), SubstituteData()).ptr()); 314 314 setProvisionalDocumentLoader(m_policyDocumentLoader.get()); 315 m_provisionalDocumentLoader->startLoadingMainResource( ShouldContinue::Yes);315 m_provisionalDocumentLoader->startLoadingMainResource(); 316 316 317 317 Ref<Frame> protect(m_frame); … … 1773 1773 void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy) 1774 1774 { 1775 ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache); 1775 if (m_frame.document() && m_frame.document()->pageCacheState() == Document::InPageCache) 1776 return; 1777 1776 1778 if (!isStopLoadingAllowed()) 1777 1779 return; … … 1866 1868 { 1867 1869 if (!loader && !m_documentLoader) 1870 return; 1871 1872 if (loader == m_documentLoader) 1868 1873 return; 1869 1874 … … 1954 1959 willTransitionToCommitted(); 1955 1960 1956 if (!m_frame.tree().parent() && history().currentItem() ) {1961 if (!m_frame.tree().parent() && history().currentItem() && history().currentItem() != history().provisionalItem()) { 1957 1962 // Check to see if we need to cache the page we are navigating away from into the back/forward cache. 1958 1963 // We are doing this here because we know for sure that a new page is about to be loaded. … … 3349 3354 } 3350 3355 3351 CompletionHandler<void()> completionHandler = [this, protectedFrame = makeRef(m_frame) , shouldContinue] () mutable {3356 CompletionHandler<void()> completionHandler = [this, protectedFrame = makeRef(m_frame)] () mutable { 3352 3357 if (!m_provisionalDocumentLoader) 3353 3358 return; 3354 3359 3355 prepareForLoadStart([this, protectedFrame = WTFMove(protectedFrame) , shouldContinue] {3360 prepareForLoadStart([this, protectedFrame = WTFMove(protectedFrame)] { 3356 3361 3357 3362 // The load might be cancelled inside of prepareForLoadStart(), nulling out the m_provisionalDocumentLoader, … … 3370 3375 m_loadingFromCachedPage = false; 3371 3376 3372 // We handle suspension by navigating forward to about:blank, which leaves us setup to navigate back to resume. 3373 if (shouldContinue == ShouldContinue::ForSuspension) { 3374 // Make sure we do a standard load to about:blank instead of reusing whatever the load type was on process swap. 3375 // For example, using a Back/Forward load to load about blank would cause the current HistoryItem's url to be 3376 // overwritten with 'about:blank', which we do not want. 3377 m_loadType = FrameLoadType::Standard; 3378 m_provisionalDocumentLoader->willContinueMainResourceLoadAfterRedirect({ WTF::blankURL() }); 3379 } 3380 3381 m_provisionalDocumentLoader->startLoadingMainResource(shouldContinue); 3377 m_provisionalDocumentLoader->startLoadingMainResource(); 3382 3378 }); 3383 3379 }; … … 3394 3390 FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy) 3395 3391 { 3396 ASSERT(shouldContinue != ShouldContinue::ForSuspension);3397 3392 if (shouldContinue != ShouldContinue::Yes) 3398 3393 return; -
trunk/Source/WebCore/loader/FrameLoaderTypes.h
r237264 r239182 45 45 Download, 46 46 Ignore, 47 Suspend, 47 Suspend, // FIXME: This is only used by WebKit2 so we shouldn't need this in the WebCore enum. 48 48 }; 49 49 -
trunk/Source/WebCore/loader/PolicyChecker.cpp
r238787 r239182 184 184 return function({ }, nullptr, ShouldContinue::No); 185 185 case PolicyAction::Suspend: 186 return function({ WTF::blankURL() }, nullptr, ShouldContinue::ForSuspension);186 RELEASE_ASSERT_NOT_REACHED(); 187 187 case PolicyAction::Use: 188 188 if (!m_frame.loader().client().canHandleRequest(request)) { -
trunk/Source/WebCore/loader/PolicyChecker.h
r238633 r239182 55 55 enum class ShouldContinue { 56 56 Yes, 57 No, 58 ForSuspension 57 No 59 58 }; 60 59 -
trunk/Source/WebKit/ChangeLog
r239170 r239182 1 2018-12-13 Chris Dumez <cdumez@apple.com> 2 3 [PSON] We should not need to navigate to 'about:blank' to suspend pages 4 https://bugs.webkit.org/show_bug.cgi?id=192668 5 <rdar://problem/46701466> 6 7 Reviewed by Alex Christensen. 8 9 To support PageCache when process-swap on cross-site navigation is enabled, 10 we've been navigating the previous process to 'about:blank' when swapping. 11 This would trigger PageCaching of the page in the old process. While 12 convenient, this design has led to a lot of bugs because we did not really 13 want a navigation to happen in the old process. 14 15 To address the issue, when a WebPage is asked to suspend (for process-swap), 16 we now attempt to add it to PageCache and save it on the current HistoryItem, 17 *without* triggering any navigation. Any pending navigation gets cancelled 18 and we just suspend in place. 19 20 Later on, when we want to go back to this HistoryItem, we simply leverage the 21 existing WebPage::goToBackForwardItem() code path. The only subtlety is that 22 we're actually asking the WebPage to load a HistoryItem that is the current 23 one in the History. I had to tweak a some logic / assertions to support this 24 as this is not something we usually do. However, it actually works with very 25 little changes and successfully restores the PageCache entry on the current 26 HistoryItem. 27 28 There is no expected overall behavior change and ProcessSwap API tests (which 29 cover PageCache) still pass. This is merely a simpler design because it avoids 30 navigating to about:blank. 31 32 * UIProcess/SuspendedPageProxy.cpp: 33 (WebKit::SuspendedPageProxy::didSuspend): 34 (WebKit::SuspendedPageProxy::didReceiveMessage): 35 * UIProcess/SuspendedPageProxy.h: 36 * UIProcess/WebPageProxy.cpp: 37 (WebKit::WebPageProxy::didSuspendAfterProcessSwap): 38 * UIProcess/WebPageProxy.h: 39 * UIProcess/WebPageProxy.messages.in: 40 * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp: 41 (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForResponse): 42 * WebProcess/WebPage/WebDocumentLoader.cpp: 43 (WebKit::WebDocumentLoader::setNavigationID): 44 * WebProcess/WebPage/WebFrame.cpp: 45 (WebKit::WebFrame::didReceivePolicyDecision): 46 * WebProcess/WebPage/WebPage.cpp: 47 (WebKit::WebPage::suspendForProcessSwap): 48 * WebProcess/WebPage/WebPage.h: 49 * WebProcess/cocoa/WebProcessCocoa.mm: 50 (WebKit::origin): 51 1 52 2018-12-13 Per Arne Vollan <pvollan@apple.com> 2 53 -
trunk/Source/WebKit/UIProcess/SuspendedPageProxy.cpp
r239080 r239182 131 131 } 132 132 133 void SuspendedPageProxy::did FinishLoad()133 void SuspendedPageProxy::didSuspend() 134 134 { 135 135 LOG(ProcessSwapping, "SuspendedPageProxy %s from process %i finished transition to suspended", loggingString(), m_process->processIdentifier()); 136 136 137 137 m_finishedSuspending = true; 138 139 m_process->send(Messages::WebProcess::UpdateActivePages(), 0);140 138 141 139 #if PLATFORM(IOS_FAMILY) … … 160 158 ASSERT(decoder.messageReceiverName() == Messages::WebPageProxy::messageReceiverName()); 161 159 162 if (decoder.messageName() == Messages::WebPageProxy::Did FinishLoadForFrame::name()) {163 did FinishLoad();160 if (decoder.messageName() == Messages::WebPageProxy::DidSuspendAfterProcessSwap::name()) { 161 didSuspend(); 164 162 return; 165 163 } -
trunk/Source/WebKit/UIProcess/SuspendedPageProxy.h
r239080 r239182 56 56 57 57 private: 58 void did FinishLoad();58 void didSuspend(); 59 59 void didFailToSuspend(); 60 60 -
trunk/Source/WebKit/UIProcess/WebPageProxy.cpp
r239144 r239182 751 751 return false; 752 752 753 if (isPageOpenedByDOMShowingInitialEmptyDocument()) 754 return false; 755 753 756 auto* currentItem = navigation.fromItem(); 754 757 if (!currentItem) { … … 2627 2630 2628 2631 if (processForNavigation.ptr() != &process()) { 2629 policyAction = PolicyAction::Suspend;2632 policyAction = isPageOpenedByDOMShowingInitialEmptyDocument() ? PolicyAction::Ignore : PolicyAction::Suspend; 2630 2633 RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", this, processIdentifier(), processForNavigation->processIdentifier(), reason.utf8().data()); 2631 2634 LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), processForNavigation->processIdentifier(), navigation->navigationID(), navigation->loggingString()); … … 2745 2748 }; 2746 2749 2747 bool isInitialNavigationInNewWindow = openedByDOM() && !hasCommittedAnyProvisionalLoads(); 2748 if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isInitialNavigationInNewWindow || !mainFrameIDInPreviousProcess) { 2750 if (!m_process->processPool().configuration().processSwapsOnWindowOpenWithOpener() || !isPageOpenedByDOMShowingInitialEmptyDocument() || !mainFrameIDInPreviousProcess) { 2749 2751 // There is no way we'll be able to return to the page in the previous page so close it. 2750 2752 if (!didSuspendPreviousPage) … … 2760 2762 } 2761 2763 2764 bool WebPageProxy::isPageOpenedByDOMShowingInitialEmptyDocument() const 2765 { 2766 return openedByDOM() && !hasCommittedAnyProvisionalLoads(); 2767 } 2768 2762 2769 // MSVC gives a redeclaration error if noreturn is used on the definition and not the declaration, while 2763 2770 // Cocoa tests segfault if it is moved to the declaration site (even if we move the definition with it!). … … 2766 2773 #endif 2767 2774 void WebPageProxy::didFailToSuspendAfterProcessSwap() 2775 { 2776 // Only the SuspendedPageProxy should be getting this call. 2777 ASSERT_NOT_REACHED(); 2778 } 2779 2780 #if !COMPILER(MSVC) 2781 NO_RETURN_DUE_TO_ASSERT 2782 #endif 2783 void WebPageProxy::didSuspendAfterProcessSwap() 2768 2784 { 2769 2785 // Only the SuspendedPageProxy should be getting this call. -
trunk/Source/WebKit/UIProcess/WebPageProxy.h
r239133 r239182 1393 1393 void setDefersLoadingForTesting(bool); 1394 1394 1395 bool isPageOpenedByDOMShowingInitialEmptyDocument() const; 1396 1395 1397 WebCore::IntRect syncRootViewToScreen(const WebCore::IntRect& viewRect); 1396 1398 … … 1568 1570 void swapToWebProcess(Ref<WebProcessProxy>&&, std::unique_ptr<SuspendedPageProxy>&&, ShouldDelayAttachingDrawingArea); 1569 1571 void didFailToSuspendAfterProcessSwap(); 1572 void didSuspendAfterProcessSwap(); 1570 1573 1571 1574 void finishAttachingToWebProcess(ShouldDelayAttachingDrawingArea = ShouldDelayAttachingDrawingArea::No); -
trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in
r239080 r239182 506 506 507 507 DidFailToSuspendAfterProcessSwap() 508 DidSuspendAfterProcessSwap() 508 509 509 510 ImageOrMediaDocumentSizeChanged(WebCore::IntSize newSize) -
trunk/Source/WebKit/UIProcess/WebProcessPool.cpp
r239080 r239182 2198 2198 return completionHandler(page.process(), nullptr, "The treatAsSameOriginNavigation flag is set"_s); 2199 2199 2200 bool isInitialLoadInNewWindowOpenedByDOM = page.openedByDOM() && !page.hasCommittedAnyProvisionalLoads();2201 2200 URL sourceURL; 2202 if ( isInitialLoadInNewWindowOpenedByDOM&& !navigation.requesterOrigin().isEmpty())2201 if (page.isPageOpenedByDOMShowingInitialEmptyDocument() && !navigation.requesterOrigin().isEmpty()) 2203 2202 sourceURL = URL { URL(), navigation.requesterOrigin().toString() }; 2204 2203 else -
trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
r238819 r239182 736 736 } 737 737 738 // For suspension loads to about:blank, no need to ask the SuspendedPageProxy.739 if (request.url() == WTF::blankURL() && webPage->isSuspended()) {740 function(PolicyAction::Use);741 return;742 }743 744 738 RefPtr<API::Object> userData; 745 739 -
trunk/Source/WebKit/WebProcess/WebPage/WebDocumentLoader.cpp
r235205 r239182 51 51 { 52 52 ASSERT(navigationID); 53 ASSERT(!m_navigationID || m_navigationID == navigationID);54 53 55 54 m_navigationID = navigationID; -
trunk/Source/WebKit/WebProcess/WebPage/WebFrame.cpp
r239080 r239182 276 276 } 277 277 278 bool shouldSuspend = false; 279 if (action == PolicyAction::Suspend) { 280 shouldSuspend = true; 281 action = PolicyAction::Ignore; 282 } 283 278 284 function(action); 285 286 if (shouldSuspend) 287 page()->suspendForProcessSwap(); 279 288 } 280 289 -
trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp
r239167 r239182 185 185 #include <WebCore/NotImplemented.h> 186 186 #include <WebCore/Page.h> 187 #include <WebCore/PageCache.h> 187 188 #include <WebCore/PageConfiguration.h> 188 189 #include <WebCore/PingLoader.h> … … 1312 1313 } 1313 1314 1315 void WebPage::suspendForProcessSwap() 1316 { 1317 auto failedToSuspend = [this, protectedThis = makeRef(*this)] { 1318 close(); 1319 send(Messages::WebPageProxy::DidFailToSuspendAfterProcessSwap()); 1320 }; 1321 1322 auto* currentHistoryItem = m_mainFrame->coreFrame()->loader().history().currentItem(); 1323 if (!currentHistoryItem) { 1324 failedToSuspend(); 1325 return; 1326 } 1327 1328 if (!PageCache::singleton().addIfCacheable(*currentHistoryItem, corePage())) { 1329 failedToSuspend(); 1330 return; 1331 } 1332 1333 send(Messages::WebPageProxy::DidSuspendAfterProcessSwap()); 1334 } 1335 1314 1336 void WebPage::loadURLInFrame(URL&& url, uint64_t frameID) 1315 1337 { -
trunk/Source/WebKit/WebProcess/WebPage/WebPage.h
r239167 r239182 451 451 void sendClose(); 452 452 453 void suspendForProcessSwap(); 454 453 455 void sendSetWindowFrame(const WebCore::FloatRect&); 454 456 -
trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm
r238771 r239182 461 461 462 462 URL mainFrameURL = { URL(), mainFrame->url() }; 463 if (page.isSuspended()) {464 // Suspended page are navigated to about:blank upon suspension so we really want to report the previous URL.465 if (auto* coreFrame = mainFrame->coreFrame()) {466 if (auto* backHistoryItem = coreFrame->loader().history().previousItem())467 mainFrameURL = { URL(), backHistoryItem->urlString() };468 }469 }470 471 463 Ref<SecurityOrigin> mainFrameOrigin = SecurityOrigin::create(mainFrameURL); 472 464 String mainFrameOriginString;
Note: See TracChangeset
for help on using the changeset viewer.