*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. -darin On Tue, Apr 7, 2009 at 9:12 PM, Jeremy Orlow <[email protected]> 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: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
