| Summary: | Remove ScrollAnimator::create factory function | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||
| Component: | WebCore Misc. | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||||
| Status: | NEW --- | ||||||||||||||
| Severity: | Normal | CC: | darin, fred.wang | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Gyuyoung Kim
2014-10-21 18:16:02 PDT
Created attachment 240234 [details]
Patch
Comment on attachment 240234 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240234&action=review > Source/WebCore/platform/ScrollableArea.cpp:76 > + m_scrollAnimator = std::make_unique<ScrollAnimator>(const_cast<ScrollableArea*>(this)); This doesn’t do the same thing the old code did. Why is it OK to change behavior and make an object of a different class than before in the case of Mac, iOS, and when scrollAnimatorEnabled is false? Comment on attachment 240234 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240234&action=review >> Source/WebCore/platform/ScrollableArea.cpp:76 >> + m_scrollAnimator = std::make_unique<ScrollAnimator>(const_cast<ScrollableArea*>(this)); > > This doesn’t do the same thing the old code did. Why is it OK to change behavior and make an object of a different class than before in the case of Mac, iOS, and when scrollAnimatorEnabled is false? Oops, I really really missed it ! Exactly right. Let me fix this patch again !. Created attachment 240326 [details]
Patch
Comment on attachment 240326 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240326&action=review > Source/WebCore/platform/ScrollableArea.cpp:80 > +#if PLATFORM(IOS) I don't like to create new instance based on #if ~ #endif though, I don't know better solution except for create() factory function. Is there better approach ? Created attachment 240332 [details]
Patch for ews
Created attachment 240338 [details]
Patch
Created attachment 240342 [details]
Patch
Comment on attachment 240342 [details]
Patch
Darin, what do you think about this change ?
CC'ing Darin. I wonder what do you think about this patch. Comment on attachment 240342 [details] Patch Need to be modified after bug 137765 |