Summary: | Make StyleRareNonInheritedData::mask and StyleBackgroundData::background DataRefs | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
Component: | CSS | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | changseok, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, koivisto, kondapallykalyan, macpherson, menard, pdr, sabouhallawa, simon.fraser, thorton, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Wenson Hsieh
2020-08-28 12:14:10 PDT
Created attachment 407496 [details]
For EWS
Created attachment 407497 [details]
For EWS (and fix GTK build)
Comment on attachment 407497 [details] For EWS (and fix GTK build) View in context: https://bugs.webkit.org/attachment.cgi?id=407497&action=review > Source/WebCore/rendering/style/FillLayer.cpp:30 > +struct SameSizeAsFillLayer : public RefCounted<SameSizeAsFillLayer> { No need for "public" here. > Source/WebCore/rendering/style/FillLayer.cpp:31 > + RefPtr<FillLayer> next; SameSize structures don’t really need to use smart pointers instead of raw pointers. > Source/WebCore/rendering/style/FillLayer.cpp:110 > + m_next = FillLayer::create(*o.m_next); Should just call create here, not FillLayer::create. > Source/WebCore/rendering/style/FillLayer.h:74 > + static Ref<FillLayer> create(FillLayerType type) > + { > + return adoptRef(*new FillLayer(type)); > + } > + > + static Ref<FillLayer> create(const FillLayer& layer) > + { > + return adoptRef(*new FillLayer(layer)); > + } I suggest not inlining these. We’d rather inline the constructor inside the create function rather than inlining the allocation and then paying function call overhead to call the constructor. > Source/WebCore/rendering/style/FillLayer.h:79 > + Ref<FillLayer> copy() const > + { > + return FillLayer::create(*this); > + } Should call just "create" and should write as a one-liner: Ref<FillLayer> copy() const { return create(*this); } Comment on attachment 407497 [details] For EWS (and fix GTK build) View in context: https://bugs.webkit.org/attachment.cgi?id=407497&action=review Thanks for the review! >> Source/WebCore/rendering/style/FillLayer.cpp:30 >> +struct SameSizeAsFillLayer : public RefCounted<SameSizeAsFillLayer> { > > No need for "public" here. Removed the public! >> Source/WebCore/rendering/style/FillLayer.cpp:31 >> + RefPtr<FillLayer> next; > > SameSize structures don’t really need to use smart pointers instead of raw pointers. Changed back to a raw pointer. >> Source/WebCore/rendering/style/FillLayer.cpp:110 >> + m_next = FillLayer::create(*o.m_next); > > Should just call create here, not FillLayer::create. Removed the FillLayer:: >> Source/WebCore/rendering/style/FillLayer.h:74 >> + } > > I suggest not inlining these. We’d rather inline the constructor inside the create function rather than inlining the allocation and then paying function call overhead to call the constructor. Sounds good — moved the definition out of the header. >> Source/WebCore/rendering/style/FillLayer.h:79 >> + } > > Should call just "create" and should write as a one-liner: > > Ref<FillLayer> copy() const { return create(*this); } Done! Created attachment 407586 [details]
Patch for landing
Committed r266344: <https://trac.webkit.org/changeset/266344> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407586 [details]. |