GNOME Bugzilla – Bug 695642
[WK2] Call to HasModifiedForms method is sync
Last modified: 2013-08-28 19:28:05 UTC
This needs to be async, otherwise the UI process can lock up for a few seconds in the worst case.
One problem here is that the EphyWebView method this implements is sync, and it is used in EphyWindow in a context that also needs a sync response (a handler for the close request signal). So doing a dbus async call won't work without changing more things. Could we make the WebProcess emit a signal if a webview is about to be closed with modified forms, and let the UI process handle it if it wants to? Seems like it fits more naturally the way things work in WK2.
Regardless of that if seems like this should not look up for seconds, and there are warnings printed in the console, so I think we are just doing something wrong. Possibly calling HasModifiedForms when the WebProcess is already dead/dying, since this happens when we close a tab. Doing that better might be enough for 3.8.
(In reply to comment #1) > Could we make the WebProcess emit a signal if a webview is about to be closed > with modified forms, and let the UI process handle it if it wants to? Seems > like it fits more naturally the way things work in WK2. The problem os this approach is how to prevent the page from closing. To implement that in the web process, first we need to close the web view, so that web process receives the close message, but it's too late, it's just a notification. So, this needs to be done before actually closing the web view from the UI process.
(In reply to comment #2) > Regardless of that if seems like this should not look up for seconds, and there > are warnings printed in the console, so I think we are just doing something > wrong. Possibly calling HasModifiedForms when the WebProcess is already > dead/dying, since this happens when we close a tab. Doing that better might be > enough for 3.8. Looks like a race condition somewhere.
Created attachment 239467 [details] [review] Patch
Review of attachment 239467 [details] [review]: I'm a bit confused, is this patch meant to be for master only? It does not seem like the code in ephy-window.c will compile for WebKit1? ::: src/ephy-window.c @@ +4544,3 @@ { + if (!window->priv->force_close && + gtk_notebook_get_n_pages (window->priv->notebook) > 0) Seems like this can just be one if block now.
(In reply to comment #6) > Review of attachment 239467 [details] [review]: > > I'm a bit confused, is this patch meant to be for master only? It does not seem > like the code in ephy-window.c will compile for WebKit1? Why not? Now ephy_web_view_has_modified_forms() is async even for WebKit1. > ::: src/ephy-window.c > @@ +4544,3 @@ > { > + if (!window->priv->force_close && > + gtk_notebook_get_n_pages (window->priv->notebook) > 0) > > Seems like this can just be one if block now. Ok.
*** Bug 698436 has been marked as a duplicate of this bug. ***
Ping?
*** Bug 706691 has been marked as a duplicate of this bug. ***
Created attachment 253438 [details] [review] ephy-window: Simplify ephy_window_close() Simply use one if block, also put the boolean variable check first.
Attachment 253438 [details] pushed as 91464ee - ephy-window: Simplify ephy_window_close()