wimp_event, menus and the search dialog

John-Mark Bell jmb202 at ecs.soton.ac.uk
Mon Mar 26 01:16:48 BST 2007

So, I've just spent 5 hours trying to work out why there are 3 crashes on 
the tracker relating to the search/selection code. In the process, I've 
come to the following conclusions:

1) wimp_event never calls the registered window close routine if cancel is
    clicked. This results in a memory leak in 401login.c if nothing else -
    the session struct doesn't get freed.
2) dialogs opened as submenus never have their window close routine called
    when either the menu is closed completely or a different branch is
    selected by the user.
3) closing persistent dialogs doesn't result in window close routines
    being called.
4) there's a hideous amount of collusion between the dialog, menu and
    search code over the search dialog
5) calling ro_gui_search_end() can lead to crashes in some circumstances.

To elaborate on #5:

Calling ro_gui_search_end() causes all the matched search terms to be 
destroyed. In the process, all selections relating to the search are 
destroyed. In the process of selection destruction, the selections are 
cleared, which causes an attempt to redraw the content. So far, so good.

Now, consider this situation:

   You've just attempted to open a new window. NS crashes.

Why is this?

Here's why:

1) The Wimp's reused the handle for a previously existing browser window
2a) When you closed that previous window, the search dialog was still open
    (and, because of #3, above, the search state was never invalidated)
2b) When you closed that previous window, the selection state was still
     valid because, when you closed the search window, it didn't get
     cleaned up (because of #1, above)
3) You've not used search since (so that previous state still exists).
4) As you've closed a window with the reused handle before, the content in
    that window has been expunged from the cache (or, if you're lucky, it's
    still in there)
5) Once the new window's content has fetched, NS checks if the new window
    has the same handle as that of current_search_window. It does, so
    ro_gui_search_end() gets called. This then calls selection_clear() with
    the redraw flag set to true. Once it's invalidated the selection state,
    selection_clear() then calls selection_redraw() which traverses the box
    tree of the _previous window's content_ (and, in one case, miraculously
    fails to crash - quite how, I've no idea) and then calls
    browser_window_redraw_rect() with the rectangle to redraw. This then
    munges the rectangle into a content_msg and calls content_broadcast()
    which, finally, causes NS to SEGV (even though the new window now has a
    current_content which is in a state where redraw is possible)

    [I suspect #2, above, is the reason why NS sometimes crashes when F4 is
     pressed - the state's not cleaned up properly, so the same thing

Fixing this one is currently beyond me - there's just too many things 
interacting with each other in the UI code that I've no idea what impact 
any change I make will have on any other part of the system. Therefore, 
someone else is going to have to fix this one.


More information about the netsurf-dev mailing list