Closed Bug 1273472 Opened 6 years ago Closed 5 years ago

[jsplugin] synchronously route events to JSPlugin

Categories

(Core Graveyard :: Plug-ins, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jj.evelyn, Assigned: jj.evelyn)

References

Details

Attachments

(4 files)

In NPAPI, the behavior is: when a Flash is playing, all keyboard events will be sent to flash without letting content know. I'm not sure if this is a bug or feature, but per our process model, events will be dispatch to jsplugin process directly. That means content has no chance to register itself on the event at capturing phase. Looks like we are aligned to this NPAPI behavior.
Component: General → Plug-ins
Product: Firefox → Core
Sorry I don't think this bugs is about plug-ins. My bug title might be a bit confusing.. :-/
Component: Plug-ins → General
Product: Core → Firefox
Evelyn, you should either put jsplugins bugs in Core:Plug-Ins or have a separate component for jsplugins.

*windowed* NPAPI captures keyboard events, but that's considered mostly a bug, not a feature. We should have plugins behave in normal DOM capture/bubble flow so that e.g. the Firefox frontend can capture Control-T and other hotkeys.
Component: General → Plug-ins
Product: Firefox → Core
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Evelyn, you should either put jsplugins bugs in Core:Plug-Ins or have a
> separate component for jsplugins.
> 

Okay, I will ask the team to categorize our bugs into a proper component.

> *windowed* NPAPI captures keyboard events, but that's considered mostly a
> bug, not a feature. We should have plugins behave in normal DOM
> capture/bubble flow so that e.g. the Firefox frontend can capture Control-T
> and other hotkeys.

Thanks for the input. Yes, I feel the same. However, directly dispatching events to the process of current focused element is a Gecko behavior in multi-process model, what jsplugin can do is forwarding events to content process via message manager and trigger another event dispatching route. It seems we can't perfectly follow normal DOM capture/bubble flow except modifying Gecko.
Please talk to overholt's DOM team about the event propagation issues. How we dispatch events seems like an important detail, and we're going to have to solve some of this for nested e10s iframes as well.
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> Please talk to overholt's DOM team about the event propagation issues.

Sure, will do.
Testing result of current implementation:
1. event will be dispatched to content process with target=<object>, experienced both capturing and bubbling phase;
2. at the same time, event will also be dispatched to jsplugin process with target=body, experienced both capturing and bubbling phase. If we call |event.stopPropagation();| here, it will stop ONLY in jsplugin process.
3. actually, event will be sent to jsplugin process twice. event.keyCode of printable character is 0 at the first time, while the second one's is correct.
* Control-T works.


Test result of Chrome:
Events will be dispatched to content. Control-T works too.
(In reply to Evelyn Hung [:evelyn] from comment #6)

> 3. actually, event will be sent to jsplugin process twice. event.keyCode of
> printable character is 0 at the first time, while the second one's is
> correct.

I couldn't reproduce this issue now. I should had uploaded my original test case here. :(
(In reply to Evelyn Hung [:evelyn] from comment #6)
> Testing result of current implementation:
> 1. event will be dispatched to content process with target=<object>,
> experienced both capturing and bubbling phase;
> 2. at the same time, event will also be dispatched to jsplugin process with
> target=body, experienced both capturing and bubbling phase. If we call
> |event.stopPropagation();| here, it will stop ONLY in jsplugin process.

Hi :smaug, the question here is about "event dispatching across nested iframes in multi-process model", and :peterv suggest you are the best go-to person. :-)

In our case, content of a <object> element is running in a remote process. The DOM structure is like this:
<window/document> (content)
-> <body>
   -> <object data="URL on different domain">
      -> <window/document> (remote)
         -> <body>
            -> <canvas>

When the <object> gets focus, every key stroke will trigger event dispatching to this remote process and content process at the same time, on different event chains. Per my observation above, event sent to content process has target=<object> element; while event sent to the remote process has target=<body>. Calling stopPropagation() in the content process at capturing phase won’t affect the event routing in the remote process, and vice versa.

I’m wondering if this is the expected behaviour, or it’s an unresolved issue in e10s model. Does the same-origin policy matter in this case?

Thanks. :)
Flags: needinfo?(bugs)
I'm not really familiar with this setup. What kind of thing has <canvas> in grandchild-process
(or sibling process of content process)?
Do you have a testcase for this?


But anyhow, stopPropagation() isn't suppose to prevent some default handler (which the plugin should be) to get the event. preventDefault() is for that.
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #9)
> I'm not really familiar with this setup. What kind of thing has <canvas> in
> grandchild-process
> (or sibling process of content process)?
> Do you have a testcase for this?
>

Please ignore <canvas> part, it doesn't really matter of the subject. (It is for rendering images that binary code sending to us)

The setup here is that we force the content loaded in <object> running in a remote process, which is a sibling process of the content process. The situation is similar to running an <iframe remote=true> in a webpage, although we don't officially support this for web content now. (we have a local Gecko patch to enable this setup for <object>.) The content loaded in <object> is a HTML in chrome:// protocol with some FrameScript injected. The content loaded in <object> is a jsplugin, behaves like a proxy to communicate with a *plugin binary*(e.g. Flash player) via Pepper API.

Since the magical part of loading jsplugin in a remote process isn't checked into Gecko yet, my test case might not be helpful. I will try to write a mochitest-chrome test to reproduce this issue by <iframe>.

> But anyhow, stopPropagation() isn't suppose to prevent some default handler
> (which the plugin should be) to get the event. preventDefault() is for that.

Cool, I tried preventDefault() in content process and it did stop event being dispatched to the plugin.  Thanks for the information.

I also noticed that calling preventDefault() in the remote process couldn't stop event routing in content process in bubbling phase. Therefore, the whole event routing flow isn't like what we know on the web - a child node can stop/prevent event being caught by its parent. Current behavior violates a rule defined by Pepper API: when a plugin registers for a specific event and deals with it, it can choose to *eat* that event so that the event won't be sent to embedder's handlers. So if the jsplugin(we are implementing) couldn't prevent events routing in the content at bubbling phase, it might break some use cases of plugins.
(In reply to Evelyn Hung [:evelyn] from comment #10)
> I also noticed that calling preventDefault() in the remote process couldn't
> stop event routing in content process in bubbling phase.


I don't understand this part. default handling happens _after_ capture/target/bubble phases, so preventDefault() shouldn't affect to those in anyway.



> Therefore, the
> whole event routing flow isn't like what we know on the web - a child node
> can stop/prevent event being caught by its parent.
Child node can prevent its ancestors to get the event in bubble phase using stopPropagation(), but that has very
little to do with preventDefault()

> Current behavior violates
> a rule defined by Pepper API: when a plugin registers for a specific event
> and deals with it, it can choose to *eat* that event so that the event won't
> be sent to embedder's handlers.
(1) Could you explain this a bit more. Do you mean Pepper API lets plugin to handle the event _before_ the web page?
If so, we need some way to send event first to the plugin process.

(2) Does Pepper API also require that in case web page can get access to the event, it can call preventDefault() to prevent the plugin to get the event? 

if (1) and (2), we need to implement something new: system process would get the event, then
if needed, dispatch to plugin process, then to child process and then again to plugin process, right?


> So if the jsplugin(we are implementing)
> couldn't prevent events routing in the content at bubbling phase, it might
> break some use cases of plugins.
This "bubbling" is confusing. default handling has nothing to do with bubble phase normally. Does Pepper somehow use bubble phase for default handlers?
I guess that jsplugin works as windowless plugins, right?

Even if so, at least "wheel" events shouldn't be sent to plugin as a part of event propagation. For preferring our mouse wheel transaction model, we send "wheel" events to windowless plugins as its default action...

If plugins eat "wheel" events as you said, scrolling its parent is *blocked* by a plugin if it's scrolled into mouse cursor position.
:smaug, thank you for keeping input here, I really appreciate. :-) 

