853759 - [activities] fire postError when unloading the app before we have fired anything

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

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

+++ This bug was initially created as a clone of Bug #846850 +++

See bug 846850.
This bug is to do postError when an activity is removed/unloaded if it never do postError/postSuccess before it's removed.

With this, any app in gaia doesn't need to watch visibility state to do postError by theyself, which is a very confusing behavior in multiple activities case.

Fabrice, if you could take this I'd appreciate.

Comment 1

[hanj.kim25]

(In reply to Alive Kuo [:alive] from comment #0)

Hi Fabrice, 

Would you be able to reply to the comment above?

Thanks!

Comment 2

[[:fabrice] Fabrice Desré]

I just had no time to look at this, sorry.

Comment 3

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

Let's find someone else to help.

Comment 4

[Wayne Chang [:wchang]]

Hi Alive,
Does this bug still block 846262, which looks pretty bad but already resolved fixed?

If this bug doesnt block 846262, what user impact does this bug itself have on the user?

Comment 5

[Kai-Chih Hu [:khu]]

My understanding is with this fixed, apps could be aware of events when other apps are removed/changed. Please correct me if I am wrong. In this case, it looks like if there have some strong relationship/activities between 2 apps, and one of them is removed, another may not know that, and it might cause some problems, am I right?

Comment 6

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

This blocks bug 846850 and bug 846850 blocks bug 880588.
Bug 846262 is just a workaround because of this root cause. The correct way is to do bug 880588.

Without this, the app who creates the activity needs to postError by themselves, which is an incorrect behavior, and therefore system couldn't do bug 880588 - set the visibility on inline activities correctly. We couldn't avoid to remove all inline activities to prevent bug 846262 happens.

Comment 7

[[:fabrice] Fabrice Desré]

Attachment: patch

I'm not sure this covers all the uses cases, but this patch makes sure that when a window is closed, we fire a postError() with a "ActivityCanceled" message if nothing was sent before.

Comment 8

[Wayne Chang [:wchang]]

Triage - per comment 6 and 880588 is a blocker for partner.

Comment 9

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

I'd tested this patch. It works fine for me. \O/

Comment 10

[Alex Keybl [:akeybl]]

Mounir, would you mind finishing this bug off while Fabrice is out? leo+ blocker :)

Comment 11

[Mounir Lamouri (:mounir)]

After discussing with Andrew it seems that this bug isn't particularly urgent and it can wait for Fabrice to be back from vacations. Given that he already worked on this, the cost of context switch isn't worth it.

Comment 12

[[:fabrice] Fabrice Desré]

Attachment: patch

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

r+ if you use inner-window-destroyed instead of an event.

Comment 14

[[:fabrice] Fabrice Desré]

Attachment: patch v2

Now using inner-window-destroyed.

Comment 15

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/projects/birch/rev/57b2db8bb3f1

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57b2db8bb3f1

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/ae87d831f048

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/ae87d831f048

Comment 19

[Jason Smith [:jsmith]]

This patch broke web activities in Gaia per bug 892708 and bug 892573.

Can someone confirm my analysis and if I'm right, back this patch out to get the smoke tests passing again?

Comment 20

[Jason Smith [:jsmith]]

Apparently we now know this was not the bug that caused the regression.

Comment 21

[dkumar]

Can you please provide steps to verify this fix? - so that we can perform blackbox testing