Further optimise activity switching

RESOLVED FIXED in Firefox 55

Status

()

Firefox for Android
General
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: JanH, Assigned: JanH)

Tracking

Trunk
Firefox 55
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(8 attachments)

59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
59 bytes, text/x-review-board-request
sebastian
: review+
walkingice
: review+
Details | Review
After a friendly nudge during review, it turned out that bug 1352997 might allow simplifying the original activity switching approach from bug 1351739.
Blocks: 1362846
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 9

a year ago
mozreview-review
Comment on attachment 8865220 [details]
Bug 1359531 - Part 1 - Call GeckoApp's tab event handler from web apps/custom tabs, too.

https://reviewboard.mozilla.org/r/134854/#review140042
Attachment #8865220 - Flags: review?(s.kaspari) → review+

Comment 10

a year ago
mozreview-review
Comment on attachment 8865221 [details]
Bug 1359531 - Part 2 - Move some tab switch functionality to the tab object.

https://reviewboard.mozilla.org/r/134866/#review140052
Attachment #8865221 - Flags: review?(s.kaspari) → review+

Comment 11

a year ago
mozreview-review
Comment on attachment 8865222 [details]
Bug 1359531 - Part 3 - Creating new intents is a job for IntentHelper.

https://reviewboard.mozilla.org/r/134868/#review141288
Attachment #8865222 - Flags: review?(s.kaspari) → review+

Comment 12

a year ago
mozreview-review
Comment on attachment 8865223 [details]
Bug 1359531 - Part 4 - Import Tab intent extra key definitions.

https://reviewboard.mozilla.org/r/134870/#review141346
Attachment #8865223 - Flags: review?(s.kaspari) → review+

Comment 13

a year ago
mozreview-review
Comment on attachment 8865224 [details]
Bug 1359531 - Part 5 - Trigger activity switching from GeckoApp.

https://reviewboard.mozilla.org/r/134856/#review141348
Attachment #8865224 - Flags: review?(s.kaspari) → review+

Comment 14

a year ago
mozreview-review
Comment on attachment 8865225 [details]
Bug 1359531 - Part 6 - Remove manual setting of current activity.

https://reviewboard.mozilla.org/r/134860/#review141350
Attachment #8865225 - Flags: review?(s.kaspari) → review+

Comment 15

a year ago
mozreview-review
Comment on attachment 8865226 [details]
Bug 1359531 - Part 7 - Don't ignore last selected tab for Intent.ACTION_MAIN.

https://reviewboard.mozilla.org/r/136622/#review141352
Attachment #8865226 - Flags: review?(s.kaspari) → review+

Comment 16

a year ago
mozreview-review
Comment on attachment 8865227 [details]
Bug 1359531 - Part 8 - Handle tabs selected while all GeckoApps were in background.

https://reviewboard.mozilla.org/r/136624/#review141354
Attachment #8865227 - Flags: review?(s.kaspari) → review+

Comment 17

a year ago
mozreview-review
Comment on attachment 8865220 [details]
Bug 1359531 - Part 1 - Call GeckoApp's tab event handler from web apps/custom tabs, too.

https://reviewboard.mozilla.org/r/134854/#review142322
Attachment #8865220 - Flags: review?(walkingice0204) → review+
So in the mean time, over in bug 1363567 jchen has discovered the GeckoActivityMonitor and found it useful for a problem of his. Besides, despite what I've written in the commit message for part 6, I'm actually still undecided on whether the activity monitor might not after all be helpful in improving our application-background/foreground logic.

Comment 19

a year ago
mozreview-review
Comment on attachment 8865221 [details]
Bug 1359531 - Part 2 - Move some tab switch functionality to the tab object.

https://reviewboard.mozilla.org/r/134866/#review142324
Attachment #8865221 - Flags: review?(walkingice0204) → review+

Comment 20

a year ago
mozreview-review
Comment on attachment 8865222 [details]
Bug 1359531 - Part 3 - Creating new intents is a job for IntentHelper.

https://reviewboard.mozilla.org/r/134868/#review142326
Attachment #8865222 - Flags: review?(walkingice0204) → review+

Comment 21

a year ago
mozreview-review
Comment on attachment 8865223 [details]
Bug 1359531 - Part 4 - Import Tab intent extra key definitions.

https://reviewboard.mozilla.org/r/134870/#review142328
Attachment #8865223 - Flags: review?(walkingice0204) → review+

Comment 22

a year ago
mozreview-review
Comment on attachment 8865224 [details]
Bug 1359531 - Part 5 - Trigger activity switching from GeckoApp.

https://reviewboard.mozilla.org/r/134856/#review142330
Attachment #8865224 - Flags: review?(walkingice0204) → review+

Comment 23

a year ago
mozreview-review
Comment on attachment 8865225 [details]
Bug 1359531 - Part 6 - Remove manual setting of current activity.

https://reviewboard.mozilla.org/r/134860/#review142332
Attachment #8865225 - Flags: review?(walkingice0204) → review+

Comment 24

a year ago
mozreview-review
Comment on attachment 8865226 [details]
Bug 1359531 - Part 7 - Don't ignore last selected tab for Intent.ACTION_MAIN.

https://reviewboard.mozilla.org/r/136622/#review142334
Attachment #8865226 - Flags: review?(walkingice0204) → review+

Comment 25

a year ago
mozreview-review
Comment on attachment 8865227 [details]
Bug 1359531 - Part 8 - Handle tabs selected while all GeckoApps were in background.

https://reviewboard.mozilla.org/r/136624/#review142336
Attachment #8865227 - Flags: review?(walkingice0204) → review+
And bug 1363567 is landing now, so part 6 needs updating to no longer rip out the GAM after all.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8865225 - Flags: review?(walkingice0204)
Attachment #8865225 - Flags: review?(s.kaspari)
Attachment #8865225 - Flags: review+
(In reply to Jan Henning [:JanH] from comment #18)
> Besides,
> despite what I've written in the commit message for part 6, I'm actually
> still undecided on whether the activity monitor might not after all be
> helpful in improving our application-background/foreground logic.

So I've looked into it again and it seems my original assessment was correct and somehow along the way I got confused and misremembered things. If we want to do synchronous application-background handling from within on(Activity)Pause(d), we still need additional logic. All the GeckoActivityMonitor could buy us is no longer having to manually call our own onActivityPause/onActivityResume implementation in GeckoApplication.
Attachment #8865225 - Flags: review?(walkingice0204) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Attachment #8865225 - Flags: review?(s.kaspari)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 52

a year ago
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/5cf774f3e277
Part 1 - Call GeckoApp's tab event handler from web apps/custom tabs, too. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/df8187a1ecc1
Part 2 - Move some tab switch functionality to the tab object. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/8dbc1f7ec748
Part 3 - Creating new intents is a job for IntentHelper. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/46b245b80245
Part 4 - Import Tab intent extra key definitions. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/bd4d35fd55f4
Part 5 - Trigger activity switching from GeckoApp. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/e3decf7e8200
Part 6 - Remove manual setting of current activity. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/ff8fe34e775f
Part 7 - Don't ignore last selected tab for Intent.ACTION_MAIN. r=sebastian,walkingice
https://hg.mozilla.org/integration/autoland/rev/b04dbc3924b9
Part 8 - Handle tabs selected while all GeckoApps were in background. r=sebastian,walkingice
Blocks: 1346413
You need to log in before you can comment on or make changes to this bug.