1189995 - Move GeckoAppShell.runGecko to GeckoThread

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

GeckoAppShell.runGecko is a relic from the past and really should belong in GeckoThread.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Move GeckoAppShell.runGecko to GeckoThread (v1)

GeckoAppShell.runGecko really should be in GeckoThread because
GeckoThread already takes care of most of the preparation when running
Gecko. This patch merges runGecko into GeckoThread.run, but splits the
argument-building code into its own method.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Move GeckoAppShell.pumpMessageLoop to GeckoThread (v1)

This method is used by Gecko to pump the Android message loop, and it's
also more suited to GeckoThread than GeckoAppShell.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8644631 [details] [diff] [review]
Move GeckoAppShell.runGecko to GeckoThread (v1)

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

I think I should keep :esawin involved in some of these changes too :)

Comment 4

[Eugen Sawin [:esawin]]

Comment on attachment 8644631 [details] [diff] [review]
Move GeckoAppShell.runGecko to GeckoThread (v1)

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

This looks good!

Is there a reason to remove the idle handler after dispatching Gecko:Exited? That seems to be the only change in semantics I see.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Eugen Sawin [:esawin] from comment #4)
> Comment on attachment 8644631 [details] [diff] [review]
> Move GeckoAppShell.runGecko to GeckoThread (v1)
> 
> Review of attachment 8644631 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good!
> 
> Is there a reason to remove the idle handler after dispatching Gecko:Exited?
> That seems to be the only change in semantics I see.

I don't think it matters where we do it, so I just put it at the end.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/46f21e3aeed5
https://hg.mozilla.org/integration/mozilla-inbound/rev/224df670cbb6

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/46f21e3aeed5
https://hg.mozilla.org/mozilla-central/rev/224df670cbb6