Bug 28094 – Cleanup DOM Storage namespace shutdown code + usage of security origin.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jeremy Orlow]

If a storage namespace is not explicitly closed, be sure to do so on 
destruction of the object.  In addition, the close call should wait
on the background thread finishing its syncing.  (Not doing so is actually
a regression from the original LocalStorage code.)

There's no point to passing in the SecurityOrigin when copying a storage
area since what was passed in is exactly what is stored within each
storage area.  In addition, the non-copy constructor should take in a
PassRefPtr rather than a pointer since that pointer was only passed into
the constuctor for RefPtr's anyway.

[Jeremy Orlow]

Created attachment 34362 [details]
Patch v1

[Michael Nordman]

A small drive by comment...

97     for (StorageAreaMap::iterator i = m_storageAreaMap.begin(); i != end; ++i) {
98         RefPtr<StorageAreaImpl> areaCopy = i->second->copy();
99         newNamespace->m_storageAreaMap.set(i->first, areaCopy.release());
100    }

I think you could remove a couple of lines with...

for (StorageAreaMap::iterator i = m_storageAreaMap.begin(); i != end; ++i)
    newNamespace->m_storageAreaMap.set(i->first, i->second->copy());

[Jeremy Orlow]

(In reply to comment #2)
> A small drive by comment...
> 
> 97     for (StorageAreaMap::iterator i = m_storageAreaMap.begin(); i != end;
> ++i) {
> 98         RefPtr<StorageAreaImpl> areaCopy = i->second->copy();
> 99         newNamespace->m_storageAreaMap.set(i->first, areaCopy.release());
> 100    }
> 
> I think you could remove a couple of lines with...
> 
> for (StorageAreaMap::iterator i = m_storageAreaMap.begin(); i != end; ++i)
>     newNamespace->m_storageAreaMap.set(i->first, i->second->copy());

Good point.  I'll change that.

[Darin Fisher (:fishd, Google)]

Comment on attachment 34362 [details]
Patch v1

r=me w/ michael's suggested cleanup

[Jeremy Orlow]

Was landed