[webkit-reviews] review denied: [Bug 39117] Finish up IndexedDB events : [Attachment 56074] Patch

Fri May 14 09:37:59 PDT 2010

Nate Chapin <japhet at chromium.org> has denied Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 39117: Finish up IndexedDB events
https://bugs.webkit.org/show_bug.cgi?id=39117

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

------- Additional Comments from Nate Chapin <japhet at chromium.org>
No Webkit/chromium/ChangeLog entry.

Rest of comments inline. On the whole, looks good.

> +	   Disable it all all !Chromium platforms since none of them compile it

Typo.

> diff --git a/LayoutTests/storage/indexeddb/script-tests/TEMPLATE.html
b/LayoutTests/storage/indexeddb/script-tests/TEMPLATE.html
> new file mode 100644
> index
0000000000000000000000000000000000000000..239a7940debd7857873574f0a09a35880735a
c19
> --- /dev/null
> +++ b/LayoutTests/storage/indexeddb/script-tests/TEMPLATE.html
> @@ -0,0 +1,12 @@
> +<html>
> +<head>
> +<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
> +<script src="../../fast/js/resources/js-test-pre.js"></script>
> +<script src="../../fast/js/resources/js-test-post-function.js"></script>
> +</head>
> +<body>
> +<p id="description"></p>
> +<div id="console"></div>
> +<script src="YOUR_JS_FILE_HERE"></script>
> +</body>
> +</html>

Do we need to propagate the TEMPLATE?

> +protected:
> +    // Do not directly instantiate.
> +    IDBEvent(const AtomicString& type, PassRefPtr<IDBAny> source);

Nit: this comment is redundant with the fact that the constructor is protected.

> -    PassRefPtr<IndexedDatabase> m_indexedDatabase;
> +    RefPtr<IndexedDatabase> m_indexedDatabase;
> +    RefPtr<IDBAny> m_this;

A variable name of m_this rubs me the wrong way.  m_source or m_request or
something?