[webkit-reviews] review denied: [Bug 59234] CrossThreadCopier should not have a default specialization for raw pointers : [Attachment 91195] Style feedback

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Dmitry Lomov <dslomov at google.com>'s
request for review:
Bug 59234: CrossThreadCopier should not have a default specialization for raw
pointers
https://bugs.webkit.org/show_bug.cgi?id=59234

Attachment 91195: Style feedback
https://bugs.webkit.org/attachment.cgi?id=91195&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=91195&action=review

Some initial comments. I need to look a little more at this though.

> Source/WebCore/platform/CrossThreadCopier.h:128
> +	   static Type copy(const AllowCrossThreadAccessWrapper<T>& r) { return
r.value(); }

Avoid single letter variables in WebKit (other places too).

> Source/WebCore/platform/CrossThreadCopier.h:134
> +    };

No ; needed here.

> Source/WebCore/platform/CrossThreadCopier.h:136
> +    template<typename T> struct AllowExtendedLifetimeWrapper {

AllowAccessLater? (feels like it mirrors the name AllowCrossThreadAccessWrapper
better).

> Source/WebCore/platform/CrossThreadCopier.h:144
> +    template<typename T> struct CrossThreadCopierBase<false, false,
AllowExtendedLifetimeWrapper<T> > {

Should add a FIXME about moving this out of this header file.

> Source/WebCore/platform/CrossThreadCopier.h:152
> +    };

No ; needed here.

> Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp:91
> +

Best not to do misc whitespace changes.

> Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp:457
> +void IDBObjectStoreBackendImpl::openCursor(PassRefPtr<IDBKeyRange> prpRange,
unsigned short direction, PassRefPtr<IDBCallbacks> prpCallbacks,
IDBTransactionBackendInterface* transaction, ExceptionCode& ec)

I like the old naming better because IDBTransactionBackendInterface* is a
pointer but RefPtr<IDBTransactionBackendInterface> is not.

> Source/WebCore/websockets/WorkerThreadableWebSocketChannel.cpp:251
> +			     AllowCrossThreadAccess(thisPtr), 

indenting is off.