[System] Improvement of InputMethod Inter-Process Communication.

NEW
Unassigned

Status

Firefox OS
General
2 years ago
6 months ago

People

(Reporter: mantaroh, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Created attachment 8610351 [details]
IMG_2047.mp4

InputMethod API is communicating with 3 process.
1) keyboard app process (mozKeyboard.js)
2) chrome process (Keyboard.jsm)
3) content app process (form.js)

Those process is connected using MessageManager.
However, It is not need chrome on some event.

Example: SendKey Event is
traveling around three process.
mozKeyboard.js  -> Keyboard.jsm  -> forms.js -> Keyboard.jsm ->
mozKeyboard.js

Attachment movie is communicating content process and Keyboard process directly using BroadcastChannelAPI.
Detail:
 - Modified BroadcastChannel in order to can communicate cross-origin.
 - Added heavy debug code in order to reproduce heavy chrome process.
 - Modified forms.js and MozKeyboard.js in order to communicate directly using BroadcastChannel.

First, I will implement fake implementation for discuss to use BroadcastChannel or not.
There are several way to communicate 
 1) Inter App Communication(IAC)
 2) MessageManager (CPMM / PPMM)
 3) BroadcastChannel API

The way of 1) and 2) is using Chrome Process..
Because, communicate with child process(content) and Parent Process(content).

So We can use 3) BroadcastChannel API.
But, BroadcastChannel can use only same-origin.
AFAIK, origin of Keyboard Apps and Content Apps is different.

So We should extend the BroadcastChannelAPI or create new API.
(Reporter)

Comment 2

2 years ago
Created attachment 8613834 [details]
[WIP]BroadcastChannelInternal.webidl

I creating the temporary implements.
This attachment is small sample in order to start implementing.

I tried extending BroadcastChannel API.
BroadcastChannel API is not allow cross-origin.
So that, I extended BroadcastChannel API in order to allow cross-origin.
(I'm going to ignore origin check when using extended BroadcsatChannel.)

The extended API is [ChromeOnly].
Because it is two module using this API in chrome permission.
1) MozKeyboard.js
2) forms.js

We can add ChromeOnly attribute to extended API.

According to ChromeOnly attribute, general apps can't access this API.

Olli,
What do you think about this way?
Attachment #8613834 - Flags: feedback?(bugs)
There is another requirement to be fulfilled, and I don't know if it's possible with this API change: you can't send key information to other content processes that does not currently receiving focus; it's a breach to the security principle.
(Reporter)

Comment 4

2 years ago
tim,
Thank you for your information.

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #3)
> There is another requirement to be fulfilled, and I don't know if it's
> possible with this API change: you can't send key information to other
> content processes that does not currently receiving focus; it's a breach to
> the security principle.
Current temporary implementation have one connection which communicate with keyboard and content always.

WIP:
1) It will close the channel using close() method when content lost focus(blur event).

2) BroadcastChannel and BroadcastChannelInternal is not cross talk.
   Even if the channel's name is same name at BroadcastChannel and BroadcastChannelInternal, it will treat different channel as.

I think that general webpage and unfocused app can't receive keyboard message by adding [ChromeOnly] attribute to BroadcastChannelInternal.

Comment 5

2 years ago
Comment on attachment 8613834 [details]
[WIP]BroadcastChannelInternal.webidl

baku should comment on BroadcastChannel.
(but in principle I like the idea to add some chrome only BC.)
Attachment #8613834 - Flags: feedback?(bugs) → feedback?(amarchesini)
Comment on attachment 8613834 [details]
[WIP]BroadcastChannelInternal.webidl

What about:

partial interface BroadcastChannel {
  [ChromeOnly]
  void postChromeMesage(any message);
}

or a better name.
Attachment #8613834 - Flags: feedback?(amarchesini) → feedback-
(Reporter)

Comment 7

2 years ago
Created attachment 8615115 [details] [diff] [review]
[WIP]BroadcastChannel.webidl

Hi baku,
Thank you for your feedback.

(In reply to Andrea Marchesini (:baku) from comment #6)
> Comment on attachment 8613834 [details]
> [WIP]BroadcastChannelInternal.webidl
> 
> What about:
> 
> partial interface BroadcastChannel {
>   [ChromeOnly]
>   void postChromeMesage(any message);
> }
> 
> or a better name.
Oh, it's looks like smart idea. thank you!

I modified Internal Message method into BC.
(and now started implementing this method.)
Attachment #8613834 - Attachment is obsolete: true
Attachment #8615115 - Flags: feedback?(amarchesini)
Out of curiosity, how does the Broadcastchannel api relay messages from processA to processB without using the parent process?
(In reply to Fabrice Desré [:fabrice] from comment #8)
> Out of curiosity, how does the Broadcastchannel api relay messages from
> processA to processB without using the parent process?

It does use the parent process, but the message is relayed in an off-main-thread manner.
Attachment #8615115 - Attachment is patch: true
Attachment #8615115 - Attachment mime type: text/x-csrc → text/plain
> It does use the parent process, but the message is relayed in an
> off-main-thread manner.

We use PBackground to send messages between processes. The WebIDL file is ok. I'm happy to review your code when it's ready and if you need help, let me know.
Comment on attachment 8615115 [details] [diff] [review]
[WIP]BroadcastChannel.webidl

1. Is it actually sending message to Chrome code only?
2. I don't know if it's really 'temporary'. If it is, file a follow-up to remove it and add the bug ID in the WebIDL file.
Attachment #8615115 - Flags: feedback?(amarchesini) → feedback+
(Reporter)

Comment 12

2 years ago
Hi baku,

Sorry for late replay.

> 1. Is it actually sending message to Chrome code only?
Yes, It is sending message to Chrome code only.
It'll check whether BroadcastChannel-Instance was created by chrome code.[1]
If it was created by chrome code, BroadcastChannel instance can receive the message.[2]

So, instance of chrome is able to receive [postMessageToChromeOnly] message.

> 2. I don't know if it's really 'temporary'. If it is, file a follow-up to
> remove it and add the bug ID in the WebIDL file.
No, It's not a patch.
It's a experimental code.

I would like to split this bug in order to avoid confusion as follow.

1) Modifying the BroadcastChannel in order to send chrome.
2) Modifying the InputMethod API. (this bug)

[1] http://mxr.mozilla.org/mozilla-central/source/dom/broadcastchannel/BroadcastChannel.cpp#384
[2] http://mxr.mozilla.org/mozilla-central/source/dom/broadcastchannel/BroadcastChannelChild.cpp#120
(Reporter)

Updated

2 years ago
Depends on: 1174624
(Reporter)

Updated

2 years ago
Depends on: 1224989
You need to log in before you can comment on or make changes to this bug.