SSIA
Created attachment 240647 [details] Patch
Created attachment 240719 [details] Patch
Created attachment 240724 [details] Patch
Comment on attachment 240724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240724&action=review > Source/WebCore/platform/graphics/GraphicsLayer.h:91 > + FloatAnimationValue(double keyTime, float value, TimingFunction* timingFunction = nullptr) > + : AnimationValue(keyTime, timingFunction) > + , m_value(value) > { > - return adoptPtr(new FloatAnimationValue(keyTime, value, timingFunction)); > } Doesn't this change expose the copy constructor as a public member function?
(In reply to comment #4) > Comment on attachment 240724 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240724&action=review > > > Source/WebCore/platform/graphics/GraphicsLayer.h:91 > > + FloatAnimationValue(double keyTime, float value, TimingFunction* timingFunction = nullptr) > > + : AnimationValue(keyTime, timingFunction) > > + , m_value(value) > > { > > - return adoptPtr(new FloatAnimationValue(keyTime, value, timingFunction)); > > } > > Doesn't this change expose the copy constructor as a public member function? When I try to create FloatAnimationValue through copy constructor, there is no build error. If you have concern about it, could you let me know it ?
(In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 240724 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=240724&action=review > > > > > Source/WebCore/platform/graphics/GraphicsLayer.h:91 > > > + FloatAnimationValue(double keyTime, float value, TimingFunction* timingFunction = nullptr) > > > + : AnimationValue(keyTime, timingFunction) > > > + , m_value(value) > > > { > > > - return adoptPtr(new FloatAnimationValue(keyTime, value, timingFunction)); > > > } > > > > Doesn't this change expose the copy constructor as a public member function? > > > When I try to create FloatAnimationValue through copy constructor, there is > no build error. I succeed to create FloatAnimationValue object using copy constructor as below, FloatAnimationValue* fValue = new FloatAnimationValue(key, keyframeStyle->opacity(), tf); FloatAnimationValue* copyValue(fValue); It looks the change can support copy constructor.
Created attachment 241266 [details] Rebased
Comment on attachment 241266 [details] Rebased View in context: https://bugs.webkit.org/attachment.cgi?id=241266&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:160 > + std::unique_ptr<AnimationValue> intialValue = std::make_unique<FloatAnimationValue>(0, 1); > + fadeKeyframes.insert(WTF::move(intialValue)); I think this would read better without a local variable: fadeKeyframes.insert(std::make_unique<FloatAnimationValue>(0, 1)); > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:163 > + std::unique_ptr<AnimationValue> finalValue = std::make_unique<FloatAnimationValue>(0.25, 0); > + fadeKeyframes.insert(WTF::move(finalValue)); I think this would read better without a local variable: fadeKeyframes.insert(std::make_unique<FloatAnimationValue>(0.25, 0));
Created attachment 241275 [details] Patch for landing
(In reply to comment #8) > Comment on attachment 241266 [details] > Rebased > > View in context: > https://bugs.webkit.org/attachment.cgi?id=241266&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:160 > > + std::unique_ptr<AnimationValue> intialValue = std::make_unique<FloatAnimationValue>(0, 1); > > + fadeKeyframes.insert(WTF::move(intialValue)); > > I think this would read better without a local variable: > > fadeKeyframes.insert(std::make_unique<FloatAnimationValue>(0, 1)); > > > Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp:163 > > + std::unique_ptr<AnimationValue> finalValue = std::make_unique<FloatAnimationValue>(0.25, 0); > > + fadeKeyframes.insert(WTF::move(finalValue)); > > I think this would read better without a local variable: > > fadeKeyframes.insert(std::make_unique<FloatAnimationValue>(0.25, 0)); Thank you for review ! Fixed all.
Comment on attachment 241275 [details] Patch for landing Clearing flags on attachment: 241275 Committed r175799: <http://trac.webkit.org/changeset/175799>
All reviewed patches have been landed. Closing bug.