Bug 153210 – ASSERTION FAILED: !m_mainFrame->coreFrame()->loader().frameHasLoaded() || !m_pendingNavigationID when reloading page while a page is loading

• Component: WebKit2
• Status: RESOLVED FIXED

[Michael Catanzaro]

The little test program pasted below, which calls webkit_web_view_load_uri() twice in a row and webkit_web_view_reload() on LOAD_FINISHED, triggers the following assert:

ASSERTION FAILED: !m_mainFrame->coreFrame()->loader().frameHasLoaded() || !m_pendingNavigationID
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp(1171) : void WebKit::WebPage::reload(uint64_t, bool, bool, const SandboxExtension::Handle &)

We should handle this gracefully. If this is really a programmer error (and while the test program is nonsensical, I do not believe this should be an error) then we should emit a critical warning rather than assert.


/* gcc test.c `pkg-config --cflags --libs webkit2gtk-4.0` */

#include <gtk/gtk.h>
#include <stdio.h>
#include <webkit2/webkit2.h>

void
load_changed_cb (WebKitWebView   *web_view,
                 WebKitLoadEvent  load_event,
                 gpointer         user_data)
{

  if (load_event == WEBKIT_LOAD_FINISHED)
    {
      webkit_web_view_reload (web_view);
      return;
    }
}

int
main (void)
{
  GtkWidget *web_view;
  GtkWidget *window;

  gtk_init (NULL, NULL);

  window = gtk_window_new (GTK_WINDOW_TOPLEVEL);

  web_view = webkit_web_view_new ();
  g_signal_connect (web_view, "load-changed", G_CALLBACK (load_changed_cb), NULL);
  webkit_web_view_load_uri (WEBKIT_WEB_VIEW (web_view), "https://www.gnome.org");
  webkit_web_view_load_uri (WEBKIT_WEB_VIEW (web_view), "https://www.webkit.org");
  gtk_container_add (GTK_CONTAINER (window), web_view);

  gtk_widget_set_size_request (web_view, 600, 500);
  gtk_widget_show_all (window);

  gtk_main ();

  return 0;
}

ASSERTION FAILED: !m_mainFrame->coreFrame()->loader().frameHasLoaded() || !m_pendingNavigationID
../../Source/WebKit2/WebProcess/WebPage/WebPage.cpp(1171) : void WebKit::WebPage::reload(uint64_t, bool, bool, const SandboxExtension::Handle &)
1   0x7f2c14c552dd /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1d) [0x7f2c14c552dd]
2   0x7f2c195f7dd3 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage6reloadEmbbRKNS_16SandboxExtension6HandleE+0xd3) [0x7f2c195f7dd3]
3   0x7f2c198849ca /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC22callMemberFunctionImplIN6WebKit7WebPageEMS2_FvmbbRKNS1_16SandboxExtension6HandleEESt5tupleIJmbbS4_EEJLm0ELm1ELm2ELm3EEEEvPT_T0_OT1_St14index_sequenceIJXspT2_EEE+0xfa) [0x7f2c198849ca]
4   0x7f2c198842ec /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC18callMemberFunctionIN6WebKit7WebPageEMS2_FvmbbRKNS1_16SandboxExtension6HandleEESt5tupleIJmbbS4_EESt19make_index_sequenceILm4EEEEvOT1_PT_T0_+0x6c) [0x7f2c198842ec]
5   0x7f2c1987946a /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC13handleMessageIN8Messages7WebPage6ReloadEN6WebKit7WebPageEMS5_FvmbbRKNS4_16SandboxExtension6HandleEEEEvRNS_14MessageDecoderEPT0_T1_+0x10a) [0x7f2c1987946a]
6   0x7f2c19874302 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage24didReceiveWebPageMessageERN3IPC10ConnectionERNS1_14MessageDecoderE+0xf92) [0x7f2c19874302]
7   0x7f2c195ff8da /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit7WebPage17didReceiveMessageERN3IPC10ConnectionERNS1_14MessageDecoderE+0x17a) [0x7f2c195ff8da]
8   0x7f2c195ff924 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZThn16_N6WebKit7WebPage17didReceiveMessageERN3IPC10ConnectionERNS1_14MessageDecoderE+0x34) [0x7f2c195ff924]
9   0x7f2c191cff68 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC18MessageReceiverMap15dispatchMessageERNS_10ConnectionERNS_14MessageDecoderE+0x118) [0x7f2c191cff68]
10  0x7f2c194541fd /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit10WebProcess17didReceiveMessageERN3IPC10ConnectionERNS1_14MessageDecoderE+0x3d) [0x7f2c194541fd]
11  0x7f2c191ba8c3 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection15dispatchMessageERNS_14MessageDecoderE+0x33) [0x7f2c191ba8c3]
12  0x7f2c191b5376 /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection15dispatchMessageESt10unique_ptrINS_14MessageDecoderESt14default_deleteIS2_EE+0x166) [0x7f2c191b5376]
13  0x7f2c191ba9eb /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection18dispatchOneMessageEv+0x11b) [0x7f2c191ba9eb]
14  0x7f2c191bd4ad /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(+0x3b014ad) [0x7f2c191bd4ad]
15  0x7f2c191bd2cd /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(+0x3b012cd) [0x7f2c191bd2cd]
16  0x7f2c1910a28e /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZNKSt8functionIFvvEEclEv+0x3e) [0x7f2c1910a28e]
17  0x7f2c14c72ada /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(_ZN3WTF7RunLoop11performWorkEv+0x13a) [0x7f2c14c72ada]
18  0x7f2c14cb5fac /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(+0x1999fac) [0x7f2c14cb5fac]
19  0x7f2c14cb5f88 /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(+0x1999f88) [0x7f2c14cb5f88]
20  0x7f2c14cb5f61 /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(+0x1999f61) [0x7f2c14cb5f61]
21  0x7f2c14cb5f08 /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(+0x1999f08) [0x7f2c14cb5f08]
22  0x7f2c10b6e632 /home/mcatanzaro/src/jhbuild/install/lib/libglib-2.0.so.0(+0x47632) [0x7f2c10b6e632]
23  0x7f2c10b70ded /home/mcatanzaro/src/jhbuild/install/lib/libglib-2.0.so.0(g_main_context_dispatch+0x23) [0x7f2c10b70ded]
24  0x7f2c10b70f36 /home/mcatanzaro/src/jhbuild/install/lib/libglib-2.0.so.0(+0x49f36) [0x7f2c10b70f36]
25  0x7f2c10b712ff /home/mcatanzaro/src/jhbuild/install/lib/libglib-2.0.so.0(g_main_loop_run+0x18a) [0x7f2c10b712ff]
26  0x7f2c14cb589f /home/mcatanzaro/src/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18(_ZN3WTF7RunLoop3runEv+0xaf) [0x7f2c14cb589f]
27  0x7f2c197bcf1d /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit16ChildProcessMainINS_10WebProcessENS_14WebProcessMainEEEiiPPc+0xfd) [0x7f2c197bcf1d]
28  0x7f2c197bce0b /home/mcatanzaro/src/jhbuild/install/lib/libwebkit2gtk-4.0.so.37(WebProcessMainUnix+0x1b) [0x7f2c197bce0b]
29  0x400c26 /home/mcatanzaro/src/jhbuild/install/libexec/webkit2gtk-4.0/WebKitWebProcess(main+0x46) [0x400c26]
30  0x7f2c0a623580 /lib64/libc.so.6(__libc_start_main+0xf0) [0x7f2c0a623580]
31  0x400b09 /home/mcatanzaro/src/jhbuild/install/libexec/webkit2gtk-4.0/WebKitWebProcess(_start+0x29) [0x400b09]

