+ chromium-dev <chromium-dev@googlegroups.com> On Tue, Apr 14, 2009 at 9:36 AM, Darin Fisher <da...@chromium.org> wrote: > DOM Storage Overview > LGTM > > LocalStorage > "The exception is when a synchronous operation (synch XHR or alert() are > examples) or when navigator.unlockStorage() is executed" <-- I think you > should also say that the lock is released when calling into a plugin. I > believe that issue was also discussed on the whatwg thread. > "For localStorage, the BrowserProcess::db_thread will be unless a new thread > is absolutely necessary." <-- I agree that we should not need a new thread. > "This thread will receive and process all localStorage IPC messages." <-- I > assume you will intercept the IPC messages on the IO thread, and delegate to > the DB thread. This means that the IO thread is also involved in receiving > and processing localStorage IPC messages. > "Luckily, each origin is completely isolated from the next, so we > essentially must implement a per-origin lock for localStorage." <-- In > discussing this with Ian Hickson, he was convinced that we would still need > a global lock to avoid dead-locks. I didn't fully understand the dead-lock > scenario he was concerned about, so it might be good to ask him about it. > He said you could imagine a frame from origin A containing a frame from > origin B. There may also be a frame from origin B containing a frame from > origin A in another process. If there was a way for script in the outer > frame to synchronously block on execution of script in the inner frame, then > I guess you could get a dead-lock. I'm not sure how to construct that > scenario though. I'm probably overlooking something... > "How do we know when a process dies?" <-- See > BrowserRenderProcessHost::OnChannelError. > "Because responses may be deferred for some time, the message handler will > probably be IPC_MESSAGE_HANDLER_DELAY_REPLY. " <-- this is also necessary in > order to delegate IPC message processing to the DB thread. in other words, > you don't want to block the IO thread waiting for the DB thread. > "When a render process asks to subscribe to storage events, that process > will be added to the set for a particular origin." <-- it seems like this > registration will happen asynchronously in chrome. there could be some > interesting race conditions that cause you to either send a notification > when one was not expected or miss a notification that would be sent by > safari. what are your plans for this? maybe the races don't matter since > it is hard to predict (even in a single threaded browser like safari) when > another window will modify localStorage. > It seems unfortunate to me that the spec requires a storage event to carry a > copy of the old and new values. Oh well :( > "The render process will then need to run a nested message loop to process > messages. TODO: How will this work? We can probably do whatever set/get > cookie does." <-- Yes, just use the existing synchronous IPC messaging code > that we use for cookies and many other things. This should be easy. > "Other blocking operations like synchronous XHR operations and calling > alert() should release the lock as well." <-- you should probably try to > recognize these cases in chrome/renderer/ so that you can have a single IPC > for unlocking. > StorageEvent: "These messages are synchronous and thus require a nested > message loop." <-- why do they need to be synchronous? OK, I see that you > are addressing the race issue I mentioned before. I think we can probably > stick with asynchronous messages but keep a corresponding data structure in > the renderer that allows us to drop notifications that we should not > dispatch. i don't think we benefit from using synchronous IPC here. > "When you fire an event, will other windows see it? If not, no need for the > rest of this... When the event is dispatched, a synchronous message will be > sent to the browser process which will be redistributed to all render > processes currently subscribed. A nested message loop will be run during > the synchronous action. (Also synchronous for serialization purposes.)" <-- > I don't understand this. Since storage events originate in the browser, why > would the renderer need to send them back to the browser to be > re-distributed? > "Cleanup of WebKit's *Storage code" <-- It would be good to start the > conversation early with folks from WebKit about how significantly different > our implementation of localStorage is. You should expect push-back unless > you can adequately justify why a separate implementation is required. I > think your design is the right approach, so this should be an "easy" > argument to make ;-) > Please ask Brett Wilson to review your DB schema and operations. > "Rejected alternative approaches" <-- i'm surprised that you don't explain > why you are not using the current webkit implementation perhaps with the > same VFS approach that we are using for HTML5 database. the only thing > missing there is a solution for storage events. > the costs to having a separate implementation from apple is non-trivial. > ideally you're solution here would be a refactoring of their code to enable > you to share some of their code. for example, i would expect that you could > share the code that interacts with sqlite. > SessionStorage > I like the idea of sharing code with LocalStorage here. I think we cannot > easily reuse the WebKit code because we need SessionStorage to be > transferred between renderer processes: a single tab could use multiple > renderer processes as navigations to new sites issued from the omnibox > result in a process switch. > If you use the same code as LocalStorage, you would just need to use an > in-memory sqlite db, and have a different way of grouping the databases. > The big downside to this approach in my mind is performance. Now you have > to go all the way to the browser process to access sessionStorage. Safari's > implementation will always be faster :(
We may be able to address the performance difference with some caching in the renderer. The 'truth' is stored in the browser process, but an local copy of the session storage area is lazily populated on reads and writes. On writes, store the value locally and signal events as needed... and send an async ipc to have that stored in the browser process too. On reads, check the local copy first, if present return it, otherwise go to the browser process, cache the retreived value locally, and return it. The best of both worlds. > If we try to keep sessionStorage in webkit, then we need to have a way to > copy it out. That may actually be required if we want to support persisting > sessionStorage for the purposes of session restore. Hmm... > -darin > > On Tue, Apr 7, 2009 at 9:12 PM, Jeremy Orlow <jor...@chromium.org> wrote: >> >> After several chromium-dev and whatwg email threads, I think the major >> issues with the spec and how to implement localStorage have been hammered >> out. I've written up my current thoughts in terms of a couple documents >> (listed below). >> This is my first major chunk of code in Chromium, so I've tried to be as >> precise as possible. There are a couple of sections highlighted in yellow >> that are things I'm not sure about (and thus need to research more). If I >> say anything in the doc that seems suspect, please point it out! >> I've made these writable for anyone with a chromium.org account, so feel >> free to make comments inline using the 'Insert > Comment' menu. I'll either >> incorporate the feedback into the doc or reply in this thread. >> http://docs.google.com/Doc?id=dhs4g97m_0hjv3nqc7 - DOM Storage Overview >> http://docs.google.com/Doc?id=dhs4g97m_1v79p3wdd - LocalStorage >> http://docs.google.com/Doc?id=dhs4g97m_2c2bvftf6 - SessionStorage >> Thanks a lot! >> Jeremy >> > > > > > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
