Implement pan gesture to scroll page during touch point is moving on the page, and flick gesture to scroll page continuously and stop smoothly after pan gesture ends.
Created attachment 183136 [details] Patch
Created attachment 191934 [details] Patch Rebased.
Comment on attachment 191934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191934&action=review > Source/WebKit2/UIProcess/API/efl/EasingUtilities.cpp:2 > +/* > + * Copyright (C) 2001 Robert Penner All rights reserved. EasingUtilities? why not EasingCurves? > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:969 > + FloatPoint oldPagePosition = m_pagePosition; > + FloatPoint newPagePosition = boundContentsPosition(m_pagePosition + offset); why not just currentPosition and newPosition this could sounds as 'new page' position instead of new 'page position'
Comment on attachment 191934 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191934&action=review >> Source/WebKit2/UIProcess/API/efl/EasingUtilities.cpp:2 >> + * Copyright (C) 2001 Robert Penner All rights reserved. > > EasingUtilities? why not EasingCurves? I agree that EasingCurves is more accurate than EasingUtilities. I will change the name :) >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:969 >> + FloatPoint newPagePosition = boundContentsPosition(m_pagePosition + offset); > > why not just currentPosition and newPosition > > this could sounds as 'new page' position instead of new 'page position' Yes, it can be confused with new 'page position', I will change the name of variables.
Created attachment 193534 [details] Patch Rebase and Update for comments.
Created attachment 193780 [details] Patch Rebase.
Created attachment 206058 [details] Patch Rebased.
Created attachment 207620 [details] Patch Rebase.
Created attachment 207646 [details] Patch Modify scrollBy() to not use webview().
Created attachment 208330 [details] Patch Rebase.
Created attachment 208388 [details] Patch Rebase.
Created attachment 208389 [details] Patch Rebase.
Comment on attachment 208389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208389&action=review > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:160 > + HistoryItem item = {m_currentPoint, ecore_time_get()}; { m_currentPoint, ecore_time_get() }; ? > Source/WebKit2/UIProcess/efl/EasingCurves.cpp:2 > + * Copyright (C) 2001 Robert Penner All rights reserved. 2001 Robert Penner. ? Missing . after Robert Penner. > Source/WebKit2/UIProcess/efl/EasingCurves.h:33 > +float easeInOutQuad(float time, const float base, const float constant, const unsigned duration); Could you explain why you need to use this function ?
Comment on attachment 208389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208389&action=review >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:160 >> + HistoryItem item = {m_currentPoint, ecore_time_get()}; > > { m_currentPoint, ecore_time_get() }; ? Yes, I will fix. >> Source/WebKit2/UIProcess/efl/EasingCurves.cpp:2 >> + * Copyright (C) 2001 Robert Penner All rights reserved. > > 2001 Robert Penner. ? Missing . after Robert Penner. Thanks, I will add. >> Source/WebKit2/UIProcess/efl/EasingCurves.h:33 >> +float easeInOutQuad(float time, const float base, const float constant, const unsigned duration); > > Could you explain why you need to use this function ? The easeInOutQuad function is used for flick. I want to stop scrolling smoothly - that is flick, and flick with easeInOutQuad graph is looks good than linear graph, so I use that function.
Created attachment 209181 [details] Patch Update for comment.
Comment on attachment 209181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209181&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1407 > + return false; return (oldPosition != position); > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:79 > + typedef struct { why not just "struct HistoryItem"? > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:107 > + ecore_animator_del(m_panAnimator); looks like it would be nice to have OwnPtr<Ecore_Animator> > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:165 > + if (m_firstHistoryItemIndex == m_history.capacity()) how this m_history is supposed to work? from what I see we're always working with m_history[m_firstHistoryItemIndex] only, how other items are used?
Comment on attachment 209181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209181&action=review Thanks for comments. >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1407 >> + return false; > > return (oldPosition != position); That's simple. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:79 >> + typedef struct { > > why not just "struct HistoryItem"? OK, I will modify to "struct HistoryItem". >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:107 >> + ecore_animator_del(m_panAnimator); > > looks like it would be nice to have OwnPtr<Ecore_Animator> Yes, that's good idea. I will make OwnPtr<Ecore_Animator> as a separated patch. :) >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:165 >> + if (m_firstHistoryItemIndex == m_history.capacity()) > > how this m_history is supposed to work? from what I see we're always working with m_history[m_firstHistoryItemIndex] only, how other items are used? Yes, m_firstHistoryItemIndex is only used and others are not used. I want to get oldest item from the m_history (at most 5 points ago) and calculate difference between oldest item and current item, so m_history array is needed. When I started to implement flick, I thought that I could get more correct velocity if I use older point than point right before current point. I've tested using point right before current point after getting your comment and I could get the reasonable velocity, so I don't have to maintain the history array. I will modify codes for that.
Created attachment 209251 [details] Patch Update for comments.
Created attachment 209252 [details] Patch Remove s_historyCapacity.
Comment on attachment 209252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209252&action=review > Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp:117 > +void WebView::setContentPosition(const WebCore::FloatPoint& position) I think we need to get an agreement from coordinatedgraphics folks.
Comment on attachment 209181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209181&action=review >>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:107 >>> + ecore_animator_del(m_panAnimator); >> >> looks like it would be nice to have OwnPtr<Ecore_Animator> > > Yes, that's good idea. I will make OwnPtr<Ecore_Animator> as a separated patch. :) Dear Mikhail, I've found patch to stop using smart pointers for Ecore_Timer - https://bugs.webkit.org/show_bug.cgi?id=109409. Like Ecore_Timer, Ecore_Animator also can become invalid when callback returns ECORE_CALLBACK_CANCEL and crash will occur when OwnPtr calls ecore_animator_del() on an invalid handle. So, I decide to not make OwnPtr for Ecore_Animator.
Comment on attachment 209181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209181&action=review >>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:165 >>> + if (m_firstHistoryItemIndex == m_history.capacity()) >> >> how this m_history is supposed to work? from what I see we're always working with m_history[m_firstHistoryItemIndex] only, how other items are used? > > Yes, m_firstHistoryItemIndex is only used and others are not used. > I want to get oldest item from the m_history (at most 5 points ago) and calculate difference between oldest item and current item, so m_history array is needed. > > When I started to implement flick, I thought that I could get more correct velocity if I use older point than point right before current point. > I've tested using point right before current point after getting your comment and I could get the reasonable velocity, so I don't have to maintain the history array. > I will modify codes for that. Dear Mikhail, I'm sorry but I have to modify patch to use history again. I've tested on target device, the flick is sometimes too slow on target even though it is OK in the MiniBrowser. I think that's because target is slower than PC, so last two touched point can be got at a time and they are not reliable.
Created attachment 209325 [details] Patch Modify codes to use history again.
Comment on attachment 209252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209252&action=review >> Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp:117 >> +void WebView::setContentPosition(const WebCore::FloatPoint& position) > > I think we need to get an agreement from coordinatedgraphics folks. Dear Luciano Wolf and Marcelo Lira, Could you look at this change? I modified UIProcess/CoordinatedGraphics/WebView.cpp's setContentPosition() function in order to check boundary of position.
(In reply to comment #24) > (From update of attachment 209252 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209252&action=review > > >> Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp:117 > >> +void WebView::setContentPosition(const WebCore::FloatPoint& position) > > > > I think we need to get an agreement from coordinatedgraphics folks. > > Dear Luciano Wolf and Marcelo Lira, > Could you look at this change? > I modified UIProcess/CoordinatedGraphics/WebView.cpp's setContentPosition() function in order to check boundary of position. Hi. My concern is about panning. Our MiniBrowser allows the user to freely move and only applies the check for boundaries on handlePanningFinished. It relies on WKViewSetContentPosition to do the panning, so it would be restricted by boundaries check during the panning.
(In reply to comment #25) > (In reply to comment #24) > > (From update of attachment 209252 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=209252&action=review > > > > >> Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp:117 > > >> +void WebView::setContentPosition(const WebCore::FloatPoint& position) > > > > > > I think we need to get an agreement from coordinatedgraphics folks. > > > > Dear Luciano Wolf and Marcelo Lira, > > Could you look at this change? > > I modified UIProcess/CoordinatedGraphics/WebView.cpp's setContentPosition() function in order to check boundary of position. > > Hi. My concern is about panning. Our MiniBrowser allows the user to freely move and only applies the check for boundaries on handlePanningFinished. It relies on WKViewSetContentPosition to do the panning, so it would be restricted by boundaries check during the panning. Thanks for your answer :) I understand what you mean. In the EFL, I want to restrict boundary for panning so, it is better to move boundary checking codes to the EwkView, not WebView. I will update patch with that.
Created attachment 209443 [details] Patch Move boundary checking code to the EwkView from WebView.
Comment on attachment 209443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209443&action=review It looks this patch need to be done refactoring a little further. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:138 > + gestureHandler->m_ewkView->scrollBy(gestureHandler->m_lastPoint - gestureHandler->m_currentPoint); It would be good if you make view() to return m_ewkView. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:151 > + m_history.resize(0); Can't we reset these variables in handlePanFinished() ? I don't think we need to keep it until panning is started again. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:164 > + m_history[m_firstHistoryItemIndex++] = item; I wonder if we should use m_firstHistoryItemIndex memeber variable. Can't we simplify this as below ? if (m_history.size() < m_history.capacity()) m_history.append(item) else m_history.clear(); Then, find first item. const HistoryItem& firstHistoryItem = m_history[m_firstHistoryItemIndex]; => const HistoryItem& firstHistoryItem = m_history.first();
Comment on attachment 209443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209443&action=review Thanks for review. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:138 >> + gestureHandler->m_ewkView->scrollBy(gestureHandler->m_lastPoint - gestureHandler->m_currentPoint); > > It would be good if you make view() to return m_ewkView. OK, I will make that. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:151 >> + m_history.resize(0); > > Can't we reset these variables in handlePanFinished() ? I don't think we need to keep it until panning is started again. Yes, it can be moved to the PanFinished(). >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:164 >> + m_history[m_firstHistoryItemIndex++] = item; > > I wonder if we should use m_firstHistoryItemIndex memeber variable. Can't we simplify this as below ? > > if (m_history.size() < m_history.capacity()) > m_history.append(item) > else > m_history.clear(); > > Then, find first item. > > const HistoryItem& firstHistoryItem = m_history[m_firstHistoryItemIndex]; > => const HistoryItem& firstHistoryItem = m_history.first(); The m_firstHistoryItemIndex is needed because I want to get the oldest point in the history. That means, if the history grows as follows: [10] -> [10, 20] -> [10, 20, 30] -> [10, 20, 30, 40] -> [10, 20, 30, 40, 50]. Until this moment, the m_firstHistoryItemIndex is 0, and I can get "10". "60" is added to the history, the m_history can be [60, 20, 30, 40, 50], and m_firstHistoryItemIndex is 1 and the vaule is "20". I think the variable name "m_firstHistoryItemIndex" can be confused, so I will change that to "m_oldestHistoryItemIndex".
Created attachment 211409 [details] Patch Update for Gyuyoung's comment.
Comment on attachment 211409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211409&action=review > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1400 > + FloatPoint newPosition = FloatPoint(oldPosition.x + offset.width(), oldPosition.y + offset.height()); FloatPoint newPosition(oldPosition.x + offset.width(), oldPosition.y + offset.height()); This is not the same thing. > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1412 > + // If the page position is not changed, notify that using returning value. // If the page position has not changed, notify the caller using the return value. > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1413 > + return !(oldPosition == position); return oldPosition != position; > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:58 > + void handleFlick(const WebCore::IntSize&); useless WebCore:: > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:72 > + WebCore::IntPoint m_lastPoint; Useless WebCore:: > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:73 > + WebCore::IntPoint m_currentPoint; Ditto. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:75 > + Ecore_Animator* m_panAnimator; Would be nice to use WTF::OwnPtr for this (adding the right specialization if needed) > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:76 > + Ecore_Animator* m_flickAnimator; Ditto. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:77 > + WebCore::IntSize m_flickOffset; useless WebCore:: > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:82 > + WebCore::IntPoint point; useless WebCore:: > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:86 > + unsigned m_oldestHistoryItemIndex; size_t > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:98 > + m_history.reserveCapacity(s_historyCapacity); reserveInitialCapacity() > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:106 > +void GestureHandler::reset() Can be cleaned up once we use smart pointers > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:205 > + return false; Would be nicer to use the EINA equavalent in callback (e.g. *_RENEW) > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:208 > + return true; Ditto. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:214 > + m_flickIndex = m_flickDuration = 1 / ecore_animator_frametime_get(); m_flickIndex and m_flickDuration are integers, 1/ecore_animator_frametime_get() returns a floating point value. It is likely m_flickIndex and m_flickDuration are always 0, right?
(In reply to comment #31) > (From update of attachment 211409 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=211409&action=review > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1400 > > + FloatPoint newPosition = FloatPoint(oldPosition.x + offset.width(), oldPosition.y + offset.height()); > > FloatPoint newPosition(oldPosition.x + offset.width(), oldPosition.y + offset.height()); > > This is not the same thing. > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1412 > > + // If the page position is not changed, notify that using returning value. > > // If the page position has not changed, notify the caller using the return value. > > > Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1413 > > + return !(oldPosition == position); > > return oldPosition != position; > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:58 > > + void handleFlick(const WebCore::IntSize&); > > useless WebCore:: > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:72 > > + WebCore::IntPoint m_lastPoint; > > Useless WebCore:: > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:73 > > + WebCore::IntPoint m_currentPoint; > > Ditto. > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:75 > > + Ecore_Animator* m_panAnimator; > > Would be nice to use WTF::OwnPtr for this (adding the right specialization if needed) > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:76 > > + Ecore_Animator* m_flickAnimator; > > Ditto. > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:77 > > + WebCore::IntSize m_flickOffset; > > useless WebCore:: > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:82 > > + WebCore::IntPoint point; > > useless WebCore:: > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:86 > > + unsigned m_oldestHistoryItemIndex; > > size_t > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:98 > > + m_history.reserveCapacity(s_historyCapacity); > > reserveInitialCapacity() > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:106 > > +void GestureHandler::reset() > > Can be cleaned up once we use smart pointers > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:205 > > + return false; > > Would be nicer to use the EINA equavalent in callback (e.g. *_RENEW) > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:208 > > + return true; > > Ditto. > > > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:214 > > + m_flickIndex = m_flickDuration = 1 / ecore_animator_frametime_get(); > > m_flickIndex and m_flickDuration are integers, 1/ecore_animator_frametime_get() returns a floating point value. It is likely m_flickIndex and m_flickDuration are always 0, right? I published by mistake, sorry. I am still reviewing.
Comment on attachment 211409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211409&action=review >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:75 >> + Ecore_Animator* m_panAnimator; > > Would be nice to use WTF::OwnPtr for this (adding the right specialization if needed) Nevermind this comment. Since they get destroyed when the callback returns false, using smart pointers is bug prone. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:106 >> +void GestureHandler::reset() > > Can be cleaned up once we use smart pointers ignore. > Source/WebKit2/UIProcess/efl/EasingCurves.h:33 > +float easeInOutQuad(float time, const float base, const float constant, const unsigned duration); No namespace?
Comment on attachment 211409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211409&action=review Thanks for review :) >>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1400 >>> + FloatPoint newPosition = FloatPoint(oldPosition.x + offset.width(), oldPosition.y + offset.height()); >> >> FloatPoint newPosition(oldPosition.x + offset.width(), oldPosition.y + offset.height()); >> >> This is not the same thing. > > I published by mistake, sorry. I am still reviewing. Yes, right. I will modify the codes. >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1412 >> + // If the page position is not changed, notify that using returning value. > > // If the page position has not changed, notify the caller using the return value. Thanks for correction. >> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1413 >> + return !(oldPosition == position); > > return oldPosition != position; I use !(oldPosition == position) because there is no "!=" operator for WKPoint. Is it better to add "!=" operator to the WKPoint? >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:58 >> + void handleFlick(const WebCore::IntSize&); > > useless WebCore:: Yes, I will remove all "WebCore::". >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:86 >> + unsigned m_oldestHistoryItemIndex; > > size_t I will change that. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:98 >> + m_history.reserveCapacity(s_historyCapacity); > > reserveInitialCapacity() Thanks, I will use that. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:205 >> + return false; > > Would be nicer to use the EINA equavalent in callback (e.g. *_RENEW) Right. I will use ECORE_CALLBACK_{RENEW, CANCEL}. >> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:214 >> + m_flickIndex = m_flickDuration = 1 / ecore_animator_frametime_get(); > > m_flickIndex and m_flickDuration are integers, 1/ecore_animator_frametime_get() returns a floating point value. It is likely m_flickIndex and m_flickDuration are always 0, right? If the FPS is 60, the ecore_animator_frametime_get() returns 1/60 (= 0.016..), so I can get the FPS from "1/ecore_animator_frametime_get()" and convert that value to integer. >> Source/WebKit2/UIProcess/efl/EasingCurves.h:33 >> +float easeInOutQuad(float time, const float base, const float constant, const unsigned duration); > > No namespace? I need namespace WebKit :)
Created attachment 211524 [details] Patch Modify for Christophe's comments.
Comment on attachment 211409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211409&action=review >>> Source/WebKit2/UIProcess/API/efl/EwkView.cpp:1413 >>> + return !(oldPosition == position); >> >> return oldPosition != position; > > I use !(oldPosition == position) because there is no "!=" operator for WKPoint. > Is it better to add "!=" operator to the WKPoint? Ok, I thought those were FloatPoint. Nevermind then. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:189 > + m_history.resize(0); Shouldn't this be in the if()? If m_history.isEmpty() then this call is useless. >>> Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:214 >>> + m_flickIndex = m_flickDuration = 1 / ecore_animator_frametime_get(); >> >> m_flickIndex and m_flickDuration are integers, 1/ecore_animator_frametime_get() returns a floating point value. It is likely m_flickIndex and m_flickDuration are always 0, right? > > If the FPS is 60, the ecore_animator_frametime_get() returns 1/60 (= 0.016..), so I can get the FPS from "1/ecore_animator_frametime_get()" and convert that value to integer. Ok, thanks for clarifying.
Comment on attachment 211524 [details] Patch No big issue for me anymore but I'll let Gyuyoung review since he started.
Comment on attachment 211524 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211524&action=review It looks there is no big issue as well. > Source/WebKit2/UIProcess/API/efl/GestureRecognizer.cpp:181 > + const HistoryItem& firstHistoryItem = m_history[m_oldestHistoryItemIndex]; > Until this moment, the m_firstHistoryItemIndex is 0, and I can get "10". "60" is added to the history, the m_history can be [60, 20, 30, 40, 50], and m_firstHistoryItemIndex is 1 and the vaule is "20". Thank you for your explanation. Don't you need to change *firstHistoryItem* with *oldestHistoryItem* as well?
Christophe and Gyuyoung, thanks for review :) I will upload new patch after fixing as follows: 1. check history size before resize. 2. change name first -> oldest.
Created attachment 211751 [details] Patch Update for Christophe and Gyuyoung's comment.
Comment on attachment 211751 [details] Patch Clearing flags on attachment: 211751 Committed r155852: <http://trac.webkit.org/changeset/155852>
All reviewed patches have been landed. Closing bug.