683099 - NS_NATIVE_WINDOW value should not be used as IPC shareable

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mozilla-central/rev/f1d496722775#l11.29
We are using widget->GetNativeData(NS_NATIVE_WINDOW) value to send it over IPC.. that is probably correct for windows, but not for Linux and other platforms.

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Provide NS_NATIVE_SHAREABLE_WINDOW native widget data.

Comment 2

[Oleg Romashin (:romaxa)]

Attachment: Also crossprocess shareable window within puppet widget

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 556768 [details] [diff] [review]
Also crossprocess shareable window within puppet widget

Review of attachment 556768 [details] [diff] [review]:
-----------------------------------------------------------------

This patch assumes PuppetWidgets are always associated with the same toplevel browser window. That might not always be true, e.g. in multiprocess desktop Firefox it's probably not going to be true when you drag tabs from one window to another.

So PuppetWidget::GetNativeData probably needs to call SendGetNativeWidgetData every time, or else we need a way to update the cached value.

Comment 4

[Oleg Romashin (:romaxa)]

Is widget SetParent/Reparent the only place where window could be changed or is there are any other place can do that?

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: crossprocess shareable window within puppet widget

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Oleg Romashin (:romaxa) from comment #4)
> Is widget SetParent/Reparent the only place where window could be changed or
> is there are any other place can do that?

I don't think we'd call SetParent on PuppetWidgets when moving a content-process tab between windows, because the PuppetWidget doesn't know about the native widget parent.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Oleg Romashin (:romaxa) from comment #0)
> http://hg.mozilla.org/mozilla-central/rev/f1d496722775#l11.29
> We are using widget->GetNativeData(NS_NATIVE_WINDOW) value to send it over
> IPC.. that is probably correct for windows, but not for Linux and other
> platforms.

Oleg, could you please give background for this patch? Why can't we pass GetNativeData(NS_NATIVE_WINDOW) through ipc and what magic you add to achieve this?

Comment 8

[Oleg Romashin (:romaxa)]

because NS_NATIVE_WINDOW return shareable handle only on window HWND.
on linux Gtk/Qt that returns Toolkit objects (GdkWindow  or QWidget), and obviously you cannot share that across the processes. Only XID (XServer window can be shared on linux) and on mac someone else there are should be some displayPort or something like that..

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Thanks roc.

Comment 10

[Ed Morley [:emorley]]

Oleg, has this been through try or do I need to test it with some stuff I'm pushing now?

Comment 11

[Ed Morley [:emorley]]

Comment on attachment 556862 [details] [diff] [review]
crossprocess shareable window within puppet widget

(Missing commit message/author)

Comment 12

[Oleg Romashin (:romaxa)]

Attachment: Provide NS_NATIVE_SHAREABLE_WINDOW native widget data. TO PUSH

Folded patch

Comment 13

[Oleg Romashin (:romaxa)]

Sent to try server:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=b7f33be9a97e

Comment 14

[Ed Morley [:emorley]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e43df9a4b36e

Comment 15

[Ed Morley [:emorley]]

http://hg.mozilla.org/mozilla-central/rev/e43df9a4b36e