Last Comment Bug 683099 - NS_NATIVE_WINDOW value should not be used as IPC shareable
: NS_NATIVE_WINDOW value should not be used as IPC shareable
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla9
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: 590299 671550
  Show dependency treegraph
 
Reported: 2011-08-29 23:56 PDT by Oleg Romashin (:romaxa)
Modified: 2011-09-01 01:53 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Provide NS_NATIVE_SHAREABLE_WINDOW native widget data. (5.84 KB, patch)
2011-08-29 23:57 PDT, Oleg Romashin (:romaxa)
roc: review+
Details | Diff | Splinter Review
Also crossprocess shareable window within puppet widget (3.25 KB, patch)
2011-08-30 00:40 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
crossprocess shareable window within puppet widget (2.16 KB, patch)
2011-08-30 08:55 PDT, Oleg Romashin (:romaxa)
roc: review+
Details | Diff | Splinter Review
Provide NS_NATIVE_SHAREABLE_WINDOW native widget data. TO PUSH (7.92 KB, patch)
2011-08-31 07:27 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2011-08-29 23:56:18 PDT
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) 2011-08-29 23:57:43 PDT
Created attachment 556765 [details] [diff] [review]
Provide NS_NATIVE_SHAREABLE_WINDOW native widget data.
Comment 2 Oleg Romashin (:romaxa) 2011-08-30 00:40:50 PDT
Created attachment 556768 [details] [diff] [review]
Also crossprocess shareable window within puppet widget
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-30 02:23:58 PDT
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) 2011-08-30 08:45:45 PDT
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) 2011-08-30 08:55:27 PDT
Created attachment 556862 [details] [diff] [review]
crossprocess shareable window within puppet widget
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-08-30 15:34:46 PDT
(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 2011-08-30 20:30:45 PDT
(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) 2011-08-30 20:42:45 PDT
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 2011-08-30 23:00:49 PDT
Thanks roc.
Comment 10 Ed Morley [:emorley] 2011-08-31 05:12:27 PDT
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] 2011-08-31 05:22:53 PDT
Comment on attachment 556862 [details] [diff] [review]
crossprocess shareable window within puppet widget

(Missing commit message/author)
Comment 12 Oleg Romashin (:romaxa) 2011-08-31 07:27:23 PDT
Created attachment 557162 [details] [diff] [review]
Provide NS_NATIVE_SHAREABLE_WINDOW native widget data. TO PUSH

Folded patch
Comment 13 Oleg Romashin (:romaxa) 2011-08-31 07:46:27 PDT
Sent to try server:
http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=b7f33be9a97e
Comment 15 Ed Morley [:emorley] 2011-09-01 01:53:53 PDT
http://hg.mozilla.org/mozilla-central/rev/e43df9a4b36e

Note You need to log in before you can comment on or make changes to this bug.