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

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Marcus Bulach <bulach at chromium.org>  2010-08-03 03:10:07 PST ---
Thanks, Andrei!
replies inline:

(In reply to comment #5)
> > WebCore/bindings/v8/SerializedScriptValue.cpp
> > String keyPathExtractor(const String& wireData, const String& keyPathString)
> 
> Does the above IDB-specific method belong to the SerializedScriptValue file? I think Jeremy added a IDBBindingsUtils class that seems more fit for your purpose.
moved to IDBBindingUtils.

> 
> Also, is this method used just for testing? Otherwise, I am a bit confused about why it always returns a string. 
this is the real method, returning string was a mistake :)
it now returns an IDBKey.

> 
> And one last thing: is the current name the best choice? You're not extracting a key path. You're extracting a value from the serialized object at the given key path.
as discussed, this is now called valueForKeyPath.

> 
> > void IDBKeyPath::parse()
> > (...)
> >  case Array : {
> > (...)
> > m_lexer.next();
>  > token = m_lexer.currentToken();
> 
> You are only checking for dot or left bracket. But what if tokEnd follows? Seems like you're returning ParserErrorAfterArray in that case? Although looking at the unit tests, it seems you have cases for key paths that end indexing into an array.

sorry, I think it wasn't clear. this bug depends on https://bugs.webkit.org/show_bug.cgi?id=42976, and in fact contains all the code from there.
as such, please avoid looking at any IDBKeyPath.* here. once that patch is in, I'll merge it here and this will only contain the binding utilities and related tests.
regardless: this comment has been applied in both places, thanks!

> 
> > class LocalContext {
> 
> Umm, do you need this? Isn't this the same as v8::Context::Scope?

apparently, it also needs a v8::HandleScope, so I put them both inside this LocalContext (and removed the HandleScope from individual tests).

another look please?

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