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.
Created attachment 34362 [details] Patch v1
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());
(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.
Comment on attachment 34362 [details] Patch v1 r=me w/ michael's suggested cleanup
Was landed