<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Update WOFF2 decoder"
href="https://bugs.webkit.org/show_bug.cgi?id=162608">bug 162608</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #289962 Flags</td>
<td>review?
</td>
<td>review+
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Update WOFF2 decoder"
href="https://bugs.webkit.org/show_bug.cgi?id=162608#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Update WOFF2 decoder"
href="https://bugs.webkit.org/show_bug.cgi?id=162608">bug 162608</a>
from <span class="vcard"><a class="email" href="mailto:mcatanzaro@igalia.com" title="Michael Catanzaro <mcatanzaro@igalia.com>"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=289962&action=diff" name="attach_289962" title="Patch, v3">attachment 289962</a> <a href="attachment.cgi?id=289962&action=edit" title="Patch, v3">[details]</a></span>
Patch, v3
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=289962&action=review">https://bugs.webkit.org/attachment.cgi?id=289962&action=review</a>
r=me on the condition that Frederic says it's good.
<span class="quote">> Source/WebCore/platform/graphics/WOFFFileFormat.cpp:100
> + bool Write(const void *data, size_t n)</span >
void* data, not void *data
Is this a virtual method? If so you need to mark it override. If not, it should be named write with a lowercase W, since WebKit methods are camelCase.
<span class="quote">> Source/WebCore/platform/graphics/WOFFFileFormat.cpp:108
> + bool Write(const void *data, size_t offset, size_t n)</span >
Ditto
<span class="quote">> Source/WebCore/platform/graphics/WOFFFileFormat.cpp:125
> + Vector<char>& m_vector;</span >
Optional: I think it would be safer to keep a plain Vector<char> here instead of a reference, and sink it to WOFF2VectorOut by changing the constructor to take a Vector<char>&& instead of a Vector<char>. Then you will need to use WTFMove to pass the vector to the constructor. The advantage of doing it that way is the WOFF2VectorOut becomes harder to misuse (more robust to future modification); currently it breaks badly if the passed in vector goes out of scope before the WOFF2VectorOut does. But this is optional; the current way is OK too.</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>