<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [SOUP] Add initial implementation of NetworkProcess disk cache"
   href="https://bugs.webkit.org/show_bug.cgi?id=143872#c8">Comment # 8</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [SOUP] Add initial implementation of NetworkProcess disk cache"
   href="https://bugs.webkit.org/show_bug.cgi?id=143872">bug 143872</a>
              from <span class="vcard"><a class="email" href="mailto:svillar&#64;igalia.com" title="Sergio Villar Senin &lt;svillar&#64;igalia.com&gt;"> <span class="fn">Sergio Villar Senin</span></a>
</span></b>
        <pre>Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=251014&amp;action=diff" name="attach_251014" title="Patch">attachment 251014</a> <a href="attachment.cgi?id=251014&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=251014&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=251014&amp;action=review</a>

Awesome stuff Carlos!

<span class="quote">&gt; Source/WebKit2/NetworkProcess/cache/NetworkCacheBlobStorage.cpp:34
&gt; +#include &lt;fcntl.h&gt;</span >

Why this?

<span class="quote">&gt; Source/WebKit2/NetworkProcess/cache/NetworkCacheDataSoup.cpp:29
&gt; +#if ENABLE(NETWORK_CACHE)</span >

Don't we need some USE(SOUP) or PLATFORM(LINUX) and the like?

<span class="quote">&gt; Source/WebKit2/NetworkProcess/cache/NetworkCacheDataSoup.cpp:129
&gt; +    ASSERT(map &amp;&amp; map != MAP_FAILED);</span >

Perhaps better to split this in 2 ASSERTs in order to better spot the actual cause of failure.

<span class="quote">&gt; Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:58
&gt; +    ASSERT(m_outputStream || m_inputStream || m_ioStream);</span >

Same here about splitting the ASSERT

<span class="quote">&gt; Source/WebKit2/NetworkProcess/cache/NetworkCacheIOChannelSoup.cpp:82
&gt; +    }</span >

Can't we use Data directly instead of having to copy it from a SoupBuffer?</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>