Closed Bug 575567 Opened 14 years ago Closed 14 years ago

Drop X[Parent/Child] widget embedding from remote browser

Categories

(Core :: IPC, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: romaxa, Assigned: romaxa)

References

Details

Attachments

(1 file, 1 obsolete file)

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 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.
(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
Blocks: 130078
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?
> 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 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)
Attachment #454814 - Flags: review?(doug.turner)
Summary: IPC tab widget initialization fails with patches from bug 130078 → Drop X[Parent/Child] widget embedding from remote browser
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)
How does focus work? Currently we focus the child widget in order to deliver keyboard events. Will the child process have *any* widget?
> 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...
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?
> 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
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.

Attachment

General

Created:
Updated:
Size: