1076613 - Separate the homescreen related thing from appwindowmanager as homescreenwindowmanager

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

To finish Bug #1074040, we have to support multiple homescreen app in current appwindowmanager. We should separate the homescreenwindowmanager from appwindowmanager and let it be aware of which homescreen app should be launched. So, the multiple homescreen app will be encapsulated in homescreenwindowmanager. 

In bug 1074040, we will create another homescreen launcher and homescreenwindowmanager for TV and use build script to swap it at build time. This TV homescreenwindowmanager dynamic loads the second homescreen launcher and determine which homescreen app should be shown based on the event type and current active homescreen.

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: HomescreenWindowManager for phone (shifted from bug 1074040)

Sorry, I shift the feedback flag from bug 1074040 here. The PR is totally the same.

Comment 2

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8498616 [details] [review]
HomescreenWindowManager for phone (shifted from bug 1074040)

LGTM!

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: HomescreenWindowManager for phone, test included.

The patch is ready but not sure about TBPL result. Wait for result before setting review.

Comment 4

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8498784 [details] [review]
HomescreenWindowManager for phone, test included.

Hi Alive,

Please review this patch. The changes in system/js/ are almost the same, except the one we found at gij, the logic of getActiveApp(). The TPBL gives almost green but one intermittent error. I feel that may be fine. Please review this patch and give me some feedback. Thanks.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8498784 [details] [review]
HomescreenWindowManager for phone, test included.

I think this is fine it no test is broken; much thanks to take your time on refactoring requested by me, great done!

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/bec2d31f589fbf0adaf43811214acf86207344ba

Thank you alive. This is a good start. We may follow similar path in the future if we need to extend system app.

Comment 7

[Dale Harvey (:daleharvey)]

Comment on attachment 8498784 [details] [review]
HomescreenWindowManager for phone, test included.

Needs a branch patch but its a very minor conflict

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Originally broken implementation
[User impact] if declined: Extremely slow transition to homescreen from landscape orientated app, this is a blocker in https://bugzilla.mozilla.org/show_bug.cgi?id=1072531
[Testing completed]: Patch looks to have a good amount of unit testing, and has settled on master
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 8

[Dale Harvey (:daleharvey)]

Pushed to try

John, before I uplift is there anything else I should worry about / changes that this may effect?

Comment 10

[Dale Harvey (:daleharvey)]

Also needinfoing myself to remind myself to land

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Please also uplift Bug 1078193. Bug 1078193 changes the sequence of event listener and this makes the interception of homescreen app launch[1] work. Although we don't have a way to launch verticalhome currently, Bug 1078193 may help us when we have a homescreen which is able to launch or with system message.


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/homescreen_window_manager.js#L77-L84

Comment 12

[John Hu [:johnhu][:johu][:醬糊小弟]]

Change milestone for tracking the issues related to connected device.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #11)
> Please also uplift Bug 1078193.

You'll need to request approval in that bug if you want it uplifted.

Comment 14

[Dale Harvey (:daleharvey)]

Didnt mean to take this off needinfo, have the branch patch but not landing until 2.1 is green

Comment 15

[Dale Harvey (:daleharvey)]

Attachment: Branch patch for 2.1

Comment 16

[Dale Harvey (:daleharvey)]

Finally for a try run that was intermittent but had all suites green https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=e122a00ac717

John if you are wanting the other patch uplifted as well you should ask for approval seperately as it looks like that only effects the ordering in which you can replace the homescreen right?

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/cce25394be83efea8aa672b16261f4c8a9aac1a1

Comment 18

[John Hu [:johnhu][:johu][:醬糊小弟]]

Thanks Dale, I will do that. And yes, it wants to prevent user to launch another homescreen app instance.