<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<!DOCTYPE bugzilla SYSTEM "https://bugs.webkit.org/page.cgi?id=bugzilla.dtd">

<bugzilla version="5.0.4.1"
          urlbase="https://bugs.webkit.org/"
          
          maintainer="admin@webkit.org"
>

    <bug>
          <bug_id>87953</bug_id>
          
          <creation_ts>2012-05-31 04:18:58 -0700</creation_ts>
          <short_desc>[SOUP] WebProcess crashes when a download is started from an existing ResourceHandle</short_desc>
          <delta_ts>2012-09-27 00:37:03 -0700</delta_ts>
          <reporter_accessible>1</reporter_accessible>
          <cclist_accessible>1</cclist_accessible>
          <classification_id>1</classification_id>
          <classification>Unclassified</classification>
          <product>WebKit</product>
          <component>WebKit2</component>
          <version>528+ (Nightly build)</version>
          <rep_platform>PC</rep_platform>
          <op_sys>Linux</op_sys>
          <bug_status>RESOLVED</bug_status>
          <resolution>FIXED</resolution>
          
          
          <bug_file_loc></bug_file_loc>
          <status_whiteboard></status_whiteboard>
          <keywords>Gtk, Soup</keywords>
          <priority>P2</priority>
          <bug_severity>Normal</bug_severity>
          <target_milestone>---</target_milestone>
          
          
          <everconfirmed>1</everconfirmed>
          <reporter name="Carlos Garcia Campos">cgarcia</reporter>
          <assigned_to name="Nobody">webkit-unassigned</assigned_to>
          <cc>gustavo</cc>
    
    <cc>mikhail.pozdnyakov</cc>
    
    <cc>svillar</cc>
          

      

      

      

          <comment_sort_order>oldest_to_newest</comment_sort_order>  
          <long_desc isprivate="0" >
    <commentid>638214</commentid>
    <comment_count>0</comment_count>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-05-31 04:18:58 -0700</bug_when>
    <thetext>The output stream to write the downloaded data is created in the didReceiveResponse callback of the download client. When a download is created for an existing ResourceHandle (this happens for example when policy decision is download), the response has already been received. In this case we should make sure that the download client is notified about the response.</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638217</commentid>
    <comment_count>1</comment_count>
      <attachid>145045</attachid>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-05-31 04:21:57 -0700</bug_when>
    <thetext>Created attachment 145045
Patch</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638313</commentid>
    <comment_count>2</comment_count>
      <attachid>145045</attachid>
    <who name="Martin Robinson">mrobinson</who>
    <bug_when>2012-05-31 06:16:07 -0700</bug_when>
    <thetext>Comment on attachment 145045
Patch

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

&gt; Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154
&gt; +    // If the handle already got a response, make sure the download client is notified.
&gt; +    ResourceHandleInternal* handleInternal = m_resourceHandle-&gt;getInternal();
&gt; +    if (!handleInternal-&gt;m_response.isNull())
&gt; +        m_downloadClient-&gt;didReceiveResponse(m_resourceHandle.get(), handleInternal-&gt;m_response);

