[Webkit-unassigned] [Bug 26189] DOM Storage: Changes for Android and macro guards

[bugzilla-daemon at webkit.org]

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





------- Comment #5 from ddkilzer at webkit.org  2009-06-05 08:48 PDT -------
(In reply to comment #4)
> > Should this have a #if PLATFORM(ANDROID)/#endif around it instead of the long
> > comment?  If anyone is curious as to why the method is there, the could look
> > for an explanation in this bug.
> > 
> > I'm split between adding more #if/#endif macros and compiling unused code on
> > other platforms.  (I think replacing the long comment with the #if/#endif
> > macros is a win, though.)
> 
> I was hesitant about this too. I'm happy to add the platform guards if you'd
> like me to, but on the other hand the code doesn't use any Android specific
> APIs and works on all platforms. It's just only used by Android right now,
> perhaps the functionality would be useful for others in the future? And the
> same for the new function in LocalStorage. It's only used on Android at the
> moment, but it's platform independent and could be useful for others now or
> later. I can replace the comment with a link to this bug either way.

Yes, a comment above PageGroup::updateLocalStoragePath() referring to the bug
would probably be best.

The LocalStorage::updatePath() method was extracted from the constructor,
though, so I'm not sure it needs a comment since the call to close() would be
required if anyone was to call updatePath() outside of the constructor.

--

You know, after looking at these methods, they seem like a bit of a hack. 
Android is only going to update the path once, correct?  Or could it update the
database path multiple times while the browser is running?

Would it be possible instead to provide a callback mechanism when opening the
LocalStorage databases so that Android could give the correct path "just in
time"?

A callback function pointer could be passed/set somewhere, and it would only be
used if not-null.  This might also mean that LocalStorage databases should only
be opened lazily, but that may not be such a bad thing.

Again, someone more familiar with this code should review the patch.


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