Fix OwnPtr issues in IndexedDB
Created attachment 91378 [details] Patch
Comment on attachment 91378 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91378&action=review > Source/WebCore/platform/leveldb/LevelDBDatabase.cpp:102 > + OwnPtr<LevelDBDatabase> result = adoptPtr(new LevelDBDatabase()); Not required (cq+ is ok with me) but might want to remove the ().
Comment on attachment 91378 [details] Patch Yay! I'm kinda surprised it got landed as it was.
In fareness to Steve, I think Darin's document is due for a refresh: http://www.webkit.org/coding/RefPtr.html. Some of what seemed "obviously" wrong in the previous commit, it not actually that obvious of patterns of WebCore. But we do need to be careful to get our memory management (and header includes) correct.
Committed r85122: <http://trac.webkit.org/changeset/85122>
http://trac.webkit.org/changeset/85122 might have broken SnowLeopard Intel Release (Tests)
This busticated the clang build: http://build.chromium.org/p/chromium.webkit/builders/Mac%20Clang%20Builder%20%28dbg%29/builds/6490/steps/compile/logs/stdio In file included from /b/build/slave/Mac_Clang_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/leveldb/LevelDBDatabase.cpp:27: In file included from ../platform/leveldb/LevelDBDatabase.h:31: ../../JavaScriptCore/wtf/OwnPtr.h:57:9: error: function 'WTF::OwnPtr<WebCore::<anonymous>::ComparatorAdapter>::OwnPtr' has internal linkage but is not defined [-Werror] OwnPtr(const OwnPtr<ValueType>&); ^ /b/build/slave/Mac_Clang_Builder__dbg_/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../platform/leveldb/LevelDBDatabase.cpp:100:31: note: used here OwnPtr<ComparatorAdapter> comparatorAdapter = adoptPtr(new ComparatorAdapter(comparator)); ^ 1 error generated.
> This busticated the clang build: I have no idea what that error means. That line of code is one of the most common patterns in WebKit. Maybe it's a compiler bug?
Maciej has a theory for how to fix this error.
Created attachment 91436 [details] Patch
Comment on attachment 91436 [details] Patch r=me
> r=me Thanks!
Committed r85169: <http://trac.webkit.org/changeset/85169>
Thanks for fixing this and apologies for missing the include problems in the original patch. I wasn't aware that we should always use (Pass)OwnPtr every time ownership of an object is being transferred. What exactly is the policy? It would be great if the document could be updated for future reference.
> I wasn't aware that we should always use (Pass)OwnPtr every time ownership of an object is being transferred. What exactly is the policy? It would be great if the document could be updated for future reference. The policy is that we should try to avoid ever having a "naked new," which means every call to new should be immediately followed by either adoptRef (for ref-counted objects) or adoptPtr (for objects with a single owner). Everything else follows from the C++ type system. Turning on strict mode tightens the type system to better enforce this style. Once we've got that all working, we'll probably add a check-webkit-style nag about "naked new" so that these problems can be flagged automagically. (There's already an ASSERT that enforces this behavior for ref-counted objects.)