Modified: branches/safari-606-branch/Source/WebKit/ChangeLog (237388 => 237389)
--- branches/safari-606-branch/Source/WebKit/ChangeLog 2018-10-24 16:56:08 UTC (rev 237388)
+++ branches/safari-606-branch/Source/WebKit/ChangeLog 2018-10-24 17:18:55 UTC (rev 237389)
@@ -1,3 +1,63 @@
+2018-10-24 Kocsen Chung <[email protected]>
+
+ Cherry-pick r236227. rdar://problem/45491949
+
+ Crash under WebPageProxy::decidePolicyForNavigationAction()
+ https://bugs.webkit.org/show_bug.cgi?id=189763
+ <rdar://problem/44597111>
+
+ Reviewed by Alex Christensen.
+
+ Update WebNavigationState::navigation() / WebNavigationState::takeNavigation()
+ to return a pointer instead of a reference as we have evidence that they can
+ return null. I kept the debug assertions to try and catch the cases where we
+ return null but at least we stop crashing in release builds.
+
+ * UIProcess/WebNavigationState.cpp:
+ (WebKit::WebNavigationState::navigation):
+ (WebKit::WebNavigationState::takeNavigation):
+ * UIProcess/WebNavigationState.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didStartProvisionalLoadForFrame):
+ (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
+ (WebKit::WebPageProxy::didCommitLoadForFrame):
+ (WebKit::WebPageProxy::didFinishDocumentLoadForFrame):
+ (WebKit::WebPageProxy::didFinishLoadForFrame):
+ (WebKit::WebPageProxy::didFailLoadForFrame):
+ (WebKit::WebPageProxy::didSameDocumentNavigationForFrame):
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+ (WebKit::WebPageProxy::decidePolicyForResponse):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236227 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-09-19 Chris Dumez <[email protected]>
+
+ Crash under WebPageProxy::decidePolicyForNavigationAction()
+ https://bugs.webkit.org/show_bug.cgi?id=189763
+ <rdar://problem/44597111>
+
+ Reviewed by Alex Christensen.
+
+ Update WebNavigationState::navigation() / WebNavigationState::takeNavigation()
+ to return a pointer instead of a reference as we have evidence that they can
+ return null. I kept the debug assertions to try and catch the cases where we
+ return null but at least we stop crashing in release builds.
+
+ * UIProcess/WebNavigationState.cpp:
+ (WebKit::WebNavigationState::navigation):
+ (WebKit::WebNavigationState::takeNavigation):
+ * UIProcess/WebNavigationState.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didStartProvisionalLoadForFrame):
+ (WebKit::WebPageProxy::didReceiveServerRedirectForProvisionalLoadForFrame):
+ (WebKit::WebPageProxy::didCommitLoadForFrame):
+ (WebKit::WebPageProxy::didFinishDocumentLoadForFrame):
+ (WebKit::WebPageProxy::didFinishLoadForFrame):
+ (WebKit::WebPageProxy::didFailLoadForFrame):
+ (WebKit::WebPageProxy::didSameDocumentNavigationForFrame):
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+ (WebKit::WebPageProxy::decidePolicyForResponse):
+
2018-10-23 Kocsen Chung <[email protected]>
Cherry-pick r237238. rdar://problem/45363879
Modified: branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.cpp (237388 => 237389)
--- branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.cpp 2018-10-24 16:56:08 UTC (rev 237388)
+++ branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.cpp 2018-10-24 17:18:55 UTC (rev 237389)
@@ -78,20 +78,20 @@
return navigation;
}
-API::Navigation& WebNavigationState::navigation(uint64_t navigationID)
+API::Navigation* WebNavigationState::navigation(uint64_t navigationID)
{
ASSERT(navigationID);
ASSERT(m_navigations.contains(navigationID));
- return *m_navigations.get(navigationID);
+ return m_navigations.get(navigationID);
}
-Ref<API::Navigation> WebNavigationState::takeNavigation(uint64_t navigationID)
+RefPtr<API::Navigation> WebNavigationState::takeNavigation(uint64_t navigationID)
{
ASSERT(navigationID);
ASSERT(m_navigations.contains(navigationID));
- return m_navigations.take(navigationID).releaseNonNull();
+ return m_navigations.take(navigationID);
}
void WebNavigationState::didDestroyNavigation(uint64_t navigationID)
Modified: branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.h (237388 => 237389)
--- branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.h 2018-10-24 16:56:08 UTC (rev 237388)
+++ branches/safari-606-branch/Source/WebKit/UIProcess/WebNavigationState.h 2018-10-24 17:18:55 UTC (rev 237389)
@@ -53,8 +53,8 @@
Ref<API::Navigation> createReloadNavigation();
Ref<API::Navigation> createLoadDataNavigation();
- API::Navigation& navigation(uint64_t navigationID);
- Ref<API::Navigation> takeNavigation(uint64_t navigationID);
+ API::Navigation* navigation(uint64_t navigationID);
+ RefPtr<API::Navigation> takeNavigation(uint64_t navigationID);
void didDestroyNavigation(uint64_t navigationID);
void clearAllNavigations();
Modified: branches/safari-606-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (237388 => 237389)
--- branches/safari-606-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-10-24 16:56:08 UTC (rev 237388)
+++ branches/safari-606-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2018-10-24 17:18:55 UTC (rev 237389)
@@ -3469,7 +3469,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
// If this seemingly new load is actually continuing a server redirect for a previous navigation in a new process,
// then we ignore this notification.
@@ -3518,7 +3518,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (navigationID) {
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
navigation->appendRedirectionURL(request.url());
}
@@ -3651,7 +3651,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
m_process->didCommitProvisionalLoad();
@@ -3738,7 +3738,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
if (m_navigationClient) {
if (frame->isMainFrame())
@@ -3759,7 +3759,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
auto transaction = m_pageLoadState.transaction();
@@ -3801,7 +3801,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
clearLoadDependentCallbacks();
@@ -3843,7 +3843,7 @@
// FIXME: We should message check that navigationID is not zero here, but it's currently zero for some navigations through the page cache.
RefPtr<API::Navigation> navigation;
if (frame->isMainFrame() && navigationID)
- navigation = &navigationState().navigation(navigationID);
+ navigation = navigationState().navigation(navigationID);
auto transaction = m_pageLoadState.transaction();
@@ -4026,7 +4026,7 @@
RefPtr<API::Navigation> navigation;
if (navigationID)
- navigation = makeRef(m_navigationState->navigation(navigationID));
+ navigation = m_navigationState->navigation(navigationID);
if (auto targetBackForwardItemIdentifier = navigationActionData.targetBackForwardItemIdentifier) {
if (auto* item = m_backForwardList->itemForID(*navigationActionData.targetBackForwardItemIdentifier)) {
@@ -4059,7 +4059,7 @@
#if ENABLE(CONTENT_FILTERING)
if (frame->didHandleContentFilterUnblockNavigation(request))
- return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, &m_navigationState->navigation(newNavigationID), { });
+ return receivedPolicyDecision(PolicyAction::Ignore, *frame, listenerID, m_navigationState->navigation(newNavigationID), { });
#else
UNUSED_PARAM(newNavigationID);
#endif
@@ -4139,8 +4139,8 @@
Ref<WebFramePolicyListenerProxy> listener = frame->setUpPolicyListenerProxy(listenerID, PolicyListenerType::Response);
if (navigationID) {
- auto& navigation = m_navigationState->navigation(navigationID);
- listener->setNavigation(navigation);
+ if (auto* navigation = m_navigationState->navigation(navigationID))
+ listener->setNavigation(*navigation);
}
if (m_navigationClient) {
