Win8 metro's app model is a little different from conventional desktop apps - the thread that enters XRE_main is not the thread the UI is rendered on. We need to be able to shunt off the main thread early and drop it into a main metro entry point, then call back using a ui worker thread to finish up threading & xpcom initialization. This patch makes that possible.

There's no need to prefix the XREMain methods with XRE_, is there?

(In reply to Ms2ger from comment #2) > There's no need to prefix the XREMain methods with XRE_, is there? Not really, but it does make searching for these methods easier. XRE_main was a really big function.

- Catch some missed early exits for command line dump commands.

- fix up NS_TIME_FUNCTION diagnostic code so that it builds.

- linux / mac bustage fixes. I ran this through try a few times, didn't see any issues.

Comment on attachment 607735 [details] [diff] [review] patch v.4 Dave, since this is a pretty big change in the xre code I figured I'd ask you for r?. Note the patch looks pretty big but really 90% of this is just code reformatting. Comment 1 explains a bit about why these changes were needed. Generally what I've done is break up XRE_main into a set of methods in a simple class names XREMain. In win8 metro, we'll need to to call the init parts and then call back so that a different thread is considered the main thread. Here's that code if your curious: https://hg.mozilla.org/projects/elm/rev/ff3c7db1fce0

Comment on attachment 607735 [details] [diff] [review] patch v.4 Review of attachment 607735 [details] [diff] [review]: ----------------------------------------------------------------- I want to look over this a second time to make sure I didn't miss anything but otherwise this is looking pretty good. ::: toolkit/xre/nsAppRunner.cpp @@ +3097,5 @@ > > // Handle -no-remote and -new-instance command line arguments. Setup > // the environment to better accommodate other components and various > // restart scenarios. > + if (CheckArg("no-remote", true) == ARG_BAD) { You need to still store this in ar for the else-if below @@ +3104,5 @@ > } else if (ar == ARG_FOUND) { > SaveToEnv("MOZ_NO_REMOTE=1"); > } > + > + if (CheckArg("new-instance", true) == ARG_BAD) { Ditto @@ +3178,5 @@ > + bool canRun = false; > + rv = mNativeApp->Start(&canRun); > + if (NS_FAILED(rv) || !canRun) { > + return 1; > + } Why move this stuff earlier? @@ +3180,5 @@ > + if (NS_FAILED(rv) || !canRun) { > + return 1; > + } > + > + rv = mDirProvider.Initialize(mAppData->directory, mAppData->xreDirectory); This seems to happen twice now

> I want to look over this a second time to make sure I didn't miss anything > but otherwise this is looking pretty good. Ok, posting a new rev now. > > ::: toolkit/xre/nsAppRunner.cpp > @@ +3097,5 @@ > > > > // Handle -no-remote and -new-instance command line arguments. Setup > > // the environment to better accommodate other components and various > > // restart scenarios. > > + if (CheckArg("no-remote", true) == ARG_BAD) { > > You need to still store this in ar for the else-if below > > @@ +3104,5 @@ > > } else if (ar == ARG_FOUND) { > > SaveToEnv("MOZ_NO_REMOTE=1"); > > } > > + > > + if (CheckArg("new-instance", true) == ARG_BAD) { > > Ditto fixed. > > @@ +3178,5 @@ > > + bool canRun = false; > > + rv = mNativeApp->Start(&canRun); > > + if (NS_FAILED(rv) || !canRun) { > > + return 1; > > + } > > Why move this stuff earlier? Debugging mistake on my part, was experimenting with this in mainInit vs. mainStartup and didn't put it back in the right place. :/ Updated. > > @@ +3180,5 @@ > > + if (NS_FAILED(rv) || !canRun) { > > + return 1; > > + } > > + > > + rv = mDirProvider.Initialize(mAppData->directory, mAppData->xreDirectory); > > This seems to happen twice now Removed the incorrectly placed chuck of code. I'll fire off a new set of try builds as well with these changes.

sorry, missed the -w diff option.

(In reply to Jim Mathies [:jimm] from comment #11) > Created attachment 608061 [details] [diff] [review] > patch v.5 -w > > sorry, missed the -w diff option. https://tbpl.mozilla.org/?noignore=0&tree=Try&rev=e1005c7e270e These changes passed try.