[webkit-reviews] review granted: [Bug 45410] Decouple Blobs from ScriptExecutionContext. : [Attachment 66966] decouple

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 8 17:31:13 PDT 2010


Jian Li <jianli at chromium.org> has granted Michael Nordman
<michaeln at google.com>'s request for review:
Bug 45410: Decouple Blobs from ScriptExecutionContext.
https://bugs.webkit.org/show_bug.cgi?id=45410

Attachment 66966: decouple
https://bugs.webkit.org/attachment.cgi?id=66966&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
Looks good except some minor issues.

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

> WebCore/bindings/js/SerializedScriptValue.cpp:977
> +	   file = File::create(String(path.ustring().impl()), KURL(KURL(),
String(url.ustring().impl())), String(type.ustring().impl()));
Do we need to keep m_isDOMGlobalObject check here? The blob reading part has
this logic kept.
    if (!m_isDOMGlobalObject)
	return jsNull();

> WebCore/fileapi/FileReader.cpp:164
> +void FileReader::clearLoaderAndReadableURL()
Probably it is better to call it cleanup since we might put other cleanup logic
in this function in the future.

> WebCore/fileapi/FileReader.h:145
> +    KURL m_readableURL;
m_readableURL sounds like to mean the URL is legible. Might be better to name
it something like m_urlForReading. If you make this change, do not forget to
update ChangeLog.

> WebCore/fileapi/FileReaderSync.cpp:154
> +    // The blob is read by routing through the request handling layer given
the tempory public url.
tempory => temporary


More information about the webkit-reviews mailing list