Move GeckoAppShell.runGecko to GeckoThread

RESOLVED FIXED in Firefox 43

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

Trunk
Firefox 43
All
Android
Points:
---

Firefox Tracking Flags

(firefox42 affected, firefox43 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
GeckoAppShell.runGecko is a relic from the past and really should belong in GeckoThread.
(Assignee)

Comment 1

2 years ago
Created attachment 8644631 [details] [diff] [review]
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.
Attachment #8644631 - Flags: review?(snorp)
(Assignee)

Comment 2

2 years ago
Created attachment 8644632 [details] [diff] [review]
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.
Attachment #8644632 - Flags: review?(snorp)
(Assignee)

Comment 3

2 years ago
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 :)
Attachment #8644631 - Flags: review?(snorp) → review?(esawin)
(Assignee)

Updated

2 years ago
Attachment #8644632 - Flags: review?(snorp) → review?(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.
Attachment #8644631 - Flags: review?(esawin) → review+

Updated

2 years ago
Attachment #8644632 - Flags: review?(esawin) → review+
(Assignee)

Comment 5

2 years ago
(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

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/46f21e3aeed5
https://hg.mozilla.org/integration/mozilla-inbound/rev/224df670cbb6
https://hg.mozilla.org/mozilla-central/rev/46f21e3aeed5
https://hg.mozilla.org/mozilla-central/rev/224df670cbb6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.