Is there a chance that data has already been received or the request already failed as well? If that was the case then we&apos;d have to forward those events. Or is the resource handle paused during policy decisions?</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638437</commentid>
    <comment_count>3</comment_count>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-05-31 08:18:12 -0700</bug_when>
    <thetext>(In reply to comment #2)
&gt; (From update of attachment 145045 [details])
&gt; View in context: https://bugs.webkit.org/attachment.cgi?id=145045&amp;action=review
&gt; 
&gt; &gt; Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154
&gt; &gt; +    // If the handle already got a response, make sure the download client is notified.
&gt; &gt; +    ResourceHandleInternal* handleInternal = m_resourceHandle-&gt;getInternal();
&gt; &gt; +    if (!handleInternal-&gt;m_response.isNull())
&gt; &gt; +        m_downloadClient-&gt;didReceiveResponse(m_resourceHandle.get(), handleInternal-&gt;m_response);
&gt; 
&gt; Is there a chance that data has already been received or the request already failed as well? If that was the case then we&apos;d have to forward those events. Or is the resource handle paused during policy decisions?

Policy decisions happen before data is sent, when the response has been received. If it fails, it&apos;s not converted to a download.</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638447</commentid>
    <comment_count>4</comment_count>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-05-31 08:30:08 -0700</bug_when>
    <thetext>Committed r119107: &lt;http://trac.webkit.org/changeset/119107&gt;</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638472</commentid>
    <comment_count>5</comment_count>
      <attachid>145045</attachid>
    <who name="Martin Robinson">mrobinson</who>
    <bug_when>2012-05-31 08:57:16 -0700</bug_when>
    <thetext>Comment on attachment 145045
Patch

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

&gt;&gt;&gt; Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154
&gt;&gt;&gt; +        m_downloadClient-&gt;didReceiveResponse(m_resourceHandle.get(), handleInternal-&gt;m_response);
&gt;&gt; 
&gt;&gt; Is there a chance that data has already been received or the request already failed as well? If that was the case then we&apos;d have to forward those events. Or is the resource handle paused during policy decisions?
&gt; 
&gt; Policy decisions happen before data is sent, when the response has been received. If it fails, it&apos;s not converted to a download.

The download client still handles didReceiveData and didFail though...</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>638478</commentid>
    <comment_count>6</comment_count>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-05-31 09:01:37 -0700</bug_when>
    <thetext>(In reply to comment #5)
&gt; (From update of attachment 145045 [details])
&gt; View in context: https://bugs.webkit.org/attachment.cgi?id=145045&amp;action=review
&gt; 
&gt; &gt;&gt;&gt; Source/WebKit2/WebProcess/Downloads/soup/DownloadSoup.cpp:154
&gt; &gt;&gt;&gt; +        m_downloadClient-&gt;didReceiveResponse(m_resourceHandle.get(), handleInternal-&gt;m_response);
&gt; &gt;&gt; 
&gt; &gt;&gt; Is there a chance that data has already been received or the request already failed as well? If that was the case then we&apos;d have to forward those events. Or is the resource handle paused during policy decisions?
&gt; &gt; 
&gt; &gt; Policy decisions happen before data is sent, when the response has been received. If it fails, it&apos;s not converted to a download.
&gt; 
&gt; The download client still handles didReceiveData and didFail though...

once the download client has been set as the handle client, if it fails, it will be handled by the download, and of course didReceiveData is handled to write the downloaded data.</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>728561</commentid>
    <comment_count>7</comment_count>
    <who name="Mikhail Pozdnyakov">mikhail.pozdnyakov</who>
    <bug_when>2012-09-26 08:01:45 -0700</bug_when>
    <thetext>Could you please clarify what kind of crashes there used to be? Layout test names, crash logs..

The landed patch caused crash of fast/loader/reload-zero-byte-plugin.html on WK2 EFL (and most probably on WK2 GTK as well). Please take a look at 
https://bugs.webkit.org/show_bug.cgi?id=97565</thetext>
  </long_desc><long_desc isprivate="0" >
    <commentid>729283</commentid>
    <comment_count>8</comment_count>
    <who name="Carlos Garcia Campos">cgarcia</who>
    <bug_when>2012-09-27 00:37:03 -0700</bug_when>
    <thetext>(In reply to comment #7)
&gt; Could you please clarify what kind of crashes there used to be? Layout test names, crash logs..
&gt; 
&gt; The landed patch caused crash of fast/loader/reload-zero-byte-plugin.html on WK2 EFL (and most probably on WK2 GTK as well). Please take a look at 
&gt; https://bugs.webkit.org/show_bug.cgi?id=97565

The problem was that the output stream is created when handling the response, so if we receive data and and the response hasn&apos;t been handled, there&apos;s no output stream to write to. When the ResourceHandle is converted into a download, typically by the policy client, the response has already been received, but the download client hasn&apos;t been notified because the download is created after the response is received. So it always crashed when a download was started by the policy client for example.</thetext>
  </long_desc>
      
          <attachment
              isobsolete="0"
              ispatch="1"
              isprivate="0"
          >
            <attachid>145045</attachid>
            <date>2012-05-31 04:21:57 -0700</date>
            <delta_ts>2012-05-31 08:57:16 -0700</delta_ts>
            <desc>Patch</desc>
            <filename>wk2-downloads-crash.diff</filename>
            <type>text/plain</type>
            <size>2265</size>
            <attacher name="Carlos Garcia Campos">cgarcia</attacher>
            
              <data encoding="base64">ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No
YW5nZUxvZwppbmRleCA1Njg0YzNiLi4wYzQ1MWVjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0
Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEg
QEAKKzIwMTItMDUtMzEgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t
PgorCisgICAgICAgIFtTT1VQXSBXZWJQcm9jZXNzIGNyYXNoZXMgd2hlbiBhIGRvd25sb2FkIGlz
IHN0YXJ0ZWQgZnJvbSBhbiBleGlzdGluZyBSZXNvdXJjZUhhbmRsZQorICAgICAgICBodHRwczov
L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODc5NTMKKworICAgICAgICBSZXZpZXdl
ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgb3V0cHV0IHN0cmVhbSB0byB3cml0
ZSB0aGUgZG93bmxvYWRlZCBkYXRhIGlzIGNyZWF0ZWQgaW4gdGhlCisgICAgICAgIGRpZFJlY2Vp
dmVSZXNwb25zZSBjYWxsYmFjayBvZiB0aGUgZG93bmxvYWQgY2xpZW50LiBXaGVuIGEKKyAgICAg
ICAgZG93bmxvYWQgaXMgY3JlYXRlZCBmb3IgYW4gZXhpc3RpbmcgUmVzb3VyY2VIYW5kbGUgKHRo
aXMgaGFwcGVucworICAgICAgICBmb3IgZXhhbXBsZSB3aGVuIHBvbGljeSBkZWNpc2lvbiBpcyBk
b3dubG9hZCksIHRoZSByZXNwb25zZSBoYXMKKyAgICAgICAgYWxyZWFkeSBiZWVuIHJlY2VpdmVk
LiBJbiB0aGlzIGNhc2Ugd2Ugc2hvdWxkIG1ha2Ugc3VyZSB0aGF0IHRoZQorICAgICAgICBkb3du
bG9hZCBjbGllbnQgaXMgbm90aWZpZWQgYWJvdXQgdGhlIHJlc3BvbnNlLCBzbyB0aGF0IHdoZW4g
ZGF0YQorICAgICAgICBhY3R1YWxseSBhcnJpdmVzIHRoZSBvdXRwdXQgc3RyZWFtIGhhcyBhbHJl
YWR5IGJlZW4gY3JlYXRlZC4KKworICAgICAgICAqIFdlYlByb2Nlc3MvRG93bmxvYWRzL3NvdXAv
RG93bmxvYWRTb3VwLmNwcDoKKyAgICAgICAgKFdlYktpdDo6RG93bmxvYWQ6OnN0YXJ0V2l0aEhh
bmRsZSk6CisKIDIwMTItMDUtMzAgIEx1aXogQWdvc3RpbmkgIDxsdWl6LmFnb3N0aW5pQG5va2lh
LmNvbT4KIAogICAgICAgICBbUXRdIGhhbmRsZWQgdG91Y2htb3ZlIGV2ZW50cyBzaG91bGQgbm90
IGNhbmNlbCB0YXAgZ2VzdHVyZSByZWNvZ25pdGlvbgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktp
dDIvV2ViUHJvY2Vzcy9Eb3dubG9hZHMvc291cC9Eb3dubG9hZFNvdXAuY3BwIGIvU291cmNlL1dl
YktpdDIvV2ViUHJvY2Vzcy9Eb3dubG9hZHMvc291cC9Eb3dubG9hZFNvdXAuY3BwCmluZGV4IDJk
OTZmZWUuLjVjOTZlYmEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvRG93
bmxvYWRzL3NvdXAvRG93bmxvYWRTb3VwLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9j
ZXNzL0Rvd25sb2Fkcy9zb3VwL0Rvd25sb2FkU291cC5jcHAKQEAgLTMwLDYgKzMwLDcgQEAKICNp
bmNsdWRlICJEYXRhUmVmZXJlbmNlLmgiCiAjaW5jbHVkZSA8V2ViQ29yZS9FcnJvcnNHdGsuaD4K
ICNpbmNsdWRlIDxXZWJDb3JlL05vdEltcGxlbWVudGVkLmg+CisjaW5jbHVkZSA8V2ViQ29yZS9S
ZXNvdXJjZUhhbmRsZUludGVybmFsLmg+CiAjaW5jbHVkZSA8Z2lvL2dpby5oPgogI2luY2x1ZGUg
PGdsaWIvZ2kxOG4tbGliLmg+CiAjaW5jbHVkZSA8d3RmL2dvYmplY3QvR093blB0ci5oPgpAQCAt
MTQ3LDYgKzE0OCwxMCBAQCB2b2lkIERvd25sb2FkOjpzdGFydFdpdGhIYW5kbGUoV2ViUGFnZSog
aW5pdGlhdGluZ1BhZ2UsIFJlc291cmNlSGFuZGxlKiByZXNvdXJjZQogICAgIHJlc291cmNlSGFu
ZGxlLT5zZXRDbGllbnQobV9kb3dubG9hZENsaWVudC5nZXQoKSk7CiAgICAgbV9yZXNvdXJjZUhh
bmRsZSA9IHJlc291cmNlSGFuZGxlOwogICAgIGRpZFN0YXJ0KCk7CisgICAgLy8gSWYgdGhlIGhh
bmRsZSBhbHJlYWR5IGdvdCBhIHJlc3BvbnNlLCBtYWtlIHN1cmUgdGhlIGRvd25sb2FkIGNsaWVu
dCBpcyBub3RpZmllZC4KKyAgICBSZXNvdXJjZUhhbmRsZUludGVybmFsKiBoYW5kbGVJbnRlcm5h
bCA9IG1fcmVzb3VyY2VIYW5kbGUtPmdldEludGVybmFsKCk7CisgICAgaWYgKCFoYW5kbGVJbnRl
cm5hbC0+bV9yZXNwb25zZS5pc051bGwoKSkKKyAgICAgICAgbV9kb3dubG9hZENsaWVudC0+ZGlk
UmVjZWl2ZVJlc3BvbnNlKG1fcmVzb3VyY2VIYW5kbGUuZ2V0KCksIGhhbmRsZUludGVybmFsLT5t
X3Jlc3BvbnNlKTsKIH0KIAogdm9pZCBEb3dubG9hZDo6Y2FuY2VsKCkK
</data>
<flag name="review"
          id="152073"
          type_id="1"
          status="+"
          setter="mrobinson"
    />
          </attachment>
      

    </bug>

</bugzilla>