Bug 44079 – [Chromium] fast/events/popup-allowed-from-gesture-initiated-form-submit.html causing debug ASSERT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Nate Chapin]

ASSERTION FAILED: m_expectedClientRedirectDest.protocolIs("javascript") || m_expectedClientRedirectDest == url
(third_party/WebKit/WebKit/chromium/src/FrameLoaderClientImpl.cpp:701 virtual void WebKit::FrameLoaderClientImpl::dispatchDidStartProvisionalLoad())

This test is causing the test that runs immediately after it to hit the above ASSERT.  I suspect r65381, but I'm not yet certain.

[Charles Reis]

I ran into this ASSERT in another context, which I think sheds some light on it.

FrameLoaderClientImpl keeps track of whether a redirect is in progress using m_expectedClientRedirect{Src,Dest}.  If so, this ASSERT is making sure the url of the next navigation matches the expectation.

However, there's at least two cases where this isn't true.  In my case, a navigation can be aborted if the embedder returns PolicyIgnore.  That doesn't clean up this m_expectedClientRedirectSrc field, so we fail the ASSERT.

In the case of this layout test, the form submission targets "_blank", causing it to open in a new window.  However, at the time of FrameLoader::submitForm, the new frame doesn't exist yet.  FrameLoader instead calls scheduleFormSubmission on the current frame, even though the navigation won't occur there.  As a result, we end up expecting the redirect in the wrong frame.  Any future navigations in the form's frame will fail the ASSERT.

There may be more cases where a redirect doesn't get canceled through the expected approach, so I'm inclined to think the ASSERT is broken.  I've put together a patch that changes it into an "if" instead of an assertion, which should make sure we do the right thing in these corner cases.

[Charles Reis]

Created attachment 89734 [details]
Patch

[Charles Reis]

Comment on attachment 89734 [details]
Patch

Layout tests seem to be happy with this change, including the one that had been disabled.

[Darin Fisher (:fishd, Google)]

Comment on attachment 89734 [details]
Patch

OK

[WebKit Commit Bot]

Comment on attachment 89734 [details]
Patch

Clearing flags on attachment: 89734

Committed r84055: <http://trac.webkit.org/changeset/84055>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/84055 might have broken WinCairo Debug (Build)