Expose MessageLoop API with custom MessagePump

NEW
Unassigned

Status

()

Core
IPC
3 years ago
2 years ago

People

(Reporter: tatiana, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Created attachment 8437656 [details] [diff] [review]
MessageLoop with custom Message Pump

In order to provide MessageLoop integration into existing External Toolkit event loop, we need to expose MessageLoop API with custom MessagePump.

Current MessageLoop API provide only option to run Blocking Message loop, which blocks native event loop make things harder to implement.
Attachment #8437656 - Flags: review?(bent.mozilla)
Hi Tatiana,

Can you provide some context for me here? Why is this needed, and which project is this being used for?

Thanks!
Flags: needinfo?(tanya.meshkova)
(Reporter)

Comment 2

3 years ago
This is for embedding project:
https://wiki.mozilla.org/Embedding/IPCLiteAPI

By default we do have MessageLoop platform implementations which allow us integrate Platform toolkit event system into Chromium message loop. But current Run() API is kinda blocking.

With custom MessagePump I'm able to expose Platform independent embedding API (https://github.com/tmeshkova/gecko-dev/blob/embedlite/embedding/embedlite/EmbedLiteMessagePump.cpp), and create Async version of Event loop.
https://github.com/tmeshkova/qtmozembed/blob/master/src/qmessagepump.cpp
Which is not blocking embedding stack.
Flags: needinfo?(tanya.meshkova)
Comment on attachment 8437656 [details] [diff] [review]
MessageLoop with custom Message Pump

Benjamin, since this is about embedding I think you'd be a better reviewer here...
Attachment #8437656 - Flags: review?(bent.mozilla) → review?(benjamin)
Comment on attachment 8437656 [details] [diff] [review]
MessageLoop with custom Message Pump

In what way is TYPE_MOZILLA_CHILD different from TYPE_EMBED? What are the assumptions about how this kind of message loop will actually work?

Also, please see the style guide for indentation; you need

MessageLoop::MessageLoop(base::MessagePump* messagePump)
  : type_(TYPE_EMBED)
  , id_(++message_loop_id_seq)
Attachment #8437656 - Flags: review?(benjamin)
Flags: needinfo?(tanya.meshkova)
(Reporter)

Comment 5

3 years ago
> In what way is TYPE_MOZILLA_CHILD different from TYPE_EMBED? What are the
> assumptions about how this kind of message loop will actually work?

MOZILLA_CHILD loop is running along with Gecko Event loop, and it is blocking executing stack after Run() call.
1) With embedlite setup we have UI thread, which is simply main thread of Widget toolkit application (Qt/Gtk).
2) Also we have Gecko content Thread which is running normal TYPE_MOZILLA_UI Message loop, and hosting dom/content/layout/js et.c.

we do have IPDL interface between 1) and 2), and in order make it happen we need to get chromium message loop running in 1) and processing Widget toolkit events + chromium ipdl messages, and not blocking 1)

so this API allow to build custom non-blocking message pump outside of embedding (Qt,Gtk,efl). see Qt example in comment#2

> 
> Also, please see the style guide for indentation; you need
> 
> MessageLoop::MessageLoop(base::MessagePump* messagePump)
>   : type_(TYPE_EMBED)
>   , id_(++message_loop_id_seq)

I was following context code style, and was using style same as in ctor below
http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/message_loop.cc#96
MessageLoop::MessageLoop(Type type)
     : type_(type),
       id_(++message_loop_id_seq),

Should I change that too? is that ok with original chromium upstream code style?
Flags: needinfo?(tanya.meshkova)
(Reporter)

Updated

3 years ago
Attachment #8437656 - Flags: feedback?(benjamin)
I am not going to have time to look at this until after my vacation, so at best the second week of August.
(Reporter)

Comment 7

3 years ago
Boris is there are change for you to look at this patch?
(Reporter)

Updated

3 years ago
Attachment #8437656 - Flags: feedback?(bzbarsky)
Comment on attachment 8437656 [details] [diff] [review]
MessageLoop with custom Message Pump

Sorry, I don't know anything about the message loop code.  :(
Attachment #8437656 - Flags: feedback?(bzbarsky)
(Reporter)

Comment 9

3 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #6)
> I am not going to have time to look at this until after my vacation, so at
> best the second week of August.

Any chance to look at this?
(Reporter)

Updated

3 years ago
Attachment #8437656 - Flags: feedback?(benjamin) → review?(benjamin)
Comment on attachment 8437656 [details] [diff] [review]
MessageLoop with custom Message Pump

I am not going to review this.
Attachment #8437656 - Flags: review?(benjamin)
You need to log in before you can comment on or make changes to this bug.