Bug 27180 – Cleanup DOM Storage dependencies.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jeremy Orlow]

DOM Storage has some really bad dependencies.  Most of these are simply because of destructors or constructors.  There's also a lot of file included that could otherwise just be declared.  Clean this stuff up.

This will allow things like StorageAreaSync to take a StorageAreaImpl* (as it should) rather than a StorageArea* which previously weren't possible because the dependencies were such a tangled mess.

[Jeremy Orlow]

Created attachment 32617 [details]
patch 1

[Darin Adler]

Comment on attachment 32617 [details]
patch 1

r=me

Having the reference counter be a friend class and a private destructor does seem slightly tighter than our usual reference counting idiom, which is good.

If it was me I wouldn't call these "really bad dependencies" or say "such a tangled mess" -- I would intend say something factual about how many extra includes there are and let the reader decide how bad it was.

[Jeremy Orlow]

Created attachment 32625 [details]
rev 2

Cleaned up the ChangeLog wording.

Ready to be landed, I think.

[David Kilzer (:ddkilzer)]

Comment on attachment 32625 [details]
rev 2

Yes, this may be landed.  Technically, the lander should use Darin Adler's r+, not mine.   I'm just confirming the ChangeLog update. :)

[Darin Adler]

Comment on attachment 32625 [details]
rev 2

I'll set the review flag again just for clarity.

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/45864.

[Brent Fulgham]

This patch broke the Windows build due to Visual Studio complaints about the template friend declaration.

Reverted in http://trac.webkit.org/changeset/45866.

I'm attaching the error log for review.

[Brent Fulgham]

Created attachment 32732 [details]
Windows build failure output.

[David Kilzer (:ddkilzer)]

Comment on attachment 32732 [details]
Windows build failure output.

See also:

http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/2762/steps/compile-webkit/logs/stdio/text

[Brent Fulgham]

Marking patches r- to remove from the application queue.

[Jeremy Orlow]

Created attachment 32759 [details]
rev 3

Removed the fancy private destructor stuff.  It's not that important.

[Jeremy Orlow]

I guess my original comment was overly terse.

I'm very sorry the first patch had issues building on visual studio.  This time I built it on windows and on mac, and everything seemed good.

The only thing that changed is that I moved all the destructors back to being public and took out the friend statements.

This patch should be ready to land.  (Yes, famous last words.  :-)

[Darin Fisher (:fishd, Google)]

Comment on attachment 32759 [details]
rev 3

r=me

[Darin Fisher (:fishd, Google)]

Landed as http://trac.webkit.org/changeset/45954