Bug 126258 - Crash when starting a download before the network process has been launched
Summary: Crash when starting a download before the network process has been launched
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 126131
  Show dependency treegraph
 
Reported: 2013-12-27 01:15 PST by Carlos Garcia Campos
Modified: 2014-01-09 09:51 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.25 KB, patch)
2013-12-27 01:19 PST, Carlos Garcia Campos
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-12-27 01:15:23 PST
We need to ensure there's a network process running before starting a new download like we do when using the web process. This makes all WebKit2GTK+ download unit tests to crash.
Comment 1 Carlos Garcia Campos 2013-12-27 01:19:50 PST
Created attachment 220054 [details]
Patch
Comment 2 Sergio Villar Senin 2013-12-27 01:30:55 PST
Comment on attachment 220054 [details]
Patch

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

Good job!

> Source/WebKit2/UIProcess/WebContext.cpp:766
> +        networkProcess()->send(Messages::NetworkProcess::DownloadRequest(downloadProxy->downloadID(), request), 0);

I guess this is totally unrelated and just to keep consistency with the rest of the file

> Source/WebKit2/UIProcess/WebContext.cpp:946
>  #endif

Looks correct. I understand that this might be difficult to reproduce in real life situations (as a webprocess should have been spawned and a networkprocess with it) but easily in the unit tests.
Comment 3 Carlos Garcia Campos 2013-12-27 01:35:02 PST
(In reply to comment #2)
> (From update of attachment 220054 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220054&action=review
> 
> Good job!
> 
> > Source/WebKit2/UIProcess/WebContext.cpp:766
> > +        networkProcess()->send(Messages::NetworkProcess::DownloadRequest(downloadProxy->downloadID(), request), 0);
> 
> I guess this is totally unrelated and just to keep consistency with the rest of the file

No, it's not, see the ChageLog.

"Use ChildProcessProxy::send() instead of using the connection to make sure messages are queued"

> > Source/WebKit2/UIProcess/WebContext.cpp:946
> >  #endif
> 
> Looks correct. I understand that this might be difficult to reproduce in real life situations (as a webprocess should have been spawned and a networkprocess with it) but easily in the unit tests.

It's difficult to happen in a web browser, yes, but our API allows to start a download from a WebContext without any web page involved.
Comment 4 Sergio Villar Senin 2013-12-27 01:37:08 PST
Comment on attachment 220054 [details]
Patch

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

>>> Source/WebKit2/UIProcess/WebContext.cpp:766
>>> +        networkProcess()->send(Messages::NetworkProcess::DownloadRequest(downloadProxy->downloadID(), request), 0);
>> 
>> I guess this is totally unrelated and just to keep consistency with the rest of the file
> 
> No, it's not, see the ChageLog.
> 
> "Use ChildProcessProxy::send() instead of using the connection to make sure messages are queued"

Oh yeah I totally overlooked it
Comment 5 Carlos Garcia Campos 2014-01-09 09:51:48 PST
Committed r161559: <http://trac.webkit.org/changeset/161559>