[Webkit-unassigned] [Bug 105180] [GTK] When the WebProcess crashes, a signal should be emitted

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=105180


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xan.lopez at gmail.com




--- Comment #11 from Xan Lopez <xan.lopez at gmail.com>  2013-03-01 11:24:10 PST ---
(In reply to comment #10)
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:177
> > +     * WebKitWebContext::process-crashed:
> > +     * @context: the #WebKitWebContext
> > +     *
> > +     * This signal is emitted when a process crashes.
> 
> I think this is only emitted when the web process crashes, and as I commented we might want to emit a signal for other processed, but I think it's unlikely. So, we either, leave this as process-crashed and add an enum parameter with the process type, or we name this web-process-crashed.

I think making it extensible without knowing what will happen in the future is weird, so I'd say web-process-crashed for now.

> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h:102
> > +    void (* process_crashed)  (WebKitWebContext *);
> > +
> 
> This class is not inheritable in this moment, and you are not adding the offset to this in the class definition anyway, so just remove it.

Well I *am* adding the offset to the class definition ;) Seems you meant the signal definition, which I indeed forgot to modify for this. But since apparently it's useless to extend this class I'll just get rid of it as you suggest.

> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebExtensions.cpp:75
> > +    g_main_loop_run(test->m_mainLoop);
> > +    g_assert(!result);
> > +    g_assert(processCrashed == TRUE);
> 
> Check the result before running the main loop. Since the main loop is finished in the callback we don't probably need the processCrashed variable, we can leave it to make sure the main loop is not finished elsewhere but I think it's redundant.

OK, I'll just rely on the mainloop finishing.

> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebExtensions.cpp:86
> > +    WebViewTest::add("WebKitWebExtension", "process-crashed", testWebExtensionAbortProcess);
> 
> Nit: what we are testing is process-crashed signal so I would use WebKitWebContext instead of WebKitWebExtension and testWebContextProcessCrashed instead of testWebExtensionAbortProcess

OK.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.