(In reply to Olli Pettay [:smaug] from comment #11)
> (In reply to Evelyn Hung [:evelyn] from comment #10)
> > I also noticed that calling preventDefault() in the remote process couldn't
> > stop event routing in content process in bubbling phase.
> 
> 
> I don't understand this part. default handling happens _after_
> capture/target/bubble phases, so preventDefault() shouldn't affect to those
> in anyway.
> 

Okay, I guess I didn't get the idea of why it's preventDefault() instead of stopPropagation(). So we define "dispatching event to handlers of the embedded frame in the remote process" as a default action of browser, therefore it happens in the end of event routing process, right? I'm wondering why do we have this special design for remote iframe? or it's just for plugins?
 
> > Therefore, the
> > whole event routing flow isn't like what we know on the web - a child node
> > can stop/prevent event being caught by its parent.
> Child node can prevent its ancestors to get the event in bubble phase using
> stopPropagation(), but that has very
> little to do with preventDefault()
> 
> > Current behavior violates
> > a rule defined by Pepper API: when a plugin registers for a specific event
> > and deals with it, it can choose to *eat* that event so that the event won't
> > be sent to embedder's handlers.
> (1) Could you explain this a bit more. Do you mean Pepper API lets plugin to
> handle the event _before_ the web page?
> If so, we need some way to send event first to the plugin process.
> 

No. The order of event being caught by web pages and plugins isn't mentioned in the document. It doesn’t make sense to me that plugins get events first, before dispatching to web pages.

> (2) Does Pepper API also require that in case web page can get access to the
> event, it can call preventDefault() to prevent the plugin to get the event? 
>

Not described in Pepper API, but as I know, the browser wants to get the event and call preventDefault() for control-T "open a new tab" case (like comment 2 suggested).
 
> if (1) and (2), we need to implement something new: system process would get
> the event, then
> if needed, dispatch to plugin process, then to child process and then again
> to plugin process, right?
> 

So per its API document[1], Pepper defines two functions for event registration - RequestInputEvents and RequestFilteringInputEvents. Both are for plugins to register specific classes of events (e.g. mouse, keyboard, touch, …), so that it can receive the events afterwards. RequestFilteringInputEvents is for filtering events *synchronously* (requires the browser to stop and block for the plugin to handle the input event), while RequestInputEvents, the spec says "When requesting input events through this function, the events will be delivered and *not* bubbled to the default handlers." My wild guess of the "default handler" it means the handlers of the web page at bubbling phase.

> 
> > So if the jsplugin(we are implementing)
> > couldn't prevent events routing in the content at bubbling phase, it might
> > break some use cases of plugins.
> This "bubbling" is confusing. default handling has nothing to do with bubble
> phase normally. Does Pepper somehow use bubble phase for default handlers?

No, what I said was inferred from the words I quoted above.

[1] https://developer.chrome.com/native-client/pepper_dev/c/struct_p_p_b___input_event__1__0#a369d79730ad84d0b8dee9127c114086e
(In reply to Evelyn Hung [:evelyn] from comment #13)
> Okay, I guess I didn't get the idea of why it's preventDefault() instead of
> stopPropagation(). So we define "dispatching event to handlers of the
> embedded frame in the remote process" as a default action of browser,
> therefore it happens in the end of event routing process, right? I'm
> wondering why do we have this special design for remote iframe? or it's just
> for plugins?
How is that special? Default handling happens after normal event dispatch. If something else happens in some other case, that is a bug.

> 
> > (2) Does Pepper API also require that in case web page can get access to the
> > event, it can call preventDefault() to prevent the plugin to get the event? 
> >
> 
> Not described in Pepper API, but as I know, the browser wants to get the
> event and call preventDefault() for control-T "open a new tab" case (like
> comment 2 suggested).
Ok, browser (parent process) (or in case of single process, capturing phase) gets the event before
anything in the contentn 



> 
> So per its API document[1], Pepper defines two functions for event
> registration - RequestInputEvents and RequestFilteringInputEvents. Both are
> for plugins to register specific classes of events (e.g. mouse, keyboard,
> touch, …), so that it can receive the events afterwards.
> RequestFilteringInputEvents is for filtering events *synchronously*
> (requires the browser to stop and block for the plugin to handle the input
> event),
I don't quite understand what is synchronous her

> while RequestInputEvents, the spec says "When requesting input
> events through this function, the events will be delivered and *not* bubbled
> to the default handlers." My wild guess of the "default handler" it means
> the handlers of the web page at bubbling phase.
I wouldn't think so. I'd assume default handlers mean 'activation behavior' in HTML terms. 
Like <a> element has the activation behavior to trigger the link after bubble phase.
Sounds like that chrome documentation uses word 'bubble' for the activation behavior phase too, but that is
actually after the bubble phase of DOM events.

So in practice child process would call preventDefault() on these requested input events to prevent any default handling to happen in it.

I don't quite understand what RequestFilteringInputEvents are, so don't know what should happen there.
My guess is that it is like RequestInputEvents, but since it lets child process to default handle the events too, child process needs to be blocked while plugin process handles the event and then continue processing the event.
Something like
Child process
  capture phase - at_target - bubble phase - blocked - default handling
Plugin process                      \                  /
                                     handling the event   

This needs some testing.



In Gecko default handling (activation behavior) happens usually during PostHandleEvent phase.
But, there is also so called "system event group", to which chrome code can add listeners. Those are handled in separate phases after the normal DOM Event capture/at_target/bubble phase, which means that for web pages system group handling looks like default handling.
http://searchfox.org/mozilla-central/source/dom/events/EventDispatcher.cpp#339,349,373,385,403,410

I'm not familiar enough with plugin handling so I don't recall now whether windowless NPAPI plugins use system group listeners or what to get the events.
(In reply to Olli Pettay [:smaug] from comment #14) 
> > while RequestInputEvents, the spec says "When requesting input
> > events through this function, the events will be delivered and *not* bubbled
> > to the default handlers." My wild guess of the "default handler" it means
> > the handlers of the web page at bubbling phase.
> I wouldn't think so. I'd assume default handlers mean 'activation behavior'
> in HTML terms. 
> Like <a> element has the activation behavior to trigger the link after
> bubble phase.
> Sounds like that chrome documentation uses word 'bubble' for the activation
> behavior phase too, but that is
> actually after the bubble phase of DOM events.

Given we know the documentation is poor so I decided not to take its wording seriously. It doesn't describe how the plugin should get events at which phase, nor says how the plugin affects web page or browser's behavior. I guess the best way to understand its logic is running more tests on Chrome. I will try to do that and update here.

 
> In Gecko default handling (activation behavior) happens usually during
> PostHandleEvent phase.
> But, there is also so called "system event group", to which chrome code can
> add listeners. Those are handled in separate phases after the normal DOM
> Event capture/at_target/bubble phase, which means that for web pages system
> group handling looks like default handling.
> http://searchfox.org/mozilla-central/source/dom/events/EventDispatcher.
> cpp#339,349,373,385,403,410
>

Thank you for explaining me the "system event group" thing, I was confused when read the code. :)
I tried to make my flash sample and tested on Chrome. It seems there is no way for a swf file to prevent browser's behavior. Taking scrolling as an example, if there is a scrollable area in Flash, using the mousewheel inside the embedded Flash still scrolls the surrounding browser window (i.e. web page). Flash developers used to inject JavaScript code via ActionScript API to prevent outside web page scrolling, but it seems PPAPI implementation breaks this trick.[1] 

[1] http://www.spikything.com/blog/index.php/2009/11/27/stop-simultaneous-flash-browser-scrolling/
(In reply to Olli Pettay [:smaug] from comment #14) 
> I don't quite understand what RequestFilteringInputEvents are, so don't know
> what should happen there.
> My guess is that it is like RequestInputEvents, but since it lets child
> process to default handle the events too, child process needs to be blocked
> while plugin process handles the event and then continue processing the
> event.
> Something like
> Child process
>   capture phase - at_target - bubble phase - blocked - default handling
> Plugin process                      \                  /
>                                      handling the event   
> 
> This needs some testing.
> 

The flow makes sense to me, but I couldn't imagine how child process(web page) being blocked in this case. For example, if the plugin RequestFilteringInputEvents for wheel event, does it mean the web page should be blocked (no scrolling) and see if the content in the plugin wants to scroll? If so, that doesn't match my observation of Chrome in comment 16...

:smaug, have you seen any similiar cases before? or any use case you think of. After the discussion here, could we get a conclusion that our Gecko implementation meets our requirements?
Flags: needinfo?(bugs)
(In reply to Evelyn Hung [:evelyn] from comment #17)
> 
> The flow makes sense to me, but I couldn't imagine how child process(web
> page) being blocked in this case. For example, if the plugin
> RequestFilteringInputEvents for wheel event, does it mean the web page
> should be blocked (no scrolling) and see if the content in the plugin wants
> to scroll? If so, that doesn't match my observation of Chrome in comment
> 16...
Ok, then RequestFilteringInputEvents means something else than what I guess.

> :smaug, have you seen any similiar cases before? 
Similar to what?

> or any use case you think
> of. After the discussion here, could we get a conclusion that our Gecko
> implementation meets our requirements?
Not sure I understand this.


In which order do processes get the events? 
Does plugin process get the events before child process has done default handling?

What is the difference between RequestInputEvents and RequestFilteringInputEvents?
Flags: needinfo?(bugs)
And sorry about the delay. Ping me on IRC or send email if reply takes time.
(In reply to Olli Pettay [:smaug] from comment #11)

> (1) Do you mean Pepper API lets plugin to handle the event _before_ the web page?

As far as I understand, I think so. The Pepper API will return PP_TRUE or PP_FALSE to indicate that if they want web page to handle this event or not. 

> If so, we need some way to send event first to the plugin process.
> 
> (2) Does Pepper API also require that in case web page can get access to the
> event, it can call preventDefault() to prevent the plugin to get the event? 

Could you share in which case the web page could preventDefault to prevent the plugin to get the event if a plugin has focus. Sorry I am not sure I understand this?

> if (1) and (2), we need to implement something new: system process would get
> the event, then
> if needed, dispatch to plugin process, then to child process and then again
> to plugin process, right?

Why we dispatch events to plugin twice? I don't understand this.


So for PPAPI, there are only two APIs to register for events. The return value of this two will indicate if the web page should get access to the event or not.

I am not sure how events will be dispatched in gecko under jsplugin architecture. Below are our setup.

(1) The setup for jsplugin is like below

<window/document> (web page)
-> <body>
   -> <object data="URL on different domain">
      -> <window/document> (iframe)

(2) I hope this comment also helps.
https://bugzilla.mozilla.org/show_bug.cgi?id=558184#c96

I wonder under jsplugin setup, which process will get the events first? Is child process(web page) or the jsplugin process(remote iframe) inside the <object> tag?

Hey smaug, 
thank you very much! I know you are super busy so if you don't understand me in any part of this, please do ask.

Thanks!
Flags: needinfo?(bugs)
Assignee: nobody → ywu
I write a small case to test how <object> work with <iframe>. As far as I test, normally <iframe> can consume most of the events except "backspace" on OSX. Note: "backspace" can be consumed by <iframe> on linux. So I think why our js-plugin behaviors weird, for example, wheel event can be captured by web page, should relate to our setup. 

Since smaug is super busy, delay to ask him questions as we should figure out what's going on make us different from normal <iframe> first.

thank you smaug.
Flags: needinfo?(bugs)
What you mean with "Note: "backspace" can be consumed"?(In reply to Ya-Chieh Wu from comment #20)


> (In reply to Olli Pettay [:smaug] from comment #11)
> Could you share in which case the web page could preventDefault to prevent
> the plugin to get the event if a plugin has focus. Sorry I am not sure I
> understand this?
Web page has capturing listener for whatever event plugin uses.

> 
> > if (1) and (2), we need to implement something new: system process would get
> > the event, then
> > if needed, dispatch to plugin process, then to child process and then again
> > to plugin process, right?
> 
> Why we dispatch events to plugin twice? I don't understand this.
> 
Well, I don't yet understand at which point plugin is supposed to handle event.

As I asked before, 
What is the difference between RequestInputEvents and RequestFilteringInputEvents?

> 
> So for PPAPI, there are only two APIs to register for events. The return
> value of this two will indicate if the web page should get access to the
> event or not.
Ok, so event is dispatched to the plugin _before_ it is dispatched to the web page, right? Always? And web page calling stopPropagation() or preventDefault() doesn't affect to which events the plugin gets?
And does RequestInputEvents mean _all_ the events are dispatched to plugin only and 
RequestFilteringInputEvents means only some? And what happens to those events which aren't dispatched to the plugin?


> I wonder under jsplugin setup, which process will get the events first? Is
> child process(web page) or the jsplugin process(remote iframe) inside the
> <object> tag?
I'm not familiar with the nested process setup, but since normally events don't propagate through iframes or anything, I would assume first parent process gets the event and then plugin process.
(In reply to Olli Pettay [:smaug] from comment #22)

Thank you Smaug for taking your time to reply. I appreciate it.
 
> What you mean with "Note: "backspace" can be consumed"?(In reply to Ya-Chieh
> Wu from comment #20)
> 

We was suffering from events that are handling by web page. And one of the case is hitting "backspace" and the web page goes back to previous page. And I found out that on linux, "backspace" won't be handling by web page. But anyway, we need to solve the problem by making sure that our events won't be captured or won't be handled by web page.

> Well, I don't yet understand at which point plugin is supposed to handle
> event.
> 
> As I asked before, 
> What is the difference between RequestInputEvents and
> RequestFilteringInputEvents?
> 

So the difference between these two API is that RequestInputEvents will always return PP_TRUE. But RequestFilteringInputEvents might return PP_TRUE or PP_FALSE depends on if plugin handles the event or not. For implementation, if we get "RequestInputEvents events", we can immediately stop events forwarding to web page.  If we get "RequestFilteringInputEvents events", we wait for return value to know if we should forward the event to web page.

> > So for PPAPI, there are only two APIs to register for events. The return
> > value of this two will indicate if the web page should get access to the
> > event or not.
> Ok, so event is dispatched to the plugin _before_ it is dispatched to the
> web page, right? Always? And web page calling stopPropagation() or
> preventDefault() doesn't affect to which events the plugin gets?

PPAPI doesn't document this and thank you for reminding me this. So I think web page should be able to preventDefault() to prevent plugin gets events. But at the same time, I also need plugin to be able to indicate if web page should handle the event. So I guess Comment 14 is what we want.

> And does RequestInputEvents mean _all_ the events are dispatched to plugin
> only and RequestFilteringInputEvents means only some? And what happens to those
> events which aren't dispatched to the plugin?

The return value of RequestInputEvents will always be TRUE so web page shouldn't do it's default action. The default action here means the default action without plugin. For example, "wheel event" should scroll the page.  
RequestFilteringInputEvents means events need to be dispatched to plugin but plugin will return a value to indicate if web page should have the default action. 
For the events that are not registered by RequestInputEvents and RequestFilteringInputEvents, plugin won't do anything even if you pass the event to it.

 
I don't have any clue about how our events are routing right now. Could you share where in gecko we dispatch event to <iframe> and <object>?

Thank you very much, Smaug!
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #22)

> And does RequestInputEvents mean _all_ the events are dispatched to plugin only 
Maybe not "only", it depends on if we want web page to preventdefault to prevent plugin to get this events.
But if web page doesn't stop the event to pass to plugin, then web page shouldn't react to the event later.


> RequestFilteringInputEvents means only some? 
As RequestInputEvents, if web page doesn't stop the events to dispatch to plugin, the events should be pass to plugin too. But web page might or might not react to the event later. For example, if plugin returns PP_TRUE on "wheel" event, then only plugin scrolls. The web page should not scroll.

> And what happens to those
> events which aren't dispatched to the plugin?
It doesn't matter because we won't pass these event to plugin in <iframe>.

So actually, in our iframe, we will launch another process to run plugin. So all the implementation with plugin interface will be hided in our iframe process. That's why I didn't explain the difference between RequestInputEvents and RequestFilteringInputEvents at first.

I think our problem here is that our <iframe> can't stop event being forwarding or propagate to web page.
So our event routing must be different than the existing code which might because of our setup.
As far as I test, there are some things I would like to put down here.

under our jsplugin setup when the focus is inside the iframe(we create an iframe to render our plugin).

(1) events are handling first by web page. For example, web page will scroll first and then scrolling happens again inside the iframe.
(2) the target of events which are captured by web page is null.
Comment 25 is only a partial result. 
So I think I need to test all events that might be handled by jsplugin to make comprehensive conclusion. 

Below are the events that I will test

"keydown"
"keyup"
"keypress"
"wheel"
"mousedown",
"mouseup"
"mousemove"
"mouseenter"
"mouseleave"
"contextmenu"
(Whenever I'm slow at responding to needinfo's, please send email or ping on IRC. my needinfo queue is long, but I tend to process it after review and feedback queues.)
(In reply to Ya-Chieh Wu from comment #23)
> So the difference between these two API is that RequestInputEvents will
> always return PP_TRUE. But RequestFilteringInputEvents might return PP_TRUE
> or PP_FALSE depends on if plugin handles the event or not. For
> implementation, if we get "RequestInputEvents events", we can immediately
> stop events forwarding to web page.  If we get "RequestFilteringInputEvents
> events", we wait for return value to know if we should forward the event to
> web page.
I see. ok thanks.


> PPAPI doesn't document this and thank you for reminding me this. So I think
> web page should be able to preventDefault() to prevent plugin gets events.
> But at the same time, I also need plugin to be able to indicate if web page
> should handle the event. So I guess Comment 14 is what we want.
But I thought plugin gets the events before web page. And you said that 
'For implementation, if we get "RequestInputEvents events", we can immediately stop events forwarding to web page'

So, at which point does plugin handle the events? After web page? But web page gets the event only if 
RequestFilteringInputEvents returns PP_FALSE?
Are Request*InputEvents something which browser calls before each event dispatch or is it something which the plugin calls at any point and browser then caches the params somehow to tell how the events should be handled?


> The return value of RequestInputEvents will always be TRUE so web page
> shouldn't do it's default action.
And you mean that web page doesn't get the event at all?


> I don't have any clue about how our events are routing right now. Could you
> share where in gecko we dispatch event to <iframe> and <object>?

So normal iframe isn't really much different to top level web page.
[parentprocess] -> [childprocess]->widgetlevel->via view->presshell(this is where hittesting happens, and the right iframe is found->PreHandleEvent on EventStateManager-> DOM event dispatch -> PostHandleEvent on EventStateManager
Flags: needinfo?(bugs)
And for nested child processes, kanru would know better the event handling setup. I can't recall whether parent process sends the events via the main-child-process or does it send them straight to the right child process.
Flags: needinfo?(kchen)
(In reply to Ya-Chieh Wu from comment #25)
> As far as I test, there are some things I would like to put down here.
> 
> under our jsplugin setup when the focus is inside the iframe(we create an
> iframe to render our plugin).
> 
> (1) events are handling first by web page. For example, web page will scroll
> first and then scrolling happens again inside the iframe.
aha, this hints that the main-child-process does get the events first. Is this the case kanru?

> (2) the target of events which are captured by web page is null.
At which point event.target is null?
(In reply to Olli Pettay [:smaug] from comment #30)

> > (2) the target of events which are captured by web page is null.
> At which point event.target is null?

oops! I just realize that was a mistake. I was trying to put down the the target of the event is <object>. Sorry for that.
The events routing under our jsplugin setup right now.

jsplugin setup:
<window/document> (web page)
-> <body>
   -> <object data="URL on different domain">
      -> <window/document> (iframe)

(1) There will be two events. First event is inside the web page with the <object> as the target element. Second event is inside the iframe with the right target element. 
(2) If we preventDefault() the first event that we capture in web page, then second event will not be dispatched.
(3) Events are handling first by web page. For example, web page will scroll first and then scrolling happens again inside the iframe. 

(In reply to Olli Pettay [:smaug] from comment #28)
> So, at which point does plugin handle the events? After web page? But web page gets the event only if RequestFilteringInputEvents returns PP_FALSE?

Right now, our *plugin/our iframe* gets events after web page. And this two events are not the same. preventDefault the first event that capture in web page will cancel the the event that dispatch inside *plugin/our iframe*. And this behavior is not what PPAPI wants.

So I think *plugin/our iframe* needs to handle events by either two ways. (1) *plugin/our iframe* gets events before the web page and then by case to forward the event to web page or (2)  *plugin/our iframe* can be able to block web page to handle default action and by case to prevent web page's default action.
I don't know if any of the ways is possible. This is just my thoughts without checking if gecko is able to do that.
(In reply to Ya-Chieh Wu from comment #32)
> The events routing under our jsplugin setup right now.
> 
> jsplugin setup:
> <window/document> (web page)
> -> <body>
>    -> <object data="URL on different domain">
>       -> <window/document> (iframe)
> 
> (1) There will be two events. First event is inside the web page with the
> <object> as the target element. Second event is inside the iframe with the
> right target element. 
> (2) If we preventDefault() the first event that we capture in web page, then
> second event will not be dispatched.
> (3) Events are handling first by web page. For example, web page will scroll
> first and then scrolling happens again inside the iframe. 

That is very different what comment 20 said.
"> (1) Do you mean Pepper API lets plugin to handle the event _before_ the web page?
As far as I understand, I think so. The Pepper API will return PP_TRUE or PP_FALSE to indicate that if they want web page to handle this event or not. "


I'd like to understand who calls Request*InputEvents and when. And after that where are the events dispatched and when:
are there _always_ two events dispatched - one to the web page and one to the plugin. And if event is dispatched to the web page, does preventDefault() affect to the event dispatched to the plugin?


> Right now, our *plugin/our iframe* gets events after web page. And this two
> events are not the same. preventDefault the first event that capture in web
> page will cancel the the event that dispatch inside *plugin/our iframe*. And
> this behavior is not what PPAPI wants.
Aah, you described above our current behavior.
So what is the behavior PPAPI wants?


> So I think *plugin/our iframe* needs to handle events by either two ways.
> (1) *plugin/our iframe* gets events before the web page and then by case to
> forward the event to web page or (2)  *plugin/our iframe* can be able to
> block web page to handle default action and by case to prevent web page's
> default action.
So which one? What happens in Chrome? Does the web page always get the event, which would be (2), even though
default action would be prevented?

> I don't know if any of the ways is possible. This is just my thoughts
> without checking if gecko is able to do that.
So you need to ask kanru about the current sibling process setup (or test and look at the code :) I would need to do that to figure this out). We could certainly make it so that inner process gets the event first, but less clear is whether we can or want to block the web page process while plugin process deals with the event.
(In reply to Olli Pettay [:smaug] from comment #30)
> (In reply to Ya-Chieh Wu from comment #25)
> > As far as I test, there are some things I would like to put down here.
> > 
> > under our jsplugin setup when the focus is inside the iframe(we create an
> > iframe to render our plugin).
> > 
> > (1) events are handling first by web page. For example, web page will scroll
> > first and then scrolling happens again inside the iframe.
> aha, this hints that the main-child-process does get the events first. Is
> this the case kanru?

If the event dispatch is going through the main process event dispatch then it will be handled by each of the child processes through the widgetlevel->via view->presshell->HandleCrossProcessEvent on EventStateManager route. However if APZ is enabled and APZ can find a child process through its layerId, APZ should be able to dispatch the event to the child process directly no matter it's main-child-process or a nested-child-process. This is implemented in bug 1020199

Regarding our APZ setup, kats should be more familiar. Beware that PAPZ has recently been moved to the compositor process/thread.
Flags: needinfo?(kchen)
Hey Peter, 

could you take a loot at Comment 32.
Summary: Dispatch event to <embed> container at capturing phase → [jsplugin] event propagation route
Hey peter, 

Sorry for hitting "Enter" when I was changing the Summary of this bug which cause I submitted the partial sentences on Comment 35.

Comment 32 has the summary of what we are finding under jsplugin setup now. I wonder if you have any feedback on this because this will relate to the "gecko patch". And also do I need to wait for the "gecko patch" to be landed? Is it possible that "gecko patch" will also take care of event propagation? 

thank you so much for your time, peter.
Flags: needinfo?(peterv)
(In reply to Olli Pettay [:smaug] from comment #33)
> (In reply to Ya-Chieh Wu from comment #32)
> > Right now, our *plugin/our iframe* gets events after web page. And this two
> > events are not the same. preventDefault the first event that capture in web
> > page will cancel the the event that dispatch inside *plugin/our iframe*. And
> > this behavior is not what PPAPI wants.
> Aah, you described above our current behavior.
> So what is the behavior PPAPI wants?
> 

Actually PPAPI doesn't document which process(content process or plugin process) should get the events first. It only documents that two of the APIs(RequestFilteringInputEvents and RequestInputEvents)'s return value will indicate if web page should handle the event or not.


> > So I think *plugin/our iframe* needs to handle events by either two ways.
> > (1) *plugin/our iframe* gets events before the web page and then by case to
> > forward the event to web page or (2)  *plugin/our iframe* can be able to
> > block web page to handle default action and by case to prevent web page's
> > default action.
> So which one? What happens in Chrome? Does the web page always get the
> event, which would be (2), even though
> default action would be prevented?

These two are just my thoughts that the event propagation should act. Because under our jsplugin setup *right now*, events are dispatching like what Comment 32 describes. As Comment 32, there will be two separate events under our jsplugin setup *right now* so plugin can't stop web page to handle events which is not PPAPI wants. 

Chrome acts like (2) because web page is able to preventDefault the events to dispatch to plugin.

> 
> > I don't know if any of the ways is possible. This is just my thoughts
> > without checking if gecko is able to do that.
> So you need to ask kanru about the current sibling process setup (or test
> and look at the code :) I would need to do that to figure this out). We
> could certainly make it so that inner process gets the event first, but less
> clear is whether we can or want to block the web page process while plugin
> process deals with the event.

Thanks smaug. I think I already ask enough questions. I should look into code directly :)
Since Evelyn would like to take over, reassign this bug to her. And clear ni from me for peter because this bug is Evelyn's now.
Assignee: ywu → ehung
Flags: needinfo?(peterv)
Thank ya-chieh's help for digesting issues here. We came up with a more clear problem statement with two use cases as following. (Let's focus on PDF cases now, as Flash is loaded in the same setup and gets the same problem.)

(1) When typing in a PDF form, hitting backspace key leads browser going back to previous page;

(2) When navigating in a PDF form, hitting tab key leads global focus moving to address bar, i.e. PDF content loses focus; (I will attach a screenshot for this)

Both cases are because browser's default actions are unexpectedly triggered. Unlike a normal webpage, the browser knows nothing about the content and the status inside a PDF. We keep dispatching keyboard events (and other input events) into the plugin, so it can act accordingly[*]. No matter it registers the input events via RequestFilteringInputEvents or RequestInputEvents Pepper API, there are cases like (1) and (2) that the plugin handled the input events and it doesn't want to trigger browser's default action.[**]

* Note that the form inside the PDF is *virtual to us*. It looks like a form but it's actually just pixels painting on a <canvas>. We rely on PDFium controls its internal status correctly and render the pixel result into a shared memory buffer, and then we paint the result on the <canvas> we prepared.

** The only difference is that by registering RequestInputEvents, the plugin by default handles the input event, while by RequestFilteringInputEvents it will response every time when it gets an event. For case (1) and (2), the PDFiume registers all keyboard events via RequestInputEvents.

=== Proposal ===

It seems the event dispatching flow in comment 14 as :smaug described earlier could solve the problems above. We can do it in DOM event level since the plugin is wrapped as a jsplugin which is loaded in a remote iframe.

Child process
  capture phase - at_target - bubble phase - blocked - default handling
Jsplugin process                   \                  /
                                     handling the event   


@smaug, do you think we should go for implementing this event route? (and sorry for being back and forth on the discussion because we were not really getting into the core of this problem before.) Thank you for your input. :-)
Flags: needinfo?(bugs)
Does Chrome (a) send the event to the child/web page process at all? Or does it (b) just send it to plugin process?

if (a), then I guess we have to block child process and get it handled in plugin process and get back the defaultPrevented value so that default handling can be prevented if needed.

If (b), this would become easier, since browser's default handling wouldn't really happen at all, expect the stuff in the chrome process.
Flags: needinfo?(bugs)
Chrome does (a), so a web page can call preventDefault() to prevent events being dispatched to plugins (like what we have now).
I found bug 1128771 might be related.
After tracing event dispatching flow in Gecko, I'm thinking to set |*aStatus = nsEventStatus_eConsumeNoDefault| when dispatching events to "JSPlugin process" in EventStateManager::DispatchCrossProcessEvent().

That means most of the time if the event is dispatched to JSPlugin process, we can assume plugin will handle it and no default actions are taken by browser. But this doesn't solve the case that plugin might want to trigger browser's default actions via using |RequestFilteringInputEvents| and replying false when it gets events. Fortunately, we only see this Pepper API usage in Flash case for wheel events. We may be able to treat it as a special case and design a solution separately. 

Hi smaug and masayuki, what do you think?  I'm not sure if it's a good idea but it solves backspace key problem #1 described in comment 39. 

For problem #2, tab key case, I found we can simply call event.preventDefault() in our jsplugin process, so we won't go further in EventStateManager::PostHandleKeyboardEvent() and bypass focus manager.[1]


[1] http://searchfox.org/mozilla-central/source/dom/events/EventStateManager.cpp#2844-2847
Flags: needinfo?(masayuki)
Flags: needinfo?(bugs)
(In reply to Evelyn Hung [:evelyn] from comment #43)
> After tracing event dispatching flow in Gecko, I'm thinking to set |*aStatus
> = nsEventStatus_eConsumeNoDefault| when dispatching events to "JSPlugin
> process" in EventStateManager::DispatchCrossProcessEvent().
> 
> That means most of the time if the event is dispatched to JSPlugin process,
> we can assume plugin will handle it and no default actions are taken by
> browser. But this doesn't solve the case that plugin might want to trigger
> browser's default actions via using |RequestFilteringInputEvents| and
> replying false when it gets events. Fortunately, we only see this Pepper API
> usage in Flash case for wheel events. We may be able to treat it as a
> special case and design a solution separately. 

Basically, yes. When the event will be handled asynchronously, we need to treat each event as "consumed". After that, the remote process should request to perform the default action if it wants.

Although, I'm not sure which method should use with AZP...

If it's not in AZP, you should run ESM::PostHandleEvent()'s this case:
https://dxr.mozilla.org/mozilla-central/rev/e3279760cd977aac30bd9e8032d3ee71f55d2a67/dom/events/EventStateManager.cpp#3180

> Hi smaug and masayuki, what do you think?  I'm not sure if it's a good idea
> but it solves backspace key problem #1 described in comment 39. 

I think that it's right approach.

> For problem #2, tab key case, I found we can simply call
> event.preventDefault() in our jsplugin process, so we won't go further in
> EventStateManager::PostHandleKeyboardEvent() and bypass focus manager.[1]
> 
> 
> [1]
> http://searchfox.org/mozilla-central/source/dom/events/EventStateManager.
> cpp#2844-2847

Yes, that should work. However, PPAPI doesn't allow to move focus between web contents and plugins? If so, always calling preventDefault() is fine, otherwise, shouldn't call preventDefault() when:
* tab key without shift state and the last widget in plugin has focus
* tab key with shift state and the first widget in plugin has focus
Flags: needinfo?(masayuki)
So how does Chrome deal with default handling? Does it (1) always prevent it when ppapi is handling an event? Does it (2) run it synchronously? (3) asynchronously?

If (1), this all is simpler and the plan sounds good.
If (2), we actually need to block child process and send some sync message from child to parent which then sends a message to plugin process and once the plugin process has processed the event, it sends message back to parent which notifies then child process. (this would be pretty unfortunate option)
if (3), we need to redispatch the event in chrome process but with mNoCrossProcessBoundaryForwarding or something like that. Also, should we move dispatching to plugin process before chrome has done all its default handling?

I assume it has been tested how different events are handled (like in case of <a href="http://mozilla.org">foo <object/> bar</a>).
Would be useful to have the results documented somewhere. In which case does Chrome allow plugin to prevent default handling and how, sync or async
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #45)

> If (2), we actually need to block child process and send some sync message
> from child to parent which then sends a message to plugin process and once
> the plugin process has processed the event, it sends message back to parent
> which notifies then child process. (this would be pretty unfortunate option)
(need to spin even loop in parent process to get messages from plugin process)
Thanks, masayuki and smaug!

First off, I was wrong by saying wheel event for Flash is the only case using |RequestFilteringInputEvents|, sorry!! I found that in PDF cases, mouse, keyboard and touch events are all requested by |RequestFilteringInputEvents| which means plugins may not handle the events - we need its response to proceed browser's default action. 

So per Chrome's behavior and Pepper API's (poor) document, we need to *synchronously* block browser until getting a response from plugin.


(In reply to Olli Pettay [:smaug] from comment #45)
> So how does Chrome deal with default handling? Does it (1) always prevent it
> when ppapi is handling an event? Does it (2) run it synchronously? (3)
> asynchronously?

So to be clear, ppapi's |RequestInputEvents| follows (1) and |RequestFilteringInputEvents| follows (2). We unfortunately have both ppapi use case in practice.

> If (1), this all is simpler and the plan sounds good.
> If (2), we actually need to block child process and send some sync message
> from child to parent which then sends a message to plugin process and once
> the plugin process has processed the event, it sends message back to parent
> which notifies then child process. (this would be pretty unfortunate option)
> if (3), we need to redispatch the event in chrome process but with
> mNoCrossProcessBoundaryForwarding or something like that. Also, should we
> move dispatching to plugin process before chrome has done all its default
> handling?

I'm thinking to combine solution (2) and (3) for not blocking parent process, but haven't had a clear idea. Will look more code and update later.

Thanks again!
Also leave a note here. Events requested by flash player and pdfium:

Flash:
- RequestInputEvents: mouse, keyboard, IME
- RequestFilteringInputEvents: wheel

PDF:
- RequestInputEvents: nope!
- RequestFilteringInputEvents: mouse, keyboard, touch
I found mWantReplyFromContentProcess and SendReplyKeyEvent/RecvReplyKeyEvent mechanism works in our setup between child process and jsplugin process. It happens like this:

   parent               child             jsplugin
 --->|                    |                   |
     |  SendRealKeyEvent  |                   |
     |------------------->|                   |
     |                    |                   |
     |  SendReplyKeyEvent | SendRealKeyEvent  |
     |<-------------------|------------------>|
(trigger default handle)  |                   |
     |                    | SendReplyKeyEvent |
     |                    |<------------------|


The replied event isn't further dispatched to parent(chrome) process where the browser default action takes place. I also suspect the asynchronous communication may have events out-of-order problem(* see my question below), and it's even harder to maintain their order across three processes.

To follow the current event dispatching flow, I'm thinking to send sync message (or rpc call) between child and jsplugin processes, before SendReplyKeyEvent to parent. That means keeping parent-child IPC asynchronously as it is now, but delay the replying event to chrome so the default actions take place later. What do you think?


* Question: How do we prevent events being out of order in this async process communication? I was guessing the following steps might cause events/commands in wrong order:
 - select some text on a web page
 - press ctrl-c for copy
 - run a 5-second for-loop in content's keypress event handler
 - in the meantime, unselect the text (but it won't take effect immediately because content UI thread is blocked)

If mouse event arrives at content side before the copy command (which is caused by the replied keypress event being processed in chrome process), the text is firstly unseleced and leads nothing to copy.

However, I couldn't reproduce this issue.
Flags: needinfo?(bugs)
I should have had some update here.

I would like to focus on PDF's case first so according to comment 48, mouse, keyboard, and touch events should be addressed first. I checked mouse events and it seems there is no big difference from keyboard event in terms of event routing. I haven't had an developing environment ready for tracing touch event which I do think things will be more complicated there.

I'm now working on a patch for sending synchronous message from TabParent to TabChild. I plan to add a "child:" section in PBrowser.ipdl and define the method with "nested(inside_sync) sync" keywords. Will update my result here.
Sorry, late ni? reply. (better to ping me on IRC or send email if ni? replies are late).

nested(inside_sync) sync? Hmm, is that only for sending from content process to plugin process?
We must not support sync parent process -> child process communication.
You will want feedback from IPC peers.
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #51)
> Sorry, late ni? reply. (better to ping me on IRC or send email if ni?
> replies are late).
> 

Will do that when I need more timely reply. Thanks!

> nested(inside_sync) sync? Hmm, is that only for sending from content process
> to plugin process?
> We must not support sync parent process -> child process communication.
> You will want feedback from IPC peers.

Yes, I'm thinking to only allow child -> jsplugin process doing sync communication.

:smaug, could I have your feedback to comment 49 as well? Thanks!
Flags: needinfo?(bugs)
"That means keeping parent-child IPC asynchronously as it is now, but delay the replying event to chrome so the default actions take place later. What do you think?"

Sounds reasonable, since send->reply is already async from parent point of view.
Flags: needinfo?(bugs)
In this WIP patch I simply add two synchronous methods for sending keyboard and mouse events to JSPluign, and glue the rest of the code together. I tested with our JSPlugin setup, it works good. However, for the first mousedown event case, the plugin claim that it handles the event, so we should prevent default. but if we do that, then the default focus movement won't happen and plugin won't get focus, which lead keyboard event won't target on the plugin.

I'm thinking to NOT do anything for these events except keyboard events, until we see a real problem. (In this WIP, I just commented out preventDefault logic and keep sync IPC part for mouse event)

:smaug and :masayuki, may I have your early feedback to the patch? I know it is quit hacky, so if you think we need a small refactor to make it better, I will try to do that. Thanks!

p.s. |IsJSPlugin()| is a ContentParent method for detecting the remote frame is loaded for JSPlugin content. This part is still in :peterv's WIP and hasn't landed to m-c yet.
Attachment #8816005 - Flags: feedback?(masayuki)
Attachment #8816005 - Flags: feedback?(bugs)
Attachment #8816005 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 8816005 [details]
Synchronous event routing to JSPlugin from content process

Could you perhaps explain the event flow with this patch.
First parent gets the events, sends to child, right? and then it goes to JSPlugin, and this part is rpc-call effectively?
>   switch (aEvent->mClass) {
>   case eMouseEventClass: {
>-    return remote->SendRealMouseEvent(*aEvent->AsMouseEvent());
>+    if (remote->IsJSPlugin()) {
>+      bool handled = false;
>+      bool retval = remote->SendRealMouseEvent(*aEvent->AsMouseEvent(), &handled);
>+      printf_stderr("/////// [JSPlugin] mouse handled? %d \n", handled);
>+      /*if (retval && handled) {
>+        *aStatus = nsEventStatus_eConsumeNoDefault;
>+        aEvent->PreventDefault();
>+      }*/
>+      return retval;
>+    } else {
>+      return remote->SendRealMouseEvent(*aEvent->AsMouseEvent());
>+    }
Why two different SendRealMouseEvent calls. Makes the code a bit hard to follow




>   case eKeyboardEventClass: {
>-    return remote->SendRealKeyEvent(*aEvent->AsKeyboardEvent());
>+    if (remote->IsJSPlugin()) {
>+      bool handled = false;
>+      bool retval = remote->SendJSPluginKeyEvent(*aEvent->AsKeyboardEvent(), &handled);
>+      printf_stderr("/////// [JSPlugin] keyboard handled? %d \n", handled);
>+      if (retval && handled) {
>+        *aStatus = nsEventStatus_eConsumeNoDefault;
>+        aEvent->PreventDefault();
>+      }
>+      return retval;
>+    } else {
>+      return remote->SendRealKeyEvent(*aEvent->AsKeyboardEvent());
>+    }
Could you perhaps refactor the code here so that SendJSPluginKeyEvent is hidden inside TabParent.
And even aEvent->PreventDefault() and *aStatus setting could happen there. Same with mouse events

> mozilla::ipc::IPCResult
>+TabChild::RecvJSPluginKeyEvent(const WidgetKeyboardEvent& aEvent, bool* result)
nit aResult. Same also elsewhere. Arguments are in form aName.

>+{
>+  WidgetKeyboardEvent localEvent(aEvent);
>+  localEvent.mWidget = mPuppetWidget;
>+  nsEventStatus status = APZCCallbackHelper::DispatchWidgetEvent(localEvent);
>+
>+  printf_stderr("/////// [JSPlugin] keyboard mDefaultPrevented? %d \n", localEvent.mFlags.mDefaultPrevented);
>+  *result = localEvent.mFlags.mDefaultPrevented;
>+  return IPC_OK();
>+}
It would be great if RecvRealKeyEvent and this method could use the same code. Some part of it would be just no-op in JSPlugins

>+
>+mozilla::ipc::IPCResult
>+TabChild::RecvJSPluginMouseEvent(const WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId, bool* result)
Similar things with this method and RectRealMouse*Events


Assuming I understand this correctly, looking pretty much what I expected.
Attachment #8816005 - Flags: feedback?(bugs) → feedback+
Hi :billm,

As I talked to you in person, my patch is on comment 54. The proposal of sending sync message from Content->JSPlugin was described in comment 49, and :peterv's patch of the whole JSPlugin IPC foundation is on bug 1291539 comment 7. Here I will also upload the diagrams I shown you to recall your memory. :)
Flags: needinfo?(wmccloskey)
Comment on attachment 8816005 [details]
Synchronous event routing to JSPlugin from content process

Sorry for the delay to check this.

I regret to say, I don't agree with this approach because if I understand your approach correctly, your approach allows plugin can block its parent process (i.e., content process). As you know, content process manages other websites too. So, busy plugin process may make content process slowdown or hang up.

Looks like the reason why you want to make them sync is for receiving the result of the events. I have no idea why you need to receive mouse event result since mouse events fired in plugin won't be needed by anybody. So, it seems that you can make it async simply.

On the other hand, you actually need to receive the result of keyboard events in the plugin. However, this is possible without sync message.

I'm working on bug 1257617 which is caused by similar issue. When a process needs reply from its remote process, the process should not fire keyboard events before sending keyboard events to the remote process. Instead, after receiving the reply, TabParent needs to dispatch keyboard events in its process and call ESM::PostHandleEvent().

If you have another work, I recommend you to wait the fix of bug 1257617, although, I still need a couple of weeks before review (could be early of Jan, 2017 if I'd receive urgent bugs).

If my understanding is wrong, let me know.
Attachment #8816005 - Flags: feedback?(masayuki) → feedback-
Evelyn and I discussed this yesterday.

Then, we decided that we should be back here after bug 1257617. Then, we can use async methods for getting reply for keyboard events on JSPlugin.
Depends on: 1257617
Hmm, async? I thought Pepper API required rpc model (similar what we have for some plugins stuff already).

how would the async setup work here?
The problem of the WIP patch is, content process sends keyboard events and mouse events to JSPlugin process synchronously. This means that if the JSPlugin process is already hangs, too busy or event handler does something very long time (e.g., creating big listbox synchronously), JSPlugin process blocks the content process.  Then, some other tabs which are managed by the content process won't work until the JSPlugin returns reply for the synchronous communication.

My idea is, content process should post keyboard events and mouse events to JSPlugin asynchronously. When a posting keyboard event needs reply to chrome process, it should be sent to chrome process when it's back from JSPlugin process.
JSPlugin process is "Plugin UI Process" in attachment 8817249 [details]. I don't think that we need to use async between "Plugin UI Process" and "Plugin Binary Process".
Yes, I know there is the problem that content process might hang, but at least it isn't the parent process. But I just thought that Pepper plugins require the rpc model, so that default handling in the content process might be prevented.
If Chrome has that model, we should too. And if not, then we shouldn't either :)
> so that default handling in the content process might be prevented.

It's not problem. My idea is, sending keyboard event after dispatching it in content process's tree and wait performing default until the reply comes back from the JSPlugin.

After fixing bug 1257617, keyboard events should be handled as following:

1. chrome process dispatches keyboard events to PresShell
2. chrome process sends keyboard events to remote process if it needs reply or there are pending keyboard events which were posted for receiving reply.
3. chrome process discards the keyboard event.
4. content process dispatches the keyboard event in its DOM tree.
5. content process sends the keyboard event to JSPlugin process as a part of default action (only when it's default isn't prevented).
6. content process discards the keyboard event.
7. JSPlugin handles it and send reply to content process.
8. content process may do default action if it's not prevented default.
9. content process sends the keyboard event to the chrome process.
10. chrome process may do default action if it's not prevented default.

So, discarding keyboard events at #3 and #6 is the point.

Are there something problem you found? If so, I might need to redesign the patches for bug 1257617.
Ah, chrome process should dispatch keyboard events into its DOM tree at #10 and then, do the default action if it's not prevented default.
So, events are fired in following order:

1. in content process
2. in JSPlugin process
3. in chrome process

Giving priority of default action is:

1. JSPlugin process
2. content process
3. chrome process
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #66)
> > so that default handling in the content process might be prevented.
> 
> It's not problem. My idea is, sending keyboard event after dispatching it in
> content process's tree and wait performing default until the reply comes
> back from the JSPlugin.
Well, how do you wait? What guarantees focus isn't moved elsewhere or mouse events aren't handled etc?
If there isn't sync processing, the "world" in the content process may look totally different.


> 1. chrome process dispatches keyboard events to PresShell
> 2. chrome process sends keyboard events to remote process if it needs reply
> or there are pending keyboard events which were posted for receiving reply.
> 3. chrome process discards the keyboard event.
> 4. content process dispatches the keyboard event in its DOM tree.
> 5. content process sends the keyboard event to JSPlugin process as a part of
> default action (only when it's default isn't prevented).
> 6. content process discards the keyboard event.
> 7. JSPlugin handles it and send reply to content process.
> 8. content process may do default action if it's not prevented default.
> 9. content process sends the keyboard event to the chrome process.
> 10. chrome process may do default action if it's not prevented default.

So steps 6-8 look scary from web compatibility point of view. Does Chrome have that model that event loop may spin while waiting for plugin to handle the event? If not, we can't have it either.
(In reply to Olli Pettay [:smaug] from comment #69)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #66)
> > > so that default handling in the content process might be prevented.
> > 
> > It's not problem. My idea is, sending keyboard event after dispatching it in
> > content process's tree and wait performing default until the reply comes
> > back from the JSPlugin.
> Well, how do you wait?

I think that TabParent::RecvReplyKeyEvent() should dispatch keyboard events into the tree in the process and/or call EventStateManager::PostHandleEvent().

> What guarantees focus isn't moved elsewhere or mouse
> events aren't handled etc?

Oh, good point. I think that we need some mechanism to decide if reply is outdated. I still don't have the idea of the design about this issue. But basically, it shouldn't be major problem. It should occur at the remote process is busy.

> If there isn't sync processing, the "world" in the content process may look
> totally different.

Yes, it's possible. However, we've already taken similar issues at e10s. In e10s mode, shourtcut key may kick shortcut key handlers with different "world" from it when user pressed the key actually.

> > 1. chrome process dispatches keyboard events to PresShell
> > 2. chrome process sends keyboard events to remote process if it needs reply
> > or there are pending keyboard events which were posted for receiving reply.
> > 3. chrome process discards the keyboard event.
> > 4. content process dispatches the keyboard event in its DOM tree.
> > 5. content process sends the keyboard event to JSPlugin process as a part of
> > default action (only when it's default isn't prevented).
> > 6. content process discards the keyboard event.
> > 7. JSPlugin handles it and send reply to content process.
> > 8. content process may do default action if it's not prevented default.
> > 9. content process sends the keyboard event to the chrome process.
> > 10. chrome process may do default action if it's not prevented default.
> 
> So steps 6-8 look scary from web compatibility point of view.

Do you have some idea about realistic scenario? I understand that for example, Ctrl+C may copy different selection, etc. However, I don't have any idea which is too critical.

> Does Chrome
> have that model that event loop may spin while waiting for plugin to handle
> the event? If not, we can't have it either.

I guess, it's no. I think that one of their most important things is quickness of response time at user operation.

In Google Chrome, even if plugin hangs up, the other tab's content must not be blocked by the plugin process. However, our e10s isn't designed so. Although, if JSPlugin would never be loaded into content process which manages other tabs, using sync message would not be so bad idea. I believe that the new stability issue caused by new sync message will break our result of e10s and OOPP.

I've not understood the design of JSPlugin yet. So, if user's input won't be sent to JSPlugin process with sync message when plugin process is too busy, it's okay, but I guess it's not so.
e10s's parent process is very different case. We certainly don't want to block parent process, but there we don't need to care about web compatibility since parent process isn't exposed to the web. 
So I don't think we should necessarily take the similar approach here, since content process is all about the web.

Does Chrome really not block other tabs when plugin is processing events (talking about the case when other tabs are in the same process). That would be rather surprising.

We will have multi-e10s soon, which will help block in many cases, and while doing preemption stuff to Quantum DOM, we can think about better ways too.
But for now, I think the most important part is to be compatible with Chrome's event handling model, given that we're going to reuse the plugins they have.
Reading the Chromium docs, it seems like they do block when dispatching filtered input events:
https://chromium.googlesource.com/chromium/src/+blame/master/ppapi/cpp/instance.h#394

I think this is how it's actually implemented:
https://chromium.googlesource.com/chromium/src/+blame/master/ppapi/proxy/ppapi_messages.h#735
When an input event comes in, a sync message is sent to the plugin. However, I can't figure out what happens if the plugin tries to send a message back while it's handling an input event. It looks like there are quite a few sync messages, and they can go in both directions. Perhaps these messages go to different threads? I don't see how else they can avoid deadlock. The Chromium IPC docs don't say anything about re-entry or RPC.

I agree with smaug that this should be implemented synchronously. Even if it weren't necessary for PPAPI, we would like to be able to use the same mechanism for nested content processes, and I don't see how we could avoid blocking there. I don't think a nested event loop would work, since we would still process other events for the same page. It doesn't seem very different from what Masayuki is proposing.

For full-blown nested iframes, I think we'll have to do a lot of refactoring to figure out how to avoid deadlocks. For PDFium, I suspect that Evelyn's approach will probably work okay. I'm less certain about Flash, but I guess we can try it out and see what happens.

What's scary about Evelyn's patch is that the sync IPC message can get "canceled" if the PPAPI process sends up a sync message or runs a nested event loop. In that case, the Send call will just return false. We should probably crash in that case so we know when it happens.
Flags: needinfo?(wmccloskey)
(In reply to Olli Pettay [:smaug] from comment #71)
> Does Chrome really not block other tabs when plugin is processing events
> (talking about the case when other tabs are in the same process). That would
> be rather surprising.

Yeah, but in my understanding, Chrome creates new process for new tab which doesn't have opener.

> We will have multi-e10s soon, which will help block in many cases, and while
> doing preemption stuff to Quantum DOM, we can think about better ways too.
> But for now, I think the most important part is to be compatible with
> Chrome's event handling model, given that we're going to reuse the plugins
> they have.

Okay, then, let's go ahead.

BTW, when we release this in current schedule? I'm not clear what stage of this project we are in.
Group: mozilla-employee-confidential
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #73)
> 
> BTW, when we release this in current schedule? I'm not clear what stage of
> this project we are in.

The target release is around 2017 mid on Nightly. This is an estimation based on our engineering progress so far, not a hard ETA.
Thanks for everyone's input here. I really appreciate. Let me sum up here to make sure I get it correctly.

Let's keep the sync message way between Content process and JSPlugin process, because 1) Pepper API spec said so and it seems Chrome implemented in this way too;
2) To avoid web compatibility issue that might happen if we try async way or use nested event loop.
3) multi-e10s and Quantum DOM will make the blocking-other-tabs situation better since we will have more processes for tabs and preemption mechanism will process higher priority events first, and probably not get blocked.  

I also think maybe we could let tabs with <embed> elements (for PDF/Flash) running in a process if possible, so they won't interfere other tabs. That could ease Masayuki's worry of blocking other pages.
(In reply to Evelyn Hung [:evelyn] from comment #75) 
> I also think maybe we could let tabs with <embed> elements (for PDF/Flash)
> running in a process if possible, ..

... I meant isolating them from other normal tabs. Not sure if it's easy because process creation will depend on content. :-/
(In reply to Bill McCloskey (:billm) from comment #72)
> What's scary about Evelyn's patch is that the sync IPC message can get
> "canceled" if the PPAPI process sends up a sync message or runs a nested
> event loop. In that case, the Send call will just return false. We should
> probably crash in that case so we know when it happens.

Thanks for the feedback! The sync IPC message is sent to Plugin UI process which is running JSPlugin code we implemented (and then we use another rpc message to communicate with PPAPI plugin binary process), so we could avoid sending sync message or running nested event loop. I will try to handle this case if I could detect it happens.
Blocks: 1264551
Duplicate of this bug: 1299752
Summary: [jsplugin] event propagation route → [jsplugin] synchronously route events to JSPlugin
Duplicate of this bug: 1273811
Priority: -- → P3
No longer depends on: 1257617
We have other plan for Flash, so these bugs become won't fix.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.