[Webkit-unassigned] [Bug 25376] Refactor localStorage code for use in multi-process browsers

[bugzilla-daemon at webkit.org]

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





------- Comment #5 from jorlow at chromium.org  2009-06-04 20:47 PDT -------
(In reply to comment #4)
> I'm really not at all familiar with the localstore code, so I'm not in a great
> position to give you a real review. (Not that I have review rights anyway:)
> 
> 
> 28         count bytes as you read the local storage databases into memory
> 
> This implies to me the entire db is slurped into memory. Is that right?

Yes.  This is how it's always been.

> 
> 65     : m_path(path.copy())
> 
> nit: is the call to .copy() really necessary


This is how the original code was, but I can take a look to see if it's OK to
skip the .copy().

> 68     if (!path.isEmpty())
> 69         m_syncManager = StorageSyncManager::create(m_path);
> 
> nit: use the same (m_)path variable in the test and create call

Oops.  Quite correct.

> 115     ASSERT(origin);
> 116     if (!origin)
> 117         return String();
> 
> nit: which is it, required or not? One of them can probably go.

ASSERT will tell you there's a problem in debug mode, but WebKit will crash a
few lines later without the if statement.  I don't know if there are actual
cases where origin could be null or if this code was "just in case".  Something
I would like to do in the future is investigate this stuff, but I don't think
it's a priority.

> 124     return pathByAppendingComponent(m_path, origin->databaseIdentifier() +
> ".localstorage");
> 
> Oh, a unique database file per origin. I see this was pre-existing code.
> Do you know why there is one per origin instead of one for all? Just curious.

Because there's a database per origin.

> WebCore/storage/StorageSyncManager.cpp
> line 80: extra whitespace here

Noted.

These really aren't enough changes for me to roll another patch as is, but I'll
do it before this gets submitted.  Thanks for reviewing, Michael.  


-- 
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.