[Milan Crha]

I can reproduce this from Evolution as well, when the preview panel is off and when I want to view the message in a separate window. This is with git clone at commit f9379e0855a19767d3a344e574d096cf453694b4:

   Date:   Tue Feb 6 10:55:31 2018 +0000

   [GTK] fast/events/message-channel-gc-4.html is flaky
   https://bugs.webkit.org/show_bug.cgi?id=182104

   ...

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

Could you do anything about it, before it gets into the stable, please?

[Carlos Garcia Campos]

This is not GTK specific. This is what happens:

 1. gnome.org request is made
 2. webkit.org request is made
 3. gnome.org request is stopped, so finished is called (didFailProvisionalLoad in C API)
 4. Reload is called, but the URL is empty because the provisional load failed.
 5. In the web process WebPage::reload() creates a new navigation ID thta is set as pending and FrameLoader::reload() is called, but it returns early because the URL is empty. Because of this createDocumentLoader is not called and the pending navigation not reset.
 6. webkit.org request finishes because the reload did nothing, so it wasn't cancelled. 
 7. Reload is called again and WebPage::reload() asserts because it still has a pending navigation from previous reload.

This might be a more general problem, all other load methods in WebPage expect that the pending navigation is reset after doing the load, but FrameLoader can return early before creating the document loader in other cases too, no only in case of reload. 

There are different ways to solve this:

 - We could simply check after calling reload() in WebPage::reload() is the pending navigation has been reset or not, and reset it, assuming that the reload simply didn't happen in such case.

 - I don't know if the early return in FrameLoader::reload() when the url is empty is correct. The comment says that's expected to happen when a window is created by javascript, but this is not the case. That code has always been there, though (since 2005). Removing the early return fixes the crash.

 - We could add another callback to the client to notify the WebPage from the frame loader when a load or reload request is ignored. This might be a more general solution if this is a problem for other load methods.

[Carlos Garcia Campos]

Created attachment 334612 [details]
Patch

This patch includes unit tests for WebKit C API and WebKitGLib. It also includes the simples fix. I don't know if this is the best (or even right) solution, but at least the unit tests will help to reproduce the problem.

[Carlos Garcia Campos]

Alex, any comment about this?

[Alex Christensen]

Comment on attachment 334612 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:1377
> +    if (m_pendingNavigationID) {

We just set m_pendingNavigationID 3 lines earlier.  Does beginLoad or reloadFrame cause it to be changed?

[Carlos Garcia Campos]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 334612 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334612&action=review
> 
> > Source/WebKit/WebProcess/WebPage/WebPage.cpp:1377
> > +    if (m_pendingNavigationID) {
> 
> We just set m_pendingNavigationID 3 lines earlier.  Does beginLoad or
> reloadFrame cause it to be changed?

It's expected to be reset by FrameLoader::reload() when the document loader is created, but that's not happening because of the early return.

[Michael Catanzaro]

Alex?

[Michael Catanzaro]

Comment on attachment 334612 [details]
Patch

Eeek, this is causing https://bugzilla.redhat.com/show_bug.cgi?id=1560312 so it's more important than I had assumed. Still needs owner approval.

[Carlos Garcia Campos]

Committed r230245: <https://trac.webkit.org/changeset/230245>

[Radar WebKit Bug Importer]

<rdar://problem/39166633>