Closed Bug 975452 Opened 7 years ago Closed 2 years ago
Implement metro widgets for input thread separation
TODO: - implement required nsIWidget method stubs - add override stubs we use in MetroWidget - figure out if we can use MetroWidget for MetroWidgetInput without breaking non thread sep functionality. We might just go ahead and copy code over to a new MetroWidgetInput class. Probably simpler. - deal with tricky sFrameworkView calls on the gecko side in MetroApp - implement communication between MetroWidgetGecko and MetroWidgetInput - get MetroWidgetInput talking to the apzc - get MetroInput talking to MetroWidgetInput - test MetroApp browser shutdown sequence
Tim is this something you could pick up and run with, or would you prefer I take this a bit further?
Yeah I'd be super happy to sink my teeth into this!
Assignee: nobody → tabraldes
Status: NEW → ASSIGNED
One helpful tip - when you build this up you can find the ipdl generated headers here - obj\ipc\ipdl\_ipdlheaders\mozilla\widget\winrt you'll need the patches in bug 974901 & bug 974989. if those don't apply cleanly ping me on irc.
One other tip - we had talked about how input events between the two widgets would transfer from one side to the other as gecko events. Thankfully we already have gecko event serializers for these which saves us a ton of work - http://mxr.mozilla.org/mozilla-central/source/widget/nsGUIEventIPC.h
Updated to get things building. I think we should limit the work in this bug to getting the plumbing fleshed out and the browser painting. The more complex interfaces between the widgets (ime, input) and things like apzc support can get fleshed out in other bugs.
Attachment #8379814 - Attachment is obsolete: true
I've worked around the original issue with the constructor, but I've come up on something else that's more serious. We attempt to connect when the first widget gets created, but this happens before nsAppShell::Run is called and as such the gecko thread isn't in a chromium message loop yet. So when the input thread attempts to connect, there's no loop set up on the other side. Not sure yet how to fix this.
This combines the winrt code from bug 974989 plus all the changes here.
I was thinking about the idle service problem, I think the best solution in cases like this would be to check to see if we're running on the main thread before accessing the main thread resource. If we are, make the call, if we aren't bounce the call to the main thread using an nsRunnable class. We might find a use for a common set of nsRunnables included via a common header, or maybe this problem isn't that common.
Breaking these up so they are more manageable.
Attachment #8382462 - Attachment is obsolete: true
(In reply to Jim Mathies [:jimm] from comment #9) > I was thinking about the idle service problem, I think the best solution in > cases like this would be to check to see if we're running on the main thread > before accessing the main thread resource. If we are, make the call, if we > aren't bounce the call to the main thread using an nsRunnable class. > > We might find a use for a common set of nsRunnables included via a common > header, or maybe this problem isn't that common. Note this won't solve the case where we need a return result, and as it turns out I found another of these in nsBaseWidget::BaseCreate, which initializes a new nsDeviceContext which creates a nsIScreenManager. We'll have to take each of these on individually to get a feel for how wide spread the problem is.
Updated to apply cleanly over 170933:4cfb6c61b137
Attachment #8383035 - Attachment is obsolete: true
Updated to apply cleanly over the part 1 patch
Attachment #8383037 - Attachment is obsolete: true
This patch contains miscellaneous fixes that I've been using to get metroFx running. 1) It enables the metroFx debug delay 2) It comments out most of MetroUtils::ScaleFactor and just returns 1.0 - WinRT was throwing a RoOriginateError when we tried to do "dispInfoStatics->GetForCurrentView(&dispInfo)" 3) It makes MetroWidget::AnswerLateCreate create and initialize an nsDeviceContext, then pass that in to the call to |Create|. I think we'll actually want to keep this change. 4) It expands the macros that were used to implement AddRef, QI, and Release for nsScreenManagerWin. I'm not sure why, but when the macros were being used I was seeing an exception thrown in AddRef and the debugger was reporting that I was in nsSimpleArrayEnumerator::AddRef. Next to tackle: A RoOriginateError is being thrown from FrameworkView::ActivateView when called from MetroApp::ActivateBaseView. This looks like it might be happening on the wrong thread (FrameworkView probably shouldn't be accessed from the gecko thread)
> 4) It expands the macros that were used to implement AddRef, QI, and Release > for nsScreenManagerWin. I'm not sure why, but when the macros were being > used I was seeing an exception thrown in AddRef and the debugger was > reporting that I was in nsSimpleArrayEnumerator::AddRef. xpcom interfaces that aren't thread safe (most of them) assert if the thread accessing them isn't the thread that creates the object - http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsISupportsImpl.h#54 I'd bet this is the source of the assertions.
Yeah I bet the assert is in the component manager somewhere when the input thread tries to create the screen manager via do_GetService. This is the heart of the problem we'll have to solve. MetroWidget(input) can't access non-thread safe xpcom interfaces directly. We'll have to find a way to bypass these types of calls. I think you can get away with just stubbing stuff like screen manager out in base widget. MetroWidget(input) doesn't need it, so there's no point in initializing or accessing it. MetroWidgetGecko on the other hand will have to call BaseCreate and create a valid screen manager. We have to look at these interfaces and figure out what they do - if they query for system information that can happen on the gecko thread, great. If they query for information only the input thread can access, we'll need our own version of these objects that intr calls over to a corresponding input thread class. Both the MetroApp and MetroWidget protocols can create additional protocols (objects) when we need them.
If you go the root you were thinking of taking, landing this all at once, MetroWidget doesn't need to inherit from base widget or implement nsIWidget, which solves the screen manager problem and other potential xpcom issues in that class.
I don't expect to work on metroFx in the near future, though it would be fun to try to use the lessons/approach here for desktopFx.
Assignee: tabraldes → nobody
Status: ASSIGNED → NEW
We never shipped the metro support, closing!
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.