WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
100316
[V8] We can merge DOMDataStore, ScopedDOMDataStore, and StaticDOMDataStore into one class
https://bugs.webkit.org/show_bug.cgi?id=100316
Summary
[V8] We can merge DOMDataStore, ScopedDOMDataStore, and StaticDOMDataStore in...
Adam Barth
Reported
2012-10-24 18:24:09 PDT
[V8] We can merge DOMDataStore, ScopedDOMDataStore, and StaticDOMDataStore into one class
Attachments
Work in progress
(25.11 KB, patch)
2012-10-24 18:24 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(26.50 KB, patch)
2012-10-24 22:40 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch for landing
(25.68 KB, patch)
2012-10-24 23:41 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Adam Barth
Comment 1
2012-10-24 18:24:27 PDT
Created
attachment 170531
[details]
Work in progress
Adam Barth
Comment 2
2012-10-24 18:24:51 PDT
This currently crashes every worker test, but I'm hoping to improve that soon.
Adam Barth
Comment 3
2012-10-24 22:40:07 PDT
Created
attachment 170552
[details]
Patch
Eric Seidel (no email)
Comment 4
2012-10-24 22:57:37 PDT
Comment on
attachment 170552
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=170552&action=review
LGTM.
> Source/WebCore/ChangeLog:11 > + unifies all these classes into DOMDataStore itself. This makese this
makese!
> Source/WebCore/bindings/v8/DOMDataStore.cpp:57 > + m_domObjectMap = adoptPtr(new DOMWrapperHashMap<void>); > + m_activeDomObjectMap = adoptPtr(new DOMWrapperHashMap<void>);
These are always the same for all 3, no? It seems this whole "Switch" is clearer as an if. But presumably you wanted the "forgot to add handling of the new enum value" help from the compiler?
Adam Barth
Comment 5
2012-10-24 23:06:14 PDT
(In reply to
comment #4
)
> (From update of
attachment 170552
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=170552&action=review
> > > Source/WebCore/bindings/v8/DOMDataStore.cpp:57 > > + m_domObjectMap = adoptPtr(new DOMWrapperHashMap<void>); > > + m_activeDomObjectMap = adoptPtr(new DOMWrapperHashMap<void>); > > These are always the same for all 3, no? It seems this whole "Switch" is clearer as an if. But presumably you wanted the "forgot to add handling of the new enum value" help from the compiler?
Yeah, I thought they were going to be more different. I'll change it to an if.
Adam Barth
Comment 6
2012-10-24 23:41:16 PDT
Created
attachment 170563
[details]
Patch for landing
WebKit Review Bot
Comment 7
2012-10-25 00:14:36 PDT
Comment on
attachment 170563
[details]
Patch for landing Clearing flags on attachment: 170563 Committed
r132454
: <
http://trac.webkit.org/changeset/132454
>
WebKit Review Bot
Comment 8
2012-10-25 00:14:40 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug