WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
127276
Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
https://bugs.webkit.org/show_bug.cgi?id=127276
Summary
Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
Zan Dobersek
Reported
2014-01-20 02:28:56 PST
Move TreeScope, IdTargetObserverRegistry to std::unique_ptr
Attachments
Patch
(8.41 KB, patch)
2014-01-20 02:31 PST
,
Zan Dobersek
no flags
Details
Formatted Diff
Diff
Patch
(8.37 KB, patch)
2014-01-20 22:34 PST
,
Zan Dobersek
kling
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Zan Dobersek
Comment 1
2014-01-20 02:31:22 PST
Created
attachment 221639
[details]
Patch
Zan Dobersek
Comment 2
2014-01-20 22:34:55 PST
Created
attachment 221721
[details]
Patch
Brent Fulgham
Comment 3
2014-01-21 12:08:08 PST
Comment on
attachment 221721
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=221721&action=review
This looks great! I would prefer we not use the 'get()' method in the shouldCacheLabelsByForAttribute. Can you confirm it must be implemented the way you have written that?
> Source/WebCore/dom/TreeScope.h:83 > + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); }
std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)".
Anders Carlsson
Comment 4
2014-01-21 12:38:46 PST
(In reply to
comment #3
)
> (From update of
attachment 221721
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=221721&action=review
> > This looks great! I would prefer we not use the 'get()' method in the shouldCacheLabelsByForAttribute. Can you confirm it must be implemented the way you have written that? > > > Source/WebCore/dom/TreeScope.h:83 > > + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } > > std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)".
the bool operator is explicit. I think .get() is better than an explicit static cast.
Andreas Kling
Comment 5
2014-02-06 12:57:31 PST
Comment on
attachment 221721
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=221721&action=review
r=me
>>> Source/WebCore/dom/TreeScope.h:83 >>> + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } >> >> std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)". > > the bool operator is explicit. I think .get() is better than an explicit static cast.
How about "return !!m_labelsByForAttribute;"
Zan Dobersek
Comment 6
2014-02-08 11:58:30 PST
Comment on
attachment 221721
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=221721&action=review
>>>> Source/WebCore/dom/TreeScope.h:83 >>>> + bool shouldCacheLabelsByForAttribute() const { return m_labelsByForAttribute.get(); } >>> >>> std::unique_ptr has an operator (bool). Why is the 'get()' necessary here? At the worst, you should be doing something like "static_cast<bool>(m_labelsByForAttribute)". >> >> the bool operator is explicit. I think .get() is better than an explicit static cast. > > How about "return !!m_labelsByForAttribute;"
Sure.
Zan Dobersek
Comment 7
2014-02-08 12:05:19 PST
Committed
r163719
: <
http://trac.webkit.org/changeset/163719
>
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