[Webkit-unassigned] [Bug 42827] Use the js-execute-active(entered) Frame to check the user gesture of page instead of checking the top frame

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42827





--- Comment #10 from Johnny Ding <jnd at chromium.org>  2010-07-26 02:08:03 PST ---
Thanks for review, Adam!

(In reply to comment #7)
> (From update of attachment 62371 [details])
> WebCore/bindings/js/ScriptController.cpp:306
>  +      return exec ? toDynamicFrame(exec) : 0;
> This is dangerous.  The lexical frame is most likely to be the active frame.  The dynamic frame isn't used for very much beyond user gesture.

I don't exactly know the difference between lexicalFrame and dynamicFrame. My only knowledge about those frame is from the comments in CallFrame.h(line 49-54).
According to the comments, the dynamicFrame is the frame in which execution began. lexicalFrame is  the frame in which current executing code was defined.
Differs from dynamicGlobalObject() during function calls across web browser frames. 
So if we define the window.open in the top frame and call the top frame's window.open in sub frame(iframe) by mouse click. The dynamicFrame seems like a right frame to test its user gesture status.
Also WebKit JS binding mostly use dynamicFrame to test the user gesture, please refer to JSDOMWindowCustom.cpp(line 520, 750, 764, 813), JSLocationCustom.cpp(line 194, 328).
also because JSDOMWindow::open test dynamicFrame's user gesture, my attached case can not affect WebKit.

> 
> WebCore/loader/FrameLoader.cpp:1133
>  +      Frame* enteredFrame = frame->script()->getActiveFrame();
> We don't use the term "enteredFrame" outside the V8 bindings.
Done.

> WebCore/loader/FrameLoader.cpp:1137
>  +      return enteredFrame->script()->processingUserGesture(mainThreadNormalWorld()); // FIXME: Use pageIsProcessingUserGesture.
> This is all way too complicated.  Every call site will need to do this dance.  Can't we push this junk into the bindings / ScriptController?

I see, yes, we should hide those details in bindings/ScriptController. Only question is whether it needs to be implemented inside ScriptController::processingUserGesture. According to Yury's change and his comments, seems like the processingUserGesture should be a property of the owning Page. How about adding a new method like activeFrameIsProcessingUserGesture and use it to test active frame's user gesture. Otherwise can we use ScriptController::anyPageIsProcessingUserGesture() to test whether any frame is prcessing user gsture.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.