Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10850010:
Make session restore understand that tabs have multiple SessionStorageNamespaces. (Closed)
|
Created:
8 years, 4 months ago by awong Modified:
7 years, 5 months ago CC:
chromium-reviews, marja+watch_chromium.org, nasko Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionMake session restore understand that tabs have multiple SessionStorageNamespaces.
Also teaches session restore to just skip malfomed session storage
association commands. It seems overkill to quit the whole restore
just because of one bad namespace.
In session restore, we associated write out the full association of (tab, session storage persistent id, partition_id). On restore, before reconstituting the session storage, we parse the partition_id and check that it still exists skipping the serialized session storage if it has been destroyed during session restore.
BUG=85121
Patch Set 1 : Fix link error. #Patch Set 2 : Add U-pipe through all the layers to put IsValidStorageParitionId() near where partition ids are cr… #Patch Set 3 : Unittest using jam's new PRE_ method #
Total comments: 13
Patch Set 4 : Rebased, with test robustified. #
Total comments: 1
Patch Set 5 : rebased #Patch Set 6 : merged with ToT #Patch Set 7 : attempt to fix unittsets. #Messages
Total messages: 23 (0 generated)
Scott, could you look at the session restore changes? Charlie, could you look at the new IsValidStoragePartitonZOMGTOOMANYLAYERS() functions? Thanks, Albert
I sent you some comments in your other review. Is this ready for review now? -Scott On Thu, Aug 2, 2012 at 6:29 PM, <ajwong@chromium.org> wrote: > Reviewers: sky, creis, > > Message: > Scott, could you look at the session restore changes? > > Charlie, could you look at the new IsValidStoragePartitonZOMGTOOMANYLAYERS() > functions? > > Thanks, > Albert > > Description: > Make session restore understand that tabs have multiple > SessionStorageNamespaces. > > Also teaches session restore to just skip malfomed session storage > association commands. It seems overkill to quit the whole restore > just because of one bad namespace. > > In session restore, we associated write out the full association of (tab, > session storage persistent id, partition_id). On restore, before > reconstituting > the session storage, we parse the partition_id and check that it still > exists > skipping the serialized session storage if it has been destroyed during > session > restore. > > BUG=85121 > > Please review this at https://chromiumcodereview.appspot.com/10850010/ > > SVN Base: svn://svn.chromium.org/chrome/trunk/src > > Affected files: > M chrome/browser/chrome_content_browser_client.h > M chrome/browser/chrome_content_browser_client.cc > M chrome/browser/sessions/session_restore.cc > M chrome/browser/sessions/session_restore_browsertest.cc > M chrome/browser/sessions/session_service.h > M chrome/browser/sessions/session_service.cc > M chrome/browser/sessions/session_types.h > M chrome/browser/sessions/tab_restore_service.h > M chrome/browser/sessions/tab_restore_service.cc > M chrome/browser/sessions/tab_restore_service_delegate.h > M chrome/browser/ui/browser_tab_restore_service_delegate.h > M chrome/browser/ui/browser_tab_restore_service_delegate.cc > M chrome/browser/ui/browser_tabrestore.h > M chrome/browser/ui/browser_tabrestore.cc > M content/browser/browser_context.cc > M content/public/browser/browser_context.h > M content/public/browser/content_browser_client.h > M content/public/browser/content_browser_client.cc > >
Hi Scott, Yes, I think this is ready for review now. One major problem though...I don't know how to trigger a session restore in the extensions browsertest framework after the browser restart. Thus, the new unittest in isolated_apps_browsertest.cc currently hangs. Any thoughts on this? -Albert
https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... File chrome/browser/sessions/session_service.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_service.cc:1287: continue; // If the storage is corrupt, just ignore it. When an user first launches the version of chromium which has this code, it's unable to read and utilize the old sessionStorageAssociated commands, because they're missing the partition IDs. So, the sessionStorages end up getting lost in that relaunch. Can you / should you handle this gracefully (by associating the session storages without a partition id into the default partition, or something like that)? https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... File content/test/test_launcher.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... content/test/test_launcher.cc:302: int RunTestInternal(const testing::TestCase* test_case, Could the test machinery enhancements go to a separate CL?
On 2012/08/04 01:32:11, awong wrote: > Hi Scott, > > Yes, I think this is ready for review now. One major problem though...I don't > know how to trigger a session restore in the extensions browsertest framework > after the browser restart. Thus, the new unittest in > isolated_apps_browsertest.cc currently hangs. Any thoughts on this? Not sure what you mean. Can you separate the test into more than one test like you've done with _PRE ? If you specify the command line kRestoreLastSession chrome should restore the last session on startup. Alternatively if session restore is enabled and you close the last browser window then create a new window chrome should trigger session restore. -Scott
I only looked at the chrome side of things, which LGTM https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/ui/b... File chrome/browser/ui/browser_tab_restore_service_delegate.h (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/ui/b... chrome/browser/ui/browser_tab_restore_service_delegate.h:42: OVERRIDE; Don't leave OVERRIDE dangling like this (just like you wouldn't leave const dangling).
https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... File chrome/browser/sessions/session_service.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_service.cc:1287: continue; // If the storage is corrupt, just ignore it. On 2012/08/06 14:38:34, marja wrote: > When an user first launches the version of chromium which has this code, it's > unable to read and utilize the old sessionStorageAssociated commands, because > they're missing the partition IDs. So, the sessionStorages end up getting lost > in that relaunch. > > Can you / should you handle this gracefully (by associating the session storages > without a partition id into the default partition, or something like that)? Actually, depending on when this CL lands it might or might not matter. Atm it doesn't matter, since the sessionStorage data is not yet saved. So maybe it's simpler to just leave it like this. (Even if the sessionStorages get lost for one restart, the impact is really small, taken into account that they're now lost on every restart :)
The current IsValidStoragePartitonId LGTM, and I agree with moving it to StoragePartition later. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/pref... File chrome/browser/prefs/pref_service_browsertest.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/pref... chrome/browser/prefs/pref_service_browsertest.cc:26: typedef InProcessBrowserTest PreservedWindowPlacement; This is just from https://chromiumcodereview.appspot.com/10832106, right? (Not part of your changes?) https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... File chrome/browser/sessions/session_restore_browsertest.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_restore_browsertest.cc:777: controller.GetSessionStorageNamespaceMap(); There will only be one in here, right? I guess that's ok, since we test the multiple namespace case in isolated_apps_browsertest.cc. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_restore_browsertest.cc:802: << " for storage partition --" << it->first << "--"; Why the dashes? https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... File content/test/test_launcher.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... content/test/test_launcher.cc:302: int RunTestInternal(const testing::TestCase* test_case, On 2012/08/06 14:38:34, marja wrote: > Could the test machinery enhancements go to a separate CL? Looks like it's already committed in https://chromiumcodereview.appspot.com/10832106. It should disappear after a rebase.
Comments addressed. Disabled the actual deep check for session restore for now since Marja's change needs to go in first in order for there to be any data in the associated SSNs. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... File chrome/browser/sessions/session_restore_browsertest.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_restore_browsertest.cc:777: controller.GetSessionStorageNamespaceMap(); On 2012/08/06 23:28:11, creis wrote: > There will only be one in here, right? I guess that's ok, since we test the > multiple namespace case in isolated_apps_browsertest.cc. Yes...I elected to do the integration-style test coverage in the isolated_apps_browsertest because otherwise I'd have to pull in extension loading code into this test. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_restore_browsertest.cc:802: << " for storage partition --" << it->first << "--"; On 2012/08/06 23:28:11, creis wrote: > Why the dashes? Cause the partition id is often the empty string. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... File chrome/browser/sessions/session_service.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/sess... chrome/browser/sessions/session_service.cc:1287: continue; // If the storage is corrupt, just ignore it. On 2012/08/06 14:53:21, marja wrote: > On 2012/08/06 14:38:34, marja wrote: > > When an user first launches the version of chromium which has this code, it's > > unable to read and utilize the old sessionStorageAssociated commands, because > > they're missing the partition IDs. So, the sessionStorages end up getting lost > > in that relaunch. > > > > Can you / should you handle this gracefully (by associating the session > storages > > without a partition id into the default partition, or something like that)? > > Actually, depending on when this CL lands it might or might not matter. Atm it > doesn't matter, since the sessionStorage data is not yet saved. So maybe it's > simpler to just leave it like this. (Even if the sessionStorages get lost for > one restart, the impact is really small, taken into account that they're now > lost on every restart :) I think we're safe for now. Like you said, we'd lose the SSNs for just one restart. As to your other question about whether or not we could/should do something about associating to the default partition, that would actually be unsafe. Over a restart, someone may go in and mess with which URLs are isolated (imagine a policy push). Thus, it's not clear that the set of associated partition IDs that were written are the same that should be in the tab on restore. To handle the case where partitions become invalid, before we try to construct the real TabContents, we check if the partition_id is valid. If not, then we just elect not to insert the associated SSN into the tab contents. They will then auto-magically pull up the default SSN via the NavControllerImpl::GetSessionStorageNamespace() method. For the case where some NavEntries inside the tab have become "partitioned" we actually don't need to do anything and they will mint themselves a fresh new SSN on first access, again using NavControllerImpl::GetSessionStorageNamespace(). I think this all works. The only thing we might want to do is when we notice a partition is invalid is somehow mark that the persisted SSN can be cleaned up. https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/ui/b... File chrome/browser/ui/browser_tab_restore_service_delegate.h (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/chrome/browser/ui/b... chrome/browser/ui/browser_tab_restore_service_delegate.h:42: OVERRIDE; On 2012/08/06 14:44:08, sky wrote: > Don't leave OVERRIDE dangling like this (just like you wouldn't leave const > dangling). Done. https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... File content/test/test_launcher.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6004/content/test/test_l... content/test/test_launcher.cc:302: int RunTestInternal(const testing::TestCase* test_case, On 2012/08/06 14:38:34, marja wrote: > Could the test machinery enhancements go to a separate CL? Sorry...yes, this is actually in another CL that has since been committed. Clean diff uploaded.
https://chromiumcodereview.appspot.com/10850010/diff/6006/chrome/browser/sess... File chrome/browser/sessions/session_service.cc (right): https://chromiumcodereview.appspot.com/10850010/diff/6006/chrome/browser/sess... chrome/browser/sessions/session_service.cc:609: tab->web_contents()->GetController().GetSessionStorageNamespaceMap(); Looks like namespaces for isolated-apps can be added to this collection sometime after this notification happens, is that right? How do those newly added relationships make it into session restore journal after TAB_PARENTED has happened?
On 2012/08/08 22:29:30, michaeln wrote: > https://chromiumcodereview.appspot.com/10850010/diff/6006/chrome/browser/sess... > File chrome/browser/sessions/session_service.cc (right): > > https://chromiumcodereview.appspot.com/10850010/diff/6006/chrome/browser/sess... > chrome/browser/sessions/session_service.cc:609: > tab->web_contents()->GetController().GetSessionStorageNamespaceMap(); > Looks like namespaces for isolated-apps can be added to this collection sometime > after this notification happens, is that right? How do those newly added > relationships make it into session restore journal after TAB_PARENTED has > happened? I think I finally understand your comment Michael. If you patch in my latest CL, you'll see my log message only ever shows that we have 1 SSN in the tab. This is probably due to exactly what you said -- the new SSNs are never written to the journal. What's the right solution here? Should I add a new notification for when a session storage namespace is created? That seems heavy weight...
> I think I finally understand your comment Michael. If you patch in my latest > CL, you'll see my log message only ever shows that we have 1 SSN in the tab. > This is probably due to exactly what you said -- the new SSNs are never written > to the journal. Yup, not what you wanted probably. > What's the right solution here? Should I add a new notification for when a > session storage namespace is created? That seems heavy weight... I don't know much about the session save/restore system, but *something* is being logged per tab navigation so the url to load into that tab is known at restore time, and the history of urls in that tab is also known at restore time to support navigating back (and forward) after having restored. The new navigation entries are added incrementally (well i'm guessing, like i said i don't know much about that stuff). Maybe we could also incrementally add associations to isolated SSNs coincident with new navigation entries. Maybe each navigation entry should have a strong association with a particular 'storage partition'?
Reviving this CL. Marja, Sky, I'm going to need some guidance here. Initially I thought this would be a simple extension of the current SessionStorageAssociate command. However, as Michael pointed out, doesn't work because new SSNs are created depending on what navigation entries are committed. NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all the associations. Do either of you know what the right notification is? -Albert On 2012/08/27 18:36:41, michaeln wrote: > > I think I finally understand your comment Michael. If you patch in my latest > > CL, you'll see my log message only ever shows that we have 1 SSN in the tab. > > This is probably due to exactly what you said -- the new SSNs are never > written > > to the journal. > > Yup, not what you wanted probably. > > > What's the right solution here? Should I add a new notification for when a > > session storage namespace is created? That seems heavy weight... > > I don't know much about the session save/restore system, but *something* is > being logged per tab navigation so the url to load into that tab is known at > restore time, and the history of urls in that tab is also known at restore time > to support navigating back (and forward) after having restored. The new > navigation entries are added incrementally (well i'm guessing, like i said i > don't know much about that stuff). Maybe we could also incrementally add > associations to isolated SSNs coincident with new navigation entries. > > Maybe each navigation entry should have a strong association with a particular > 'storage partition'?
I haven't been following session storage enough to offer guidance. You'll have to explain it in more detail for me to be able to offer any input. -Scott On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: > Reviving this CL. Marja, Sky, I'm going to need some guidance here. > > Initially I thought this would be a simple extension of the current > SessionStorageAssociate command. However, as Michael pointed out, doesn't > work > because new SSNs are created depending on what navigation entries are > committed. > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all the > associations. > > Do either of you know what the right notification is? > > -Albert > > > On 2012/08/27 18:36:41, michaeln wrote: >> >> > I think I finally understand your comment Michael. If you patch in my > > latest >> >> > CL, you'll see my log message only ever shows that we have 1 SSN in the >> > tab. > > >> > This is probably due to exactly what you said -- the new SSNs are never >> written >> > to the journal. > > >> Yup, not what you wanted probably. > > >> > What's the right solution here? Should I add a new notification for >> > when a >> > session storage namespace is created? That seems heavy weight... > > >> I don't know much about the session save/restore system, but *something* >> is >> being logged per tab navigation so the url to load into that tab is known >> at >> restore time, and the history of urls in that tab is also known at restore > > time >> >> to support navigating back (and forward) after having restored. The new >> navigation entries are added incrementally (well i'm guessing, like i said >> i >> don't know much about that stuff). Maybe we could also incrementally add >> associations to isolated SSNs coincident with new navigation entries. > > >> Maybe each navigation entry should have a strong association with a >> particular >> 'storage partition'? > > > > > https://chromiumcodereview.appspot.com/10850010/
The redux is this: SessionStorage is a per-tab concept. Each tab has a set of "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a Dictionary of Key-value pairs. In the simplest case (no isolated sites), two navigation entries whose top-level frames are in the same origin will see the same dictionary of key-value pairs. In the no isolation case, you could make the assumption that a single Tab has one SSN associated with it for the lifetime of the tab. The currently committed code relies on constraint implicitly by logging the Tab -> SSN relation when it receives NOTIFICATION_TAB_PARENTED. The Isolation Sites feature we are adding ( http://www.chromium.org/developers/design-documents/isolated-sites) complicates matters because it allows us to specify that different parts of an origin should get a different set of persistent state. For example, we might want to specify that the following URL pattern should NOT be able to share cookies, session storage, etc. with your normal web browsing. http://www.chase.com/login/* -- we define via an extensions that this goes to StoragePartition "A" In this setup an XSS vulnerability in, for example, http://www.chase.com/help will be useless in attacking http://www.chase.com/login. For SessonStorage specifically, if http://www.chase.com/login executed window.sessionStorage.setItem("auth", "secret"); and then http://www.chase.com/help tried to read it back using window.sessionStorage.getItem("auth"), the getItem() call would return nothing because the two pages are being serviced by different StoragePartitions. The implication here is that a tab can now have multiple SessionStorageNamespaces. It will gain a new one every time it navigates the top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can no longer sufficiently know all the SSNs that a tab might eventually have and on restore, any isolated sites would suddenly lose their storage. I'm not quite sure where's the best place to record the additional tab -> SSN associations. If we did it once per navigation entry commit, I worry that the commit log will become very noisy. Also, we'd likely have duplicate associations since, unless we adding extra accounting, there's no currently no way to know if a SSN has been written into the session restore log. Does that build a full enough picture? If not, maybe we should chat over VC? -Albert On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: > I haven't been following session storage enough to offer guidance. > You'll have to explain it in more detail for me to be able to offer > any input. > > -Scott > > On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: > > Reviving this CL. Marja, Sky, I'm going to need some guidance here. > > > > Initially I thought this would be a simple extension of the current > > SessionStorageAssociate command. However, as Michael pointed out, > doesn't > > work > > because new SSNs are created depending on what navigation entries are > > committed. > > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all the > > associations. > > > > Do either of you know what the right notification is? > > > > -Albert > > > > > > On 2012/08/27 18:36:41, michaeln wrote: > >> > >> > I think I finally understand your comment Michael. If you patch in my > > > > latest > >> > >> > CL, you'll see my log message only ever shows that we have 1 SSN in > the > >> > tab. > > > > > >> > This is probably due to exactly what you said -- the new SSNs are > never > >> written > >> > to the journal. > > > > > >> Yup, not what you wanted probably. > > > > > >> > What's the right solution here? Should I add a new notification for > >> > when a > >> > session storage namespace is created? That seems heavy weight... > > > > > >> I don't know much about the session save/restore system, but *something* > >> is > >> being logged per tab navigation so the url to load into that tab is > known > >> at > >> restore time, and the history of urls in that tab is also known at > restore > > > > time > >> > >> to support navigating back (and forward) after having restored. The new > >> navigation entries are added incrementally (well i'm guessing, like i > said > >> i > >> don't know much about that stuff). Maybe we could also incrementally add > >> associations to isolated SSNs coincident with new navigation entries. > > > > > >> Maybe each navigation entry should have a strong association with a > >> particular > >> 'storage partition'? > > > > > > > > > > https://chromiumcodereview.appspot.com/10850010/ >
ping... Marja, Scott, any thoughts? We've only got a few days left before m23, and if there's a chance I can preemptively fix the formatting of the Storage Associate command by adding another field like I did below, I'd love to do that now. However, I don't have confidence that such a change would fix the issue described in my last e-mail. -Albert On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) <ajwong@chromium.org>wrote: > The redux is this: > > SessionStorage is a per-tab concept. Each tab has a set of > "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a > Dictionary of Key-value pairs. > > In the simplest case (no isolated sites), two navigation entries whose > top-level frames are in the same origin will see the same dictionary of > key-value pairs. In the no isolation case, you could make the assumption > that a single Tab has one SSN associated with it for the lifetime of the > tab. The currently committed code relies on constraint implicitly by > logging the Tab -> SSN relation when it receives NOTIFICATION_TAB_PARENTED. > > The Isolation Sites feature we are adding ( > http://www.chromium.org/developers/design-documents/isolated-sites) complicates > matters because it allows us to specify that different parts of an origin > should get a different set of persistent state. For example, we might want > to specify that the following URL pattern should NOT be able to share > cookies, session storage, etc. with your normal web browsing. > > http://www.chase.com/login/* -- we define via an extensions that this > goes to StoragePartition "A" > > In this setup an XSS vulnerability in, for example, > http://www.chase.com/help will be useless in attacking > http://www.chase.com/login. For SessonStorage specifically, if > http://www.chase.com/login executed > > window.sessionStorage.setItem("auth", "secret"); > > and then http://www.chase.com/help tried to read it back > using window.sessionStorage.getItem("auth"), the getItem() call would > return nothing because the two pages are being serviced by different > StoragePartitions. > > The implication here is that a tab can now have multiple > SessionStorageNamespaces. It will gain a new one every time it navigates > the top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can > no longer sufficiently know all the SSNs that a tab might eventually have > and on restore, any isolated sites would suddenly lose their storage. > > I'm not quite sure where's the best place to record the additional tab -> > SSN associations. If we did it once per navigation entry commit, I worry > that the commit log will become very noisy. Also, we'd likely have > duplicate associations since, unless we adding extra accounting, there's no > currently no way to know if a SSN has been written into the session restore > log. > > Does that build a full enough picture? If not, maybe we should chat over > VC? > > -Albert > > > On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: > >> I haven't been following session storage enough to offer guidance. >> You'll have to explain it in more detail for me to be able to offer >> any input. >> >> -Scott >> >> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. >> > >> > Initially I thought this would be a simple extension of the current >> > SessionStorageAssociate command. However, as Michael pointed out, >> doesn't >> > work >> > because new SSNs are created depending on what navigation entries are >> > committed. >> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all the >> > associations. >> > >> > Do either of you know what the right notification is? >> > >> > -Albert >> > >> > >> > On 2012/08/27 18:36:41, michaeln wrote: >> >> >> >> > I think I finally understand your comment Michael. If you patch in >> my >> > >> > latest >> >> >> >> > CL, you'll see my log message only ever shows that we have 1 SSN in >> the >> >> > tab. >> > >> > >> >> > This is probably due to exactly what you said -- the new SSNs are >> never >> >> written >> >> > to the journal. >> > >> > >> >> Yup, not what you wanted probably. >> > >> > >> >> > What's the right solution here? Should I add a new notification for >> >> > when a >> >> > session storage namespace is created? That seems heavy weight... >> > >> > >> >> I don't know much about the session save/restore system, but >> *something* >> >> is >> >> being logged per tab navigation so the url to load into that tab is >> known >> >> at >> >> restore time, and the history of urls in that tab is also known at >> restore >> > >> > time >> >> >> >> to support navigating back (and forward) after having restored. The new >> >> navigation entries are added incrementally (well i'm guessing, like i >> said >> >> i >> >> don't know much about that stuff). Maybe we could also incrementally >> add >> >> associations to isolated SSNs coincident with new navigation entries. >> > >> > >> >> Maybe each navigation entry should have a strong association with a >> >> particular >> >> 'storage partition'? >> > >> > >> > >> > >> > https://chromiumcodereview.appspot.com/10850010/ >> > >
Sorry, I've been samped too. Thanks for the explanation, that clarifies things. If the ssn may be unique for each navigation then record it with UpdateTabNavigation. Specifically CreateUpdateTabNavigationCommand could include the ssn. If you needed to do a pass when restoring to figure out the set of ssns you could do that. -Scott On Wed, Sep 12, 2012 at 4:38 PM, Albert J. Wong (王重傑) <ajwong@chromium.org> wrote: > ping... > > Marja, Scott, any thoughts? We've only got a few days left before m23, and > if there's a chance I can preemptively fix the formatting of the Storage > Associate command by adding another field like I did below, I'd love to do > that now. > > However, I don't have confidence that such a change would fix the issue > described in my last e-mail. > > -Albert > > > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) <ajwong@chromium.org> > wrote: >> >> The redux is this: >> >> SessionStorage is a per-tab concept. Each tab has a set of >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a >> Dictionary of Key-value pairs. >> >> In the simplest case (no isolated sites), two navigation entries whose >> top-level frames are in the same origin will see the same dictionary of >> key-value pairs. In the no isolation case, you could make the assumption >> that a single Tab has one SSN associated with it for the lifetime of the >> tab. The currently committed code relies on constraint implicitly by >> logging the Tab -> SSN relation when it receives NOTIFICATION_TAB_PARENTED. >> >> The Isolation Sites feature we are adding >> (http://www.chromium.org/developers/design-documents/isolated-sites) >> complicates matters because it allows us to specify that different parts of >> an origin should get a different set of persistent state. For example, we >> might want to specify that the following URL pattern should NOT be able to >> share cookies, session storage, etc. with your normal web browsing. >> >> http://www.chase.com/login/* -- we define via an extensions that this >> goes to StoragePartition "A" >> >> In this setup an XSS vulnerability in, for example, >> http://www.chase.com/help will be useless in attacking >> http://www.chase.com/login. For SessonStorage specifically, if >> http://www.chase.com/login executed >> >> window.sessionStorage.setItem("auth", "secret"); >> >> and then http://www.chase.com/help tried to read it back using >> window.sessionStorage.getItem("auth"), the getItem() call would return >> nothing because the two pages are being serviced by different >> StoragePartitions. >> >> The implication here is that a tab can now have multiple >> SessionStorageNamespaces. It will gain a new one every time it navigates the >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can no >> longer sufficiently know all the SSNs that a tab might eventually have and >> on restore, any isolated sites would suddenly lose their storage. >> >> I'm not quite sure where's the best place to record the additional tab -> >> SSN associations. If we did it once per navigation entry commit, I worry >> that the commit log will become very noisy. Also, we'd likely have >> duplicate associations since, unless we adding extra accounting, there's no >> currently no way to know if a SSN has been written into the session restore >> log. >> >> Does that build a full enough picture? If not, maybe we should chat over >> VC? >> >> -Albert >> >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: >>> >>> I haven't been following session storage enough to offer guidance. >>> You'll have to explain it in more detail for me to be able to offer >>> any input. >>> >>> -Scott >>> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. >>> > >>> > Initially I thought this would be a simple extension of the current >>> > SessionStorageAssociate command. However, as Michael pointed out, >>> > doesn't >>> > work >>> > because new SSNs are created depending on what navigation entries are >>> > committed. >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all >>> > the >>> > associations. >>> > >>> > Do either of you know what the right notification is? >>> > >>> > -Albert >>> > >>> > >>> > On 2012/08/27 18:36:41, michaeln wrote: >>> >> >>> >> > I think I finally understand your comment Michael. If you patch in >>> >> > my >>> > >>> > latest >>> >> >>> >> > CL, you'll see my log message only ever shows that we have 1 SSN in >>> >> > the >>> >> > tab. >>> > >>> > >>> >> > This is probably due to exactly what you said -- the new SSNs are >>> >> > never >>> >> written >>> >> > to the journal. >>> > >>> > >>> >> Yup, not what you wanted probably. >>> > >>> > >>> >> > What's the right solution here? Should I add a new notification for >>> >> > when a >>> >> > session storage namespace is created? That seems heavy weight... >>> > >>> > >>> >> I don't know much about the session save/restore system, but >>> >> *something* >>> >> is >>> >> being logged per tab navigation so the url to load into that tab is >>> >> known >>> >> at >>> >> restore time, and the history of urls in that tab is also known at >>> >> restore >>> > >>> > time >>> >> >>> >> to support navigating back (and forward) after having restored. The >>> >> new >>> >> navigation entries are added incrementally (well i'm guessing, like i >>> >> said >>> >> i >>> >> don't know much about that stuff). Maybe we could also incrementally >>> >> add >>> >> associations to isolated SSNs coincident with new navigation entries. >>> > >>> > >>> >> Maybe each navigation entry should have a strong association with a >>> >> particular >>> >> 'storage partition'? >>> > >>> > >>> > >>> > >>> > https://chromiumcodereview.appspot.com/10850010/ >> >> >
Hi, sorry for late reply, and thanks for describing the isolated page thing, it's a lot clearer now. I don't know the answer to the problem though. Is there a way for you to notice when a tab gets a new SSN? Maybe send a notification when that happens? Marja On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> wrote: > ping... > > Marja, Scott, any thoughts? We've only got a few days left before m23, and > if there's a chance I can preemptively fix the formatting of the Storage > Associate command by adding another field like I did below, I'd love to do > that now. > > However, I don't have confidence that such a change would fix the issue > described in my last e-mail. > > -Albert > > > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) <ajwong@chromium.org> > wrote: >> >> The redux is this: >> >> SessionStorage is a per-tab concept. Each tab has a set of >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a >> Dictionary of Key-value pairs. >> >> In the simplest case (no isolated sites), two navigation entries whose >> top-level frames are in the same origin will see the same dictionary of >> key-value pairs. In the no isolation case, you could make the assumption >> that a single Tab has one SSN associated with it for the lifetime of the >> tab. The currently committed code relies on constraint implicitly by >> logging the Tab -> SSN relation when it receives NOTIFICATION_TAB_PARENTED. >> >> The Isolation Sites feature we are adding >> (http://www.chromium.org/developers/design-documents/isolated-sites) >> complicates matters because it allows us to specify that different parts of >> an origin should get a different set of persistent state. For example, we >> might want to specify that the following URL pattern should NOT be able to >> share cookies, session storage, etc. with your normal web browsing. >> >> http://www.chase.com/login/* -- we define via an extensions that this >> goes to StoragePartition "A" >> >> In this setup an XSS vulnerability in, for example, >> http://www.chase.com/help will be useless in attacking >> http://www.chase.com/login. For SessonStorage specifically, if >> http://www.chase.com/login executed >> >> window.sessionStorage.setItem("auth", "secret"); >> >> and then http://www.chase.com/help tried to read it back using >> window.sessionStorage.getItem("auth"), the getItem() call would return >> nothing because the two pages are being serviced by different >> StoragePartitions. >> >> The implication here is that a tab can now have multiple >> SessionStorageNamespaces. It will gain a new one every time it navigates the >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can no >> longer sufficiently know all the SSNs that a tab might eventually have and >> on restore, any isolated sites would suddenly lose their storage. >> >> I'm not quite sure where's the best place to record the additional tab -> >> SSN associations. If we did it once per navigation entry commit, I worry >> that the commit log will become very noisy. Also, we'd likely have >> duplicate associations since, unless we adding extra accounting, there's no >> currently no way to know if a SSN has been written into the session restore >> log. >> >> Does that build a full enough picture? If not, maybe we should chat over >> VC? >> >> -Albert >> >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: >>> >>> I haven't been following session storage enough to offer guidance. >>> You'll have to explain it in more detail for me to be able to offer >>> any input. >>> >>> -Scott >>> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. >>> > >>> > Initially I thought this would be a simple extension of the current >>> > SessionStorageAssociate command. However, as Michael pointed out, >>> > doesn't >>> > work >>> > because new SSNs are created depending on what navigation entries are >>> > committed. >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all >>> > the >>> > associations. >>> > >>> > Do either of you know what the right notification is? >>> > >>> > -Albert >>> > >>> > >>> > On 2012/08/27 18:36:41, michaeln wrote: >>> >> >>> >> > I think I finally understand your comment Michael. If you patch in >>> >> > my >>> > >>> > latest >>> >> >>> >> > CL, you'll see my log message only ever shows that we have 1 SSN in >>> >> > the >>> >> > tab. >>> > >>> > >>> >> > This is probably due to exactly what you said -- the new SSNs are >>> >> > never >>> >> written >>> >> > to the journal. >>> > >>> > >>> >> Yup, not what you wanted probably. >>> > >>> > >>> >> > What's the right solution here? Should I add a new notification for >>> >> > when a >>> >> > session storage namespace is created? That seems heavy weight... >>> > >>> > >>> >> I don't know much about the session save/restore system, but >>> >> *something* >>> >> is >>> >> being logged per tab navigation so the url to load into that tab is >>> >> known >>> >> at >>> >> restore time, and the history of urls in that tab is also known at >>> >> restore >>> > >>> > time >>> >> >>> >> to support navigating back (and forward) after having restored. The >>> >> new >>> >> navigation entries are added incrementally (well i'm guessing, like i >>> >> said >>> >> i >>> >> don't know much about that stuff). Maybe we could also incrementally >>> >> add >>> >> associations to isolated SSNs coincident with new navigation entries. >>> > >>> > >>> >> Maybe each navigation entry should have a strong association with a >>> >> particular >>> >> 'storage partition'? >>> > >>> > >>> > >>> > >>> > https://chromiumcodereview.appspot.com/10850010/ >> >> >
I could create a new notification I guess, but that feels heavy weight. What about Scott's idea about adding the SSN mapping into the CreateUpdateTabNavigationCommand? On restore, we'd need to collapse all of them to find the right set of SSN persistent IDs, but maybe that's not too bad? In terms of session restore log size growth, we'd add one extra copy of the persistent ID per UpdateTabNavigationCommand. In this configuration, the SessionStorageAssociateCommand would no longer be needed. Thoughts? -Albert On Thu, Sep 13, 2012 at 12:24 AM, Marja Hölttä <marja@google.com> wrote: > Hi, sorry for late reply, and thanks for describing the isolated page > thing, it's a lot clearer now. > > I don't know the answer to the problem though. > > Is there a way for you to notice when a tab gets a new SSN? Maybe send > a notification when that happens? > > Marja > > On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> > wrote: > > ping... > > > > Marja, Scott, any thoughts? We've only got a few days left before m23, > and > > if there's a chance I can preemptively fix the formatting of the Storage > > Associate command by adding another field like I did below, I'd love to > do > > that now. > > > > However, I don't have confidence that such a change would fix the issue > > described in my last e-mail. > > > > -Albert > > > > > > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) < > ajwong@chromium.org> > > wrote: > >> > >> The redux is this: > >> > >> SessionStorage is a per-tab concept. Each tab has a set of > >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a > >> Dictionary of Key-value pairs. > >> > >> In the simplest case (no isolated sites), two navigation entries whose > >> top-level frames are in the same origin will see the same dictionary of > >> key-value pairs. In the no isolation case, you could make the > assumption > >> that a single Tab has one SSN associated with it for the lifetime of the > >> tab. The currently committed code relies on constraint implicitly by > >> logging the Tab -> SSN relation when it receives > NOTIFICATION_TAB_PARENTED. > >> > >> The Isolation Sites feature we are adding > >> (http://www.chromium.org/developers/design-documents/isolated-sites) > >> complicates matters because it allows us to specify that different > parts of > >> an origin should get a different set of persistent state. For example, > we > >> might want to specify that the following URL pattern should NOT be able > to > >> share cookies, session storage, etc. with your normal web browsing. > >> > >> http://www.chase.com/login/* -- we define via an extensions that > this > >> goes to StoragePartition "A" > >> > >> In this setup an XSS vulnerability in, for example, > >> http://www.chase.com/help will be useless in attacking > >> http://www.chase.com/login. For SessonStorage specifically, if > >> http://www.chase.com/login executed > >> > >> window.sessionStorage.setItem("auth", "secret"); > >> > >> and then http://www.chase.com/help tried to read it back using > >> window.sessionStorage.getItem("auth"), the getItem() call would return > >> nothing because the two pages are being serviced by different > >> StoragePartitions. > >> > >> The implication here is that a tab can now have multiple > >> SessionStorageNamespaces. It will gain a new one every time it > navigates the > >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can > no > >> longer sufficiently know all the SSNs that a tab might eventually have > and > >> on restore, any isolated sites would suddenly lose their storage. > >> > >> I'm not quite sure where's the best place to record the additional tab > -> > >> SSN associations. If we did it once per navigation entry commit, I > worry > >> that the commit log will become very noisy. Also, we'd likely have > >> duplicate associations since, unless we adding extra accounting, > there's no > >> currently no way to know if a SSN has been written into the session > restore > >> log. > >> > >> Does that build a full enough picture? If not, maybe we should chat > over > >> VC? > >> > >> -Albert > >> > >> > >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: > >>> > >>> I haven't been following session storage enough to offer guidance. > >>> You'll have to explain it in more detail for me to be able to offer > >>> any input. > >>> > >>> -Scott > >>> > >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: > >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. > >>> > > >>> > Initially I thought this would be a simple extension of the current > >>> > SessionStorageAssociate command. However, as Michael pointed out, > >>> > doesn't > >>> > work > >>> > because new SSNs are created depending on what navigation entries are > >>> > committed. > >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all > >>> > the > >>> > associations. > >>> > > >>> > Do either of you know what the right notification is? > >>> > > >>> > -Albert > >>> > > >>> > > >>> > On 2012/08/27 18:36:41, michaeln wrote: > >>> >> > >>> >> > I think I finally understand your comment Michael. If you patch > in > >>> >> > my > >>> > > >>> > latest > >>> >> > >>> >> > CL, you'll see my log message only ever shows that we have 1 SSN > in > >>> >> > the > >>> >> > tab. > >>> > > >>> > > >>> >> > This is probably due to exactly what you said -- the new SSNs are > >>> >> > never > >>> >> written > >>> >> > to the journal. > >>> > > >>> > > >>> >> Yup, not what you wanted probably. > >>> > > >>> > > >>> >> > What's the right solution here? Should I add a new notification > for > >>> >> > when a > >>> >> > session storage namespace is created? That seems heavy weight... > >>> > > >>> > > >>> >> I don't know much about the session save/restore system, but > >>> >> *something* > >>> >> is > >>> >> being logged per tab navigation so the url to load into that tab is > >>> >> known > >>> >> at > >>> >> restore time, and the history of urls in that tab is also known at > >>> >> restore > >>> > > >>> > time > >>> >> > >>> >> to support navigating back (and forward) after having restored. The > >>> >> new > >>> >> navigation entries are added incrementally (well i'm guessing, like > i > >>> >> said > >>> >> i > >>> >> don't know much about that stuff). Maybe we could also incrementally > >>> >> add > >>> >> associations to isolated SSNs coincident with new navigation > entries. > >>> > > >>> > > >>> >> Maybe each navigation entry should have a strong association with a > >>> >> particular > >>> >> 'storage partition'? > >>> > > >>> > > >>> > > >>> > > >>> > https://chromiumcodereview.appspot.com/10850010/ > >> > >> > > >
Yeah, using CreateUpdateTabNavigationCommand would work, too. It's quite redundant to have the SSN in every command though, since I'd guess >> 95% of the navigations don't involve an isolated app. But if Scott says it's okay to add it, I won't object :) On 13 September 2012 09:38, Albert J. Wong (王重傑) <ajwong@chromium.org> wrote: > I could create a new notification I guess, but that feels heavy weight. > > What about Scott's idea about adding the SSN mapping into the > CreateUpdateTabNavigationCommand? On restore, we'd need to collapse all of > them to find the right set of SSN persistent IDs, but maybe that's not too > bad? In terms of session restore log size growth, we'd add one extra copy > of the persistent ID per UpdateTabNavigationCommand. In this configuration, > the SessionStorageAssociateCommand would no longer be needed. > > Thoughts? > > -Albert > > On Thu, Sep 13, 2012 at 12:24 AM, Marja Hölttä <marja@google.com> wrote: >> >> Hi, sorry for late reply, and thanks for describing the isolated page >> thing, it's a lot clearer now. >> >> I don't know the answer to the problem though. >> >> Is there a way for you to notice when a tab gets a new SSN? Maybe send >> a notification when that happens? >> >> Marja >> >> On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> >> wrote: >> > ping... >> > >> > Marja, Scott, any thoughts? We've only got a few days left before m23, >> > and >> > if there's a chance I can preemptively fix the formatting of the Storage >> > Associate command by adding another field like I did below, I'd love to >> > do >> > that now. >> > >> > However, I don't have confidence that such a change would fix the issue >> > described in my last e-mail. >> > >> > -Albert >> > >> > >> > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) >> > <ajwong@chromium.org> >> > wrote: >> >> >> >> The redux is this: >> >> >> >> SessionStorage is a per-tab concept. Each tab has a set of >> >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a >> >> Dictionary of Key-value pairs. >> >> >> >> In the simplest case (no isolated sites), two navigation entries whose >> >> top-level frames are in the same origin will see the same dictionary of >> >> key-value pairs. In the no isolation case, you could make the >> >> assumption >> >> that a single Tab has one SSN associated with it for the lifetime of >> >> the >> >> tab. The currently committed code relies on constraint implicitly by >> >> logging the Tab -> SSN relation when it receives >> >> NOTIFICATION_TAB_PARENTED. >> >> >> >> The Isolation Sites feature we are adding >> >> (http://www.chromium.org/developers/design-documents/isolated-sites) >> >> complicates matters because it allows us to specify that different >> >> parts of >> >> an origin should get a different set of persistent state. For example, >> >> we >> >> might want to specify that the following URL pattern should NOT be able >> >> to >> >> share cookies, session storage, etc. with your normal web browsing. >> >> >> >> http://www.chase.com/login/* -- we define via an extensions that >> >> this >> >> goes to StoragePartition "A" >> >> >> >> In this setup an XSS vulnerability in, for example, >> >> http://www.chase.com/help will be useless in attacking >> >> http://www.chase.com/login. For SessonStorage specifically, if >> >> http://www.chase.com/login executed >> >> >> >> window.sessionStorage.setItem("auth", "secret"); >> >> >> >> and then http://www.chase.com/help tried to read it back using >> >> window.sessionStorage.getItem("auth"), the getItem() call would return >> >> nothing because the two pages are being serviced by different >> >> StoragePartitions. >> >> >> >> The implication here is that a tab can now have multiple >> >> SessionStorageNamespaces. It will gain a new one every time it >> >> navigates the >> >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can >> >> no >> >> longer sufficiently know all the SSNs that a tab might eventually have >> >> and >> >> on restore, any isolated sites would suddenly lose their storage. >> >> >> >> I'm not quite sure where's the best place to record the additional tab >> >> -> >> >> SSN associations. If we did it once per navigation entry commit, I >> >> worry >> >> that the commit log will become very noisy. Also, we'd likely have >> >> duplicate associations since, unless we adding extra accounting, >> >> there's no >> >> currently no way to know if a SSN has been written into the session >> >> restore >> >> log. >> >> >> >> Does that build a full enough picture? If not, maybe we should chat >> >> over >> >> VC? >> >> >> >> -Albert >> >> >> >> >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: >> >>> >> >>> I haven't been following session storage enough to offer guidance. >> >>> You'll have to explain it in more detail for me to be able to offer >> >>> any input. >> >>> >> >>> -Scott >> >>> >> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >> >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. >> >>> > >> >>> > Initially I thought this would be a simple extension of the current >> >>> > SessionStorageAssociate command. However, as Michael pointed out, >> >>> > doesn't >> >>> > work >> >>> > because new SSNs are created depending on what navigation entries >> >>> > are >> >>> > committed. >> >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all >> >>> > the >> >>> > associations. >> >>> > >> >>> > Do either of you know what the right notification is? >> >>> > >> >>> > -Albert >> >>> > >> >>> > >> >>> > On 2012/08/27 18:36:41, michaeln wrote: >> >>> >> >> >>> >> > I think I finally understand your comment Michael. If you patch >> >>> >> > in >> >>> >> > my >> >>> > >> >>> > latest >> >>> >> >> >>> >> > CL, you'll see my log message only ever shows that we have 1 SSN >> >>> >> > in >> >>> >> > the >> >>> >> > tab. >> >>> > >> >>> > >> >>> >> > This is probably due to exactly what you said -- the new SSNs are >> >>> >> > never >> >>> >> written >> >>> >> > to the journal. >> >>> > >> >>> > >> >>> >> Yup, not what you wanted probably. >> >>> > >> >>> > >> >>> >> > What's the right solution here? Should I add a new notification >> >>> >> > for >> >>> >> > when a >> >>> >> > session storage namespace is created? That seems heavy weight... >> >>> > >> >>> > >> >>> >> I don't know much about the session save/restore system, but >> >>> >> *something* >> >>> >> is >> >>> >> being logged per tab navigation so the url to load into that tab is >> >>> >> known >> >>> >> at >> >>> >> restore time, and the history of urls in that tab is also known at >> >>> >> restore >> >>> > >> >>> > time >> >>> >> >> >>> >> to support navigating back (and forward) after having restored. The >> >>> >> new >> >>> >> navigation entries are added incrementally (well i'm guessing, like >> >>> >> i >> >>> >> said >> >>> >> i >> >>> >> don't know much about that stuff). Maybe we could also >> >>> >> incrementally >> >>> >> add >> >>> >> associations to isolated SSNs coincident with new navigation >> >>> >> entries. >> >>> > >> >>> > >> >>> >> Maybe each navigation entry should have a strong association with a >> >>> >> particular >> >>> >> 'storage partition'? >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > https://chromiumcodereview.appspot.com/10850010/ >> >> >> >> >> > > >
I'm assuming the SSN is smallish. Is that not the case? -Scott On Thu, Sep 13, 2012 at 12:38 AM, Albert J. Wong (王重傑) <ajwong@chromium.org> wrote: > I could create a new notification I guess, but that feels heavy weight. > > What about Scott's idea about adding the SSN mapping into the > CreateUpdateTabNavigationCommand? On restore, we'd need to collapse all of > them to find the right set of SSN persistent IDs, but maybe that's not too > bad? In terms of session restore log size growth, we'd add one extra copy > of the persistent ID per UpdateTabNavigationCommand. In this configuration, > the SessionStorageAssociateCommand would no longer be needed. > > Thoughts? > > -Albert > > On Thu, Sep 13, 2012 at 12:24 AM, Marja Hölttä <marja@google.com> wrote: >> >> Hi, sorry for late reply, and thanks for describing the isolated page >> thing, it's a lot clearer now. >> >> I don't know the answer to the problem though. >> >> Is there a way for you to notice when a tab gets a new SSN? Maybe send >> a notification when that happens? >> >> Marja >> >> On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> >> wrote: >> > ping... >> > >> > Marja, Scott, any thoughts? We've only got a few days left before m23, >> > and >> > if there's a chance I can preemptively fix the formatting of the Storage >> > Associate command by adding another field like I did below, I'd love to >> > do >> > that now. >> > >> > However, I don't have confidence that such a change would fix the issue >> > described in my last e-mail. >> > >> > -Albert >> > >> > >> > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) >> > <ajwong@chromium.org> >> > wrote: >> >> >> >> The redux is this: >> >> >> >> SessionStorage is a per-tab concept. Each tab has a set of >> >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin to a >> >> Dictionary of Key-value pairs. >> >> >> >> In the simplest case (no isolated sites), two navigation entries whose >> >> top-level frames are in the same origin will see the same dictionary of >> >> key-value pairs. In the no isolation case, you could make the >> >> assumption >> >> that a single Tab has one SSN associated with it for the lifetime of >> >> the >> >> tab. The currently committed code relies on constraint implicitly by >> >> logging the Tab -> SSN relation when it receives >> >> NOTIFICATION_TAB_PARENTED. >> >> >> >> The Isolation Sites feature we are adding >> >> (http://www.chromium.org/developers/design-documents/isolated-sites) >> >> complicates matters because it allows us to specify that different >> >> parts of >> >> an origin should get a different set of persistent state. For example, >> >> we >> >> might want to specify that the following URL pattern should NOT be able >> >> to >> >> share cookies, session storage, etc. with your normal web browsing. >> >> >> >> http://www.chase.com/login/* -- we define via an extensions that >> >> this >> >> goes to StoragePartition "A" >> >> >> >> In this setup an XSS vulnerability in, for example, >> >> http://www.chase.com/help will be useless in attacking >> >> http://www.chase.com/login. For SessonStorage specifically, if >> >> http://www.chase.com/login executed >> >> >> >> window.sessionStorage.setItem("auth", "secret"); >> >> >> >> and then http://www.chase.com/help tried to read it back using >> >> window.sessionStorage.getItem("auth"), the getItem() call would return >> >> nothing because the two pages are being serviced by different >> >> StoragePartitions. >> >> >> >> The implication here is that a tab can now have multiple >> >> SessionStorageNamespaces. It will gain a new one every time it >> >> navigates the >> >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED can >> >> no >> >> longer sufficiently know all the SSNs that a tab might eventually have >> >> and >> >> on restore, any isolated sites would suddenly lose their storage. >> >> >> >> I'm not quite sure where's the best place to record the additional tab >> >> -> >> >> SSN associations. If we did it once per navigation entry commit, I >> >> worry >> >> that the commit log will become very noisy. Also, we'd likely have >> >> duplicate associations since, unless we adding extra accounting, >> >> there's no >> >> currently no way to know if a SSN has been written into the session >> >> restore >> >> log. >> >> >> >> Does that build a full enough picture? If not, maybe we should chat >> >> over >> >> VC? >> >> >> >> -Albert >> >> >> >> >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> wrote: >> >>> >> >>> I haven't been following session storage enough to offer guidance. >> >>> You'll have to explain it in more detail for me to be able to offer >> >>> any input. >> >>> >> >>> -Scott >> >>> >> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >> >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance here. >> >>> > >> >>> > Initially I thought this would be a simple extension of the current >> >>> > SessionStorageAssociate command. However, as Michael pointed out, >> >>> > doesn't >> >>> > work >> >>> > because new SSNs are created depending on what navigation entries >> >>> > are >> >>> > committed. >> >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording all >> >>> > the >> >>> > associations. >> >>> > >> >>> > Do either of you know what the right notification is? >> >>> > >> >>> > -Albert >> >>> > >> >>> > >> >>> > On 2012/08/27 18:36:41, michaeln wrote: >> >>> >> >> >>> >> > I think I finally understand your comment Michael. If you patch >> >>> >> > in >> >>> >> > my >> >>> > >> >>> > latest >> >>> >> >> >>> >> > CL, you'll see my log message only ever shows that we have 1 SSN >> >>> >> > in >> >>> >> > the >> >>> >> > tab. >> >>> > >> >>> > >> >>> >> > This is probably due to exactly what you said -- the new SSNs are >> >>> >> > never >> >>> >> written >> >>> >> > to the journal. >> >>> > >> >>> > >> >>> >> Yup, not what you wanted probably. >> >>> > >> >>> > >> >>> >> > What's the right solution here? Should I add a new notification >> >>> >> > for >> >>> >> > when a >> >>> >> > session storage namespace is created? That seems heavy weight... >> >>> > >> >>> > >> >>> >> I don't know much about the session save/restore system, but >> >>> >> *something* >> >>> >> is >> >>> >> being logged per tab navigation so the url to load into that tab is >> >>> >> known >> >>> >> at >> >>> >> restore time, and the history of urls in that tab is also known at >> >>> >> restore >> >>> > >> >>> > time >> >>> >> >> >>> >> to support navigating back (and forward) after having restored. The >> >>> >> new >> >>> >> navigation entries are added incrementally (well i'm guessing, like >> >>> >> i >> >>> >> said >> >>> >> i >> >>> >> don't know much about that stuff). Maybe we could also >> >>> >> incrementally >> >>> >> add >> >>> >> associations to isolated SSNs coincident with new navigation >> >>> >> entries. >> >>> > >> >>> > >> >>> >> Maybe each navigation entry should have a strong association with a >> >>> >> particular >> >>> >> 'storage partition'? >> >>> > >> >>> > >> >>> > >> >>> > >> >>> > https://chromiumcodereview.appspot.com/10850010/ >> >> >> >> >> > > >
The SSN's persistent ID, which is what would be written to disk is a GUID. So it's a 128-bits of hex with 4 dash...36 chars I think? Is that's small enough to be added to UpdateTabNavigationCommand? If it is, do we care enough about this to try rush a patch in that would switch from SessionStorageAssociateCommand to UpdateTabNavigationCommand in m23? -Albert On Thu, Sep 13, 2012 at 9:03 AM, Scott Violet <sky@chromium.org> wrote: > I'm assuming the SSN is smallish. Is that not the case? > > -Scott > > On Thu, Sep 13, 2012 at 12:38 AM, Albert J. Wong (王重傑) > <ajwong@chromium.org> wrote: > > I could create a new notification I guess, but that feels heavy weight. > > > > What about Scott's idea about adding the SSN mapping into the > > CreateUpdateTabNavigationCommand? On restore, we'd need to collapse all > of > > them to find the right set of SSN persistent IDs, but maybe that's not > too > > bad? In terms of session restore log size growth, we'd add one extra > copy > > of the persistent ID per UpdateTabNavigationCommand. In this > configuration, > > the SessionStorageAssociateCommand would no longer be needed. > > > > Thoughts? > > > > -Albert > > > > On Thu, Sep 13, 2012 at 12:24 AM, Marja Hölttä <marja@google.com> wrote: > >> > >> Hi, sorry for late reply, and thanks for describing the isolated page > >> thing, it's a lot clearer now. > >> > >> I don't know the answer to the problem though. > >> > >> Is there a way for you to notice when a tab gets a new SSN? Maybe send > >> a notification when that happens? > >> > >> Marja > >> > >> On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> > >> wrote: > >> > ping... > >> > > >> > Marja, Scott, any thoughts? We've only got a few days left before > m23, > >> > and > >> > if there's a chance I can preemptively fix the formatting of the > Storage > >> > Associate command by adding another field like I did below, I'd love > to > >> > do > >> > that now. > >> > > >> > However, I don't have confidence that such a change would fix the > issue > >> > described in my last e-mail. > >> > > >> > -Albert > >> > > >> > > >> > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) > >> > <ajwong@chromium.org> > >> > wrote: > >> >> > >> >> The redux is this: > >> >> > >> >> SessionStorage is a per-tab concept. Each tab has a set of > >> >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin > to a > >> >> Dictionary of Key-value pairs. > >> >> > >> >> In the simplest case (no isolated sites), two navigation entries > whose > >> >> top-level frames are in the same origin will see the same dictionary > of > >> >> key-value pairs. In the no isolation case, you could make the > >> >> assumption > >> >> that a single Tab has one SSN associated with it for the lifetime of > >> >> the > >> >> tab. The currently committed code relies on constraint implicitly by > >> >> logging the Tab -> SSN relation when it receives > >> >> NOTIFICATION_TAB_PARENTED. > >> >> > >> >> The Isolation Sites feature we are adding > >> >> (http://www.chromium.org/developers/design-documents/isolated-sites) > >> >> complicates matters because it allows us to specify that different > >> >> parts of > >> >> an origin should get a different set of persistent state. For > example, > >> >> we > >> >> might want to specify that the following URL pattern should NOT be > able > >> >> to > >> >> share cookies, session storage, etc. with your normal web browsing. > >> >> > >> >> http://www.chase.com/login/* -- we define via an extensions that > >> >> this > >> >> goes to StoragePartition "A" > >> >> > >> >> In this setup an XSS vulnerability in, for example, > >> >> http://www.chase.com/help will be useless in attacking > >> >> http://www.chase.com/login. For SessonStorage specifically, if > >> >> http://www.chase.com/login executed > >> >> > >> >> window.sessionStorage.setItem("auth", "secret"); > >> >> > >> >> and then http://www.chase.com/help tried to read it back using > >> >> window.sessionStorage.getItem("auth"), the getItem() call would > return > >> >> nothing because the two pages are being serviced by different > >> >> StoragePartitions. > >> >> > >> >> The implication here is that a tab can now have multiple > >> >> SessionStorageNamespaces. It will gain a new one every time it > >> >> navigates the > >> >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED > can > >> >> no > >> >> longer sufficiently know all the SSNs that a tab might eventually > have > >> >> and > >> >> on restore, any isolated sites would suddenly lose their storage. > >> >> > >> >> I'm not quite sure where's the best place to record the additional > tab > >> >> -> > >> >> SSN associations. If we did it once per navigation entry commit, I > >> >> worry > >> >> that the commit log will become very noisy. Also, we'd likely have > >> >> duplicate associations since, unless we adding extra accounting, > >> >> there's no > >> >> currently no way to know if a SSN has been written into the session > >> >> restore > >> >> log. > >> >> > >> >> Does that build a full enough picture? If not, maybe we should chat > >> >> over > >> >> VC? > >> >> > >> >> -Albert > >> >> > >> >> > >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> > wrote: > >> >>> > >> >>> I haven't been following session storage enough to offer guidance. > >> >>> You'll have to explain it in more detail for me to be able to offer > >> >>> any input. > >> >>> > >> >>> -Scott > >> >>> > >> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: > >> >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance > here. > >> >>> > > >> >>> > Initially I thought this would be a simple extension of the > current > >> >>> > SessionStorageAssociate command. However, as Michael pointed out, > >> >>> > doesn't > >> >>> > work > >> >>> > because new SSNs are created depending on what navigation entries > >> >>> > are > >> >>> > committed. > >> >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording > all > >> >>> > the > >> >>> > associations. > >> >>> > > >> >>> > Do either of you know what the right notification is? > >> >>> > > >> >>> > -Albert > >> >>> > > >> >>> > > >> >>> > On 2012/08/27 18:36:41, michaeln wrote: > >> >>> >> > >> >>> >> > I think I finally understand your comment Michael. If you > patch > >> >>> >> > in > >> >>> >> > my > >> >>> > > >> >>> > latest > >> >>> >> > >> >>> >> > CL, you'll see my log message only ever shows that we have 1 > SSN > >> >>> >> > in > >> >>> >> > the > >> >>> >> > tab. > >> >>> > > >> >>> > > >> >>> >> > This is probably due to exactly what you said -- the new SSNs > are > >> >>> >> > never > >> >>> >> written > >> >>> >> > to the journal. > >> >>> > > >> >>> > > >> >>> >> Yup, not what you wanted probably. > >> >>> > > >> >>> > > >> >>> >> > What's the right solution here? Should I add a new > notification > >> >>> >> > for > >> >>> >> > when a > >> >>> >> > session storage namespace is created? That seems heavy > weight... > >> >>> > > >> >>> > > >> >>> >> I don't know much about the session save/restore system, but > >> >>> >> *something* > >> >>> >> is > >> >>> >> being logged per tab navigation so the url to load into that tab > is > >> >>> >> known > >> >>> >> at > >> >>> >> restore time, and the history of urls in that tab is also known > at > >> >>> >> restore > >> >>> > > >> >>> > time > >> >>> >> > >> >>> >> to support navigating back (and forward) after having restored. > The > >> >>> >> new > >> >>> >> navigation entries are added incrementally (well i'm guessing, > like > >> >>> >> i > >> >>> >> said > >> >>> >> i > >> >>> >> don't know much about that stuff). Maybe we could also > >> >>> >> incrementally > >> >>> >> add > >> >>> >> associations to isolated SSNs coincident with new navigation > >> >>> >> entries. > >> >>> > > >> >>> > > >> >>> >> Maybe each navigation entry should have a strong association > with a > >> >>> >> particular > >> >>> >> 'storage partition'? > >> >>> > > >> >>> > > >> >>> > > >> >>> > > >> >>> > https://chromiumcodereview.appspot.com/10850010/ > >> >> > >> >> > >> > > > > > >
Adding to UpdateTabNavigationCommand is fine by me. If you have time to change for 23, go for it. -Scott On Thu, Sep 13, 2012 at 5:30 PM, Albert J. Wong (王重傑) <ajwong@chromium.org> wrote: > The SSN's persistent ID, which is what would be written to disk is a GUID. > So it's a 128-bits of hex with 4 dash...36 chars I think? > > Is that's small enough to be added to UpdateTabNavigationCommand? > > If it is, do we care enough about this to try rush a patch in that would > switch from SessionStorageAssociateCommand to UpdateTabNavigationCommand in > m23? > > -Albert > > > On Thu, Sep 13, 2012 at 9:03 AM, Scott Violet <sky@chromium.org> wrote: >> >> I'm assuming the SSN is smallish. Is that not the case? >> >> -Scott >> >> On Thu, Sep 13, 2012 at 12:38 AM, Albert J. Wong (王重傑) >> <ajwong@chromium.org> wrote: >> > I could create a new notification I guess, but that feels heavy weight. >> > >> > What about Scott's idea about adding the SSN mapping into the >> > CreateUpdateTabNavigationCommand? On restore, we'd need to collapse all >> > of >> > them to find the right set of SSN persistent IDs, but maybe that's not >> > too >> > bad? In terms of session restore log size growth, we'd add one extra >> > copy >> > of the persistent ID per UpdateTabNavigationCommand. In this >> > configuration, >> > the SessionStorageAssociateCommand would no longer be needed. >> > >> > Thoughts? >> > >> > -Albert >> > >> > On Thu, Sep 13, 2012 at 12:24 AM, Marja Hölttä <marja@google.com> wrote: >> >> >> >> Hi, sorry for late reply, and thanks for describing the isolated page >> >> thing, it's a lot clearer now. >> >> >> >> I don't know the answer to the problem though. >> >> >> >> Is there a way for you to notice when a tab gets a new SSN? Maybe send >> >> a notification when that happens? >> >> >> >> Marja >> >> >> >> On 13 September 2012 01:38, Albert J. Wong (王重傑) <ajwong@chromium.org> >> >> wrote: >> >> > ping... >> >> > >> >> > Marja, Scott, any thoughts? We've only got a few days left before >> >> > m23, >> >> > and >> >> > if there's a chance I can preemptively fix the formatting of the >> >> > Storage >> >> > Associate command by adding another field like I did below, I'd love >> >> > to >> >> > do >> >> > that now. >> >> > >> >> > However, I don't have confidence that such a change would fix the >> >> > issue >> >> > described in my last e-mail. >> >> > >> >> > -Albert >> >> > >> >> > >> >> > On Tue, Sep 11, 2012 at 4:26 PM, Albert J. Wong (王重傑) >> >> > <ajwong@chromium.org> >> >> > wrote: >> >> >> >> >> >> The redux is this: >> >> >> >> >> >> SessionStorage is a per-tab concept. Each tab has a set of >> >> >> "SessionStorageNamespace" (SSN) objects. These SSNs map an origin >> >> >> to a >> >> >> Dictionary of Key-value pairs. >> >> >> >> >> >> In the simplest case (no isolated sites), two navigation entries >> >> >> whose >> >> >> top-level frames are in the same origin will see the same dictionary >> >> >> of >> >> >> key-value pairs. In the no isolation case, you could make the >> >> >> assumption >> >> >> that a single Tab has one SSN associated with it for the lifetime of >> >> >> the >> >> >> tab. The currently committed code relies on constraint implicitly >> >> >> by >> >> >> logging the Tab -> SSN relation when it receives >> >> >> NOTIFICATION_TAB_PARENTED. >> >> >> >> >> >> The Isolation Sites feature we are adding >> >> >> (http://www.chromium.org/developers/design-documents/isolated-sites) >> >> >> complicates matters because it allows us to specify that different >> >> >> parts of >> >> >> an origin should get a different set of persistent state. For >> >> >> example, >> >> >> we >> >> >> might want to specify that the following URL pattern should NOT be >> >> >> able >> >> >> to >> >> >> share cookies, session storage, etc. with your normal web browsing. >> >> >> >> >> >> http://www.chase.com/login/* -- we define via an extensions that >> >> >> this >> >> >> goes to StoragePartition "A" >> >> >> >> >> >> In this setup an XSS vulnerability in, for example, >> >> >> http://www.chase.com/help will be useless in attacking >> >> >> http://www.chase.com/login. For SessonStorage specifically, if >> >> >> http://www.chase.com/login executed >> >> >> >> >> >> window.sessionStorage.setItem("auth", "secret"); >> >> >> >> >> >> and then http://www.chase.com/help tried to read it back using >> >> >> window.sessionStorage.getItem("auth"), the getItem() call would >> >> >> return >> >> >> nothing because the two pages are being serviced by different >> >> >> StoragePartitions. >> >> >> >> >> >> The implication here is that a tab can now have multiple >> >> >> SessionStorageNamespaces. It will gain a new one every time it >> >> >> navigates the >> >> >> top-level frame to an isolated site. Now NOTIFICATION_TAB_PARENTED >> >> >> can >> >> >> no >> >> >> longer sufficiently know all the SSNs that a tab might eventually >> >> >> have >> >> >> and >> >> >> on restore, any isolated sites would suddenly lose their storage. >> >> >> >> >> >> I'm not quite sure where's the best place to record the additional >> >> >> tab >> >> >> -> >> >> >> SSN associations. If we did it once per navigation entry commit, I >> >> >> worry >> >> >> that the commit log will become very noisy. Also, we'd likely have >> >> >> duplicate associations since, unless we adding extra accounting, >> >> >> there's no >> >> >> currently no way to know if a SSN has been written into the session >> >> >> restore >> >> >> log. >> >> >> >> >> >> Does that build a full enough picture? If not, maybe we should chat >> >> >> over >> >> >> VC? >> >> >> >> >> >> -Albert >> >> >> >> >> >> >> >> >> On Tue, Sep 11, 2012 at 4:05 PM, Scott Violet <sky@chromium.org> >> >> >> wrote: >> >> >>> >> >> >>> I haven't been following session storage enough to offer guidance. >> >> >>> You'll have to explain it in more detail for me to be able to offer >> >> >>> any input. >> >> >>> >> >> >>> -Scott >> >> >>> >> >> >>> On Tue, Sep 11, 2012 at 2:23 PM, <ajwong@chromium.org> wrote: >> >> >>> > Reviving this CL. Marja, Sky, I'm going to need some guidance >> >> >>> > here. >> >> >>> > >> >> >>> > Initially I thought this would be a simple extension of the >> >> >>> > current >> >> >>> > SessionStorageAssociate command. However, as Michael pointed >> >> >>> > out, >> >> >>> > doesn't >> >> >>> > work >> >> >>> > because new SSNs are created depending on what navigation entries >> >> >>> > are >> >> >>> > committed. >> >> >>> > NOTIFICATION_TAB_PARENTED is no longer sufficient for recording >> >> >>> > all >> >> >>> > the >> >> >>> > associations. >> >> >>> > >> >> >>> > Do either of you know what the right notification is? >> >> >>> > >> >> >>> > -Albert >> >> >>> > >> >> >>> > >> >> >>> > On 2012/08/27 18:36:41, michaeln wrote: >> >> >>> >> >> >> >>> >> > I think I finally understand your comment Michael. If you >> >> >>> >> > patch >> >> >>> >> > in >> >> >>> >> > my >> >> >>> > >> >> >>> > latest >> >> >>> >> >> >> >>> >> > CL, you'll see my log message only ever shows that we have 1 >> >> >>> >> > SSN >> >> >>> >> > in >> >> >>> >> > the >> >> >>> >> > tab. >> >> >>> > >> >> >>> > >> >> >>> >> > This is probably due to exactly what you said -- the new SSNs >> >> >>> >> > are >> >> >>> >> > never >> >> >>> >> written >> >> >>> >> > to the journal. >> >> >>> > >> >> >>> > >> >> >>> >> Yup, not what you wanted probably. >> >> >>> > >> >> >>> > >> >> >>> >> > What's the right solution here? Should I add a new >> >> >>> >> > notification >> >> >>> >> > for >> >> >>> >> > when a >> >> >>> >> > session storage namespace is created? That seems heavy >> >> >>> >> > weight... >> >> >>> > >> >> >>> > >> >> >>> >> I don't know much about the session save/restore system, but >> >> >>> >> *something* >> >> >>> >> is >> >> >>> >> being logged per tab navigation so the url to load into that tab >> >> >>> >> is >> >> >>> >> known >> >> >>> >> at >> >> >>> >> restore time, and the history of urls in that tab is also known >> >> >>> >> at >> >> >>> >> restore >> >> >>> > >> >> >>> > time >> >> >>> >> >> >> >>> >> to support navigating back (and forward) after having restored. >> >> >>> >> The >> >> >>> >> new >> >> >>> >> navigation entries are added incrementally (well i'm guessing, >> >> >>> >> like >> >> >>> >> i >> >> >>> >> said >> >> >>> >> i >> >> >>> >> don't know much about that stuff). Maybe we could also >> >> >>> >> incrementally >> >> >>> >> add >> >> >>> >> associations to isolated SSNs coincident with new navigation >> >> >>> >> entries. >> >> >>> > >> >> >>> > >> >> >>> >> Maybe each navigation entry should have a strong association >> >> >>> >> with a >> >> >>> >> particular >> >> >>> >> 'storage partition'? >> >> >>> > >> >> >>> > >> >> >>> > >> >> >>> > >> >> >>> > https://chromiumcodereview.appspot.com/10850010/ >> >> >> >> >> >> >> >> > >> > >> > > > |
