[Webkit-unassigned] [Bug 43276] Implements IDBKeyPath extractor.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43276


Jeremy Orlow <jorlow at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64017|                            |review-
               Flag|                            |




--- Comment #17 from Jeremy Orlow <jorlow at chromium.org>  2010-08-10 10:13:42 PST ---
(From update of attachment 64017)
WebCore/bindings/v8/IDBBindingUtilities.cpp:51
 +  bool GetValueFrom(v8::Handle<v8::Value>& v8Value, T indexOrName)
lowercase g

WebCore/bindings/v8/IDBBindingUtilities.cpp:55
 +          return 0;
return false

WebCore/bindings/v8/IDBBindingUtilities.cpp:51
 +  bool GetValueFrom(v8::Handle<v8::Value>& v8Value, T indexOrName)
Hm...should the output param come second?

WebCore/bindings/v8/IDBBindingUtilities.cpp:68
 +          } else {
else if maybe...and maybe even add an else after this that ASSERT_NOT_REACHED?

Maybe use a switch?

WebCore/bindings/v8/IDBBindingUtilities.h:32
 +  #include <wtf/Vector.h>
alpha order

WebCore/bindings/v8/IDBBindingUtilities.h:38
 +  struct IDBKeyPathElement;
structs come after classes

WebKit/chromium/WebKit.gyp:180
 +                  'public/WebIDBBindingUtilities.h',
So this file is alpha, not ascii order?

WebKit/chromium/public/WebIDBBindingUtilities.h:35
 +  WebIDBKey WebValueForKeyPath(const WebSerializedScriptValue&, const WebIDBKeyPath&);
lower case w

WebKit/chromium/public/WebIDBKeyPath.h:33
 +  namespace WebCore {
these could be one liners if you wanted

WebKit/chromium/src/WebIDBBindingUtilities.cpp:40
 +  WebIDBKey WebValueForKeyPath(const WebSerializedScriptValue& serializedScriptValue, const WebIDBKeyPath& idbKeyPath)
I still kind of feel like this should just be a static method on WebIDBKeyPath or something.

WebKit/chromium/src/WebIDBKeyPath.cpp:44
 +      IDBParseKeyPath(keyPath, &idbElements, &idbError);
Hm....isn't the norm to just pass by reference, not pointer?

WebKit/chromium/src/WebIDBKeyPath.cpp:47
 +        return NULL;
0

and 4 spaces

WebKit/chromium/src/WebIDBKeyPath.cpp:40
 +  WebIDBKeyPath* WebIDBKeyPath::create(const WebString& keyPath, int* error)
Why do we need to return a *?  Just return a copy of this object.

WebKit/chromium/src/WebIDBKeyPath.cpp:58
 +      m_private.reset();
The destructor needn't call reset anymore now that you have a class to do it...so you could get rid of reset.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.