Bug 193779 – Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction():
Thread[0] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000020)
[  0] 0x00007fff4a23502e WebKit`WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) [inlined] WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::RefPtr(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> > const&) at RefPtr.h:58:53

     0x00007fff4a23501e:     movq %rdx, %r15
     0x00007fff4a235021:     movq %rsi, %rbx
     0x00007fff4a235024:     movq %rdi, %r12
     0x00007fff4a235027:     movq 0x138(%rsi), %rax
 ->  0x00007fff4a23502e:     movq 0x20(%rax), %r13
     0x00007fff4a235032:    testq %r13, %r13
     0x00007fff4a235035:       je 0x1c603c             ; <+50> [inlined] WTF::DumbPtrTraits<API::NavigationAction>::unwrap(API::NavigationAction* const&) at Ref.h:119
     0x00007fff4a235037:     addl $0x2, (%r13)
     0x00007fff4a23503c:     movq (%r15), %rax

[  0] 0x00007fff4a23502e WebKit`WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) [inlined] WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> >::RefPtr(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl> > const&) at RefPtr.h:58
[  0] 0x00007fff4a23502e WebKit`WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) [inlined] WTF::String::String(WTF::String const&) at WTFString.h:129
[  0] 0x00007fff4a23502e WebKit`WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) [inlined] WTF::String::String(WTF::String const&) at WTFString.h:129
[  0] 0x00007fff4a23502e WebKit`WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction(WebKit::WebPageProxy&, WTF::Ref<API::NavigationAction, WTF::DumbPtrTraits<API::NavigationAction> >&&, WTF::Ref<WebKit::WebFramePolicyListenerProxy, WTF::DumbPtrTraits<WebKit::WebFramePolicyListenerProxy> >&&, API::Object*) + 36 at NavigationState.mm:490
       486 	
       487 	void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageProxy& webPageProxy, Ref<API::NavigationAction>&& navigationAction, Ref<WebFramePolicyListenerProxy>&& listener, API::Object* userInfo)
       488 	{
       489 	    ASSERT(webPageProxy.mainFrame());
    -> 490 	    String mainFrameURLString = webPageProxy.mainFrame()->url();
       491 	    bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame();
       492 	
       493 	    if (!m_navigationState.m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionDecisionHandler
       494 	        && !m_navigationState.m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionDecisionHandlerWebsitePolicies
    
[  1] 0x00007fff4a28ec10 WebKit`WebKit::WebPageProxy::decidePolicyForNavigationAction(WebKit::WebFrameProxy&, WebCore::SecurityOriginData&&, unsigned long long, WebKit::NavigationActionData&&, WebKit::FrameInfoData&&, unsigned long long, WebCore::ResourceRequest const&, WebCore::ResourceRequest&&, IPC::FormDataReference&&, WebCore::ResourceResponse&&, WebKit::UserData const&, WTF::Ref<WebKit::WebPageProxy::PolicyDecisionSender, WTF::DumbPtrTraits<WebKit::WebPageProxy::PolicyDecisionSender> >&&) + 3212 at WebPageProxy.cpp:4327:29
       4323	        bool shouldOpenAppLinks = !m_shouldSuppressAppLinksInNextNavigationPolicyDecision && destinationFrameInfo->isMainFrame() && !hostsAreEqual(URL({ }, m_mainFrame->url()), request.url()) && navigationActionData.navigationType != WebCore::NavigationType::BackForward;
       4324	
       4325	        auto navigationAction = API::NavigationAction::create(WTFMove(navigationActionData), sourceFrameInfo.get(), destinationFrameInfo.ptr(), std::nullopt, WTFMove(request), originalRequest.url(), shouldOpenAppLinks, WTFMove(userInitiatedActivity), mainFrameNavigation);
       4326	
    -> 4327	        m_navigationClient->decidePolicyForNavigationAction(*this, WTFMove(navigationAction), WTFMove(listener), m_process->transformHandlesToObjects(userData.object()).get());
       4328	    }
       4329	
       4330	    m_shouldSuppressAppLinksInNextNavigationPolicyDecision = false;
       4331	}

[Chris Dumez]

<rdar://problem/46170903>

[Chris Dumez]

Created attachment 360036 [details]
Patch

[EWS Watchlist]

Attachment 360036 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:2139:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:2146:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:2164:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Comment on attachment 360036 [details]
Patch

ping review?

[Antti Koivisto]

Comment on attachment 360036 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360036&action=review

> Source/WebKit/ChangeLog:13
> +        We were crashing when trying to get the URL of the main frame, which was sad because we never
> +        ended up using the main frame URL. Therefore, this patch drops the code in question.

The best fix.

[WebKit Commit Bot]

Comment on attachment 360036 [details]
Patch

Clearing flags on attachment: 360036

Committed r240477: <https://trac.webkit.org/changeset/240477>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

*** Bug 194018 has been marked as a duplicate of this bug. ***