[webkit-reviews] review denied: [Bug 44969] Track the right gesture state during the asynchronous form submission. : [Attachment 66211] fix patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Johnny Ding <jnd at chromium.org>'s
request for review:
Bug 44969: Track the right gesture state during the asynchronous form
submission.
https://bugs.webkit.org/show_bug.cgi?id=44969

Attachment 66211: fix patch
https://bugs.webkit.org/attachment.cgi?id=66211&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66211&action=prettypatch

Mostly just questions.

> WebCore/ChangeLog:9
> +	   fast/events/popup-blocked-to-post-blank.html can cover the test in
WebKit.
> +	   A UI test will be added in chromium to address chromium's bug.
Is fast/events/popup-blocked-to-post-blank.html test currently failing?  If so,
do we need to remove it from the Skipped list?	If not, then it doesn't seem to
cover this change.

> WebCore/loader/RedirectScheduler.cpp:188
> +	   // Please do not delete the following local variable
gestureIndicator definition.
> +	   // The variable gestureIndicator toggles a static gesture state to
let WebKit correctly
> +	   // track the user gesture state across async operations.
This comment is not needed.  Instead, we should have a test that breaks if you
remove this line.

> WebCore/loader/RedirectScheduler.cpp:324
>      bool lockBackForwardList = mustLockBackForwardList(m_frame,
UserGestureIndicator::processingUserGesture()) ||
(submission->state()->formSubmissionTrigger() == SubmittedByJavaScript &&
m_frame->tree()->parent());
>  
> -    schedule(adoptPtr(new ScheduledFormSubmission(submission,
lockBackForwardList, duringLoad)));
> +    schedule(adoptPtr(new ScheduledFormSubmission(submission,
lockBackForwardList, duringLoad,
m_frame->loader()->isProcessingUserGesture())));
Why do we use  m_frame->loader()->isProcessingUserGesture() here but
UserGestureIndicator::processingUserGesture() two lines above?