On Wed, Apr 15, 2009 at 5:25 PM, Michael Nordman <micha...@chromium.org>wrote:
> > + 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. > I actually tried to explain this in the "second cut" section of the optimizations section. Obviously I didn't do a very good job, so I made the text more clear (and fixed one typo). > > > 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 -~----------~----~----~----~------~----~------~--~---
