Closed
Bug 575567
Opened 14 years ago
Closed 14 years ago
Drop X[Parent/Child] widget embedding from remote browser
Categories
(Core :: IPC, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: romaxa, Assigned: romaxa)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
I'm using patches from bug 130078 which does not have widgets structure...
and IPC nsFrameLoader::ShowRemoteFrame fails on
http://mxr.mozilla.org/projects-central/source/electrolysis/content/base/src/nsFrameLoader.cpp#767
Attachment #454814 -
Flags: review?(Olli.Pettay)
Comment 1•14 years ago
|
||
Comment on attachment 454814 [details] [diff] [review]
Kill XEmbed for Qt... (I think it make sense for GTK builds too)
I'm not at all familiar how bug 130078 changes this all.
Assignee | ||
Comment 2•14 years ago
|
||
(In reply to comment #1)
> (From update of attachment 454814 [details] [diff] [review])
> I'm not at all familiar how bug 130078 changes this all.
that bug is removing multiple nsIWidgets structure, and making only one nsIWidget per window.. and this code just cannot find nsIWidget for some nsIView...
I would suggest to remove all this XEmbed code completely... because it does not make any sense
Comment 3•14 years ago
|
||
Comment on attachment 454814 [details] [diff] [review]
Kill XEmbed for Qt... (I think it make sense for GTK builds too)
So could you still explain how the widget handling works with the patch in e10s. How is the child process connected to parent?
Assignee | ||
Comment 4•14 years ago
|
||
> So could you still explain how the widget handling works with the patch in
> e10s. How is the child process connected to parent?
Rendering working with remote layers or canvas..
Should it be connected somehow else?
If yes then I think XEmbed is not good way to connect child and parent process... also I think child process should be widgetless in general.
Comment 5•14 years ago
|
||
Comment on attachment 454814 [details] [diff] [review]
Kill XEmbed for Qt... (I think it make sense for GTK builds too)
I think someone more familiar with widget handling should review this.
Attachment #454814 -
Flags: review?(Olli.Pettay)
Assignee | ||
Updated•14 years ago
|
Attachment #454814 -
Flags: review?(doug.turner)
Assignee | ||
Updated•14 years ago
|
Summary: IPC tab widget initialization fails with patches from bug 130078 → Drop X[Parent/Child] widget embedding from remote browser
Assignee | ||
Comment 6•14 years ago
|
||
Currently we are initializing some weird X-Embedding stuff for browser element... that brings some confusing about how remote browser works, and bring some unexpected behavior...
I think we should handle remote tab views the same way for all platforms... using remote layers environment.
Assignee: nobody → romaxa
Attachment #454814 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #457841 -
Flags: review?(jones.chris.g)
Attachment #454814 -
Flags: review?(doug.turner)
Comment 7•14 years ago
|
||
How does focus work? Currently we focus the child widget in order to deliver keyboard events. Will the child process have *any* widget?
Assignee | ||
Comment 8•14 years ago
|
||
> How does focus work? Currently we focus the child widget in order to deliver
Focus events should be delivered from parent -> child, the same way as mouse,key and other events.
> keyboard events. Will the child process have *any* widget?
I hope no real widget... probably only nsIWidget wrapper class without real gdk/qt/win widgets...
Assignee | ||
Comment 9•14 years ago
|
||
Comment on attachment 457841 [details] [diff] [review]
Drop Widget embedding completely and stop confusing people.
This patch make same remote browser initialization for all platforms.
XEmbedding approach is not going to be used in fennec 2.0
And currently XEmbedding on Gtk/Win brings not correct expose events which are activating ShadowLayer tree... and on Qt/Android shadowLayer tree not activated at all, because XEmbedding disabled there.
I think we should disable XEmbed and create implementation for Focus/ShadowTreeActivate events.
Attachment #457841 -
Flags: review?(jones.chris.g) → feedback?(jones.chris.g)
(In reply to comment #8)
> > keyboard events. Will the child process have *any* widget?
> I hope no real widget... probably only nsIWidget wrapper class without real
> gdk/qt/win widgets...
I talked to roc about that approach at the summit, and he's not in favor of fake widgets. I'm still pretty confident we can de-widgetize the relevant code here. (Or at least teach it to deal with widget=NULL.)
I'm afraid I don't have the requisite widget-fu to know if these changes will work. Isn't not parenting the widgets to the chrome process's window going to cause problems? I think it would.
I definitely like the massive block of removed code here, but I'm not sure what you're trying to accomplish with this intermediate step. Maybe energy is better focused on nuking widgets from content processes entirely?
Assignee | ||
Comment 12•14 years ago
|
||
> better focused on nuking widgets from content processes entirely?
yep, that is the next part... with current patch we still have real widget on child process... next part is replace parent native widget with something like win=0x12345, and make sure that gtk/qt-nsIWidget can be created and work without real platform widgets.
(also microb on N900 does not have any real widgets in child process)
We should have taken this patch, my mistake.
http://hg.mozilla.org/mozilla-central/rev/be857a136ffd
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•14 years ago
|
||
Comment on attachment 457841 [details] [diff] [review]
Drop Widget embedding completely and stop confusing people.
Ok, this is already fixed in cedar... 582057
Attachment #457841 -
Flags: feedback?(jones.chris.g)
You need to log in
before you can comment on or make changes to this bug.
Description
•