Bug 26698 - Combined LocalStorageArea and SessionStorageArea into StorageArea
Summary: Combined LocalStorageArea and SessionStorageArea into StorageArea
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks: 25376
  Show dependency treegraph
 
Reported: 2009-06-24 17:34 PDT by Jeremy Orlow
Modified: 2009-06-25 14:33 PDT (History)
9 users (show)

See Also:


Attachments
patch v1 (39.54 KB, patch)
2009-06-24 18:28 PDT, Jeremy Orlow
no flags Details | Formatted Diff | Diff
patch v2 (39.37 KB, patch)
2009-06-24 18:31 PDT, Jeremy Orlow
fishd: review+
Details | Formatted Diff | Diff
patch v3 (39.47 KB, patch)
2009-06-25 12:09 PDT, Jeremy Orlow
fishd: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 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.
Comment 1 Jeremy Orlow 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.
Comment 2 Jeremy Orlow 2009-06-24 18:31:17 PDT
Created attachment 31824 [details]
patch v2

oops.  Fix extra return in the change log.
Comment 3 Darin Fisher (:fishd, Google) 2009-06-24 23:17:09 PDT
Landed as http://trac.webkit.org/changeset/45144
Comment 4 Eric Seidel (no email) 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.
Comment 5 Eric Seidel (no email) 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
Comment 6 Jeremy Orlow 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.
Comment 7 Jeremy Orlow 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).
Comment 8 Darin Fisher (:fishd, Google) 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