Bug 26698

Summary: Combined LocalStorageArea and SessionStorageArea into StorageArea
Product: WebKit Reporter: Jeremy Orlow <jorlow>
Component: New BugsAssignee: Jeremy Orlow <jorlow>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, beidson, dglazkov, fishd, michaeln, playmobil, sam, yael, zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 25376    
Attachments:
Description Flags
patch v1
none
patch v2
fishd: review+
patch v3 fishd: review+

Jeremy Orlow
Reported 2009-06-24 17:34:50 PDT
Combined LocalStorageArea and SessionStorageArea into StorageArea since (after my other refactorings) there are no longer substantial differences between the two.
Attachments
patch v1 (39.54 KB, patch)
2009-06-24 18:28 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
patch v2 (39.37 KB, patch)
2009-06-24 18:31 PDT, Jeremy Orlow 		fishd: review+
DetailsFormatted DiffDiff
patch v3 (39.47 KB, patch)
2009-06-25 12:09 PDT, Jeremy Orlow 		fishd: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jeremy Orlow
Comment 1 2009-06-24 18:28:49 PDT
Created attachment 31823 [details] patch v1 Combined LocalStorageArea and SessionStorageArea into StorageArea since (after my other refactorings) there are no longer substantial differences between the two.
Jeremy Orlow
Comment 2 2009-06-24 18:31:17 PDT
Created attachment 31824 [details] patch v2 oops. Fix extra return in the change log.
Darin Fisher (:fishd, Google)
Comment 3 2009-06-24 23:17:09 PDT
Landed as http://trac.webkit.org/changeset/45144
Eric Seidel (no email)
Comment 4 2009-06-25 01:20:14 PDT
Reverted as: Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/GNUmakefile.am M WebCore/WebCore.vcproj/WebCore.vcproj M WebCore/WebCore.xcodeproj/project.pbxproj M WebCore/WebCoreSources.bkl M WebCore/storage/LocalStorage.cpp M WebCore/storage/LocalStorage.h M WebCore/storage/SessionStorage.cpp M WebCore/storage/SessionStorage.h M WebCore/storage/StorageArea.cpp M WebCore/storage/StorageArea.h M WebCore/storage/StorageAreaSync.cpp M WebCore/storage/StorageAreaSync.h M WebCore/storage/StorageSyncManager.h Committed r45162 Broke 18 layout tests.
Eric Seidel (no email)
Comment 5 2009-06-25 01:27:52 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog A WebCore/storage/LocalStorageArea.cpp A WebCore/storage/LocalStorageArea.h A WebCore/storage/SessionStorageArea.cpp A WebCore/storage/SessionStorageArea.h Committed r45165 http://trac.webkit.org/changeset/45165
Jeremy Orlow
Comment 6 2009-06-25 01:41:15 PDT
Must have forgotten to run the layout tests before submitting the patch. Sorry guys. :-( I think the change is a one liner. Will send a new patch tomorrow.
Jeremy Orlow
Comment 7 2009-06-25 12:09:32 PDT
Created attachment 31869 [details] patch v3 Rebuilt on HEAD and ran all layout tests again to make sure I don't break the build again. Only change is in StorageArea's constructor for SessionStorage. Instead of passing in a StorageArea pointer (that could be NULL dereferenced), I pass in a storageMap PassRefPtr (or 0 when it's not a clone).
Darin Fisher (:fishd, Google)
Comment 8 2009-06-25 12:53:24 PDT
Comment on attachment 31869 [details] patch v3 > Index: WebCore/storage/StorageArea.cpp ... > +PassRefPtr<StorageArea> StorageArea::createLocalStorage(SecurityOrigin* origin, PassRefPtr<StorageSyncManager> syncManager) ... > +StorageArea::StorageArea(SecurityOrigin* origin, PassRefPtr<StorageSyncManager> syncManager) ... > +PassRefPtr<StorageArea> StorageArea::createSessionStorage(SecurityOrigin* origin, Page* page) ... > +PassRefPtr<StorageArea> StorageArea::copy(SecurityOrigin* origin, Page* page) ... > +StorageArea::StorageArea(SecurityOrigin* origin, Page* page, PassRefPtr<StorageMap> storageMap) stylistic nit: it is usually good to make the order of the methods in the .cpp file match the order in which the methods are declared in the .h file. maybe you can fix this up in a follow up patch. still, r=me
Note You need to log in before you can comment on or make changes to this bug.