Currently we're guessing which activity is closing because mozChromeEvent:activity-done only tells us who is the activity caller(manifestURL + pageURL). I would like to improve this by having activity-done event for each activity frame. :fabrice said it's possible to do. Could we implement this in mozBrowserAPI? It should be similar as mozbrowserclose event.

Proposal: In ActivityWrapper.wrapMessage, store the innerWindowID or aWindow in the handler, In ActivityRequestHandler, send the innerWindowID or aWindow to ActivityService once postResult/postError is called. In ActivityService use notifyObserver about activity-done to mozBrowserChildPreload with subject=innerWindowID, In mozBrowserChildPreload send mozbrowseractivitydone event out. At the same time remove the observer in shell.js and listen to mozbrowseractivitydone in system app.

That's almost it, Alive. The mistake you made is that ActivityService lives in the parent, but we need to notify from the child process (inner window IDs are only unique per-process). So we need to notify from ActivityRequestHandler.js and from ActivityWrapper.js

(In reply to Fabrice Desré [:fabrice] from comment #2) > That's almost it, Alive. The mistake you made is that ActivityService lives > in the parent, but we need to notify from the child process (inner window > IDs are only unique per-process). So we need to notify from > ActivityRequestHandler.js and from ActivityWrapper.js Thanks, have a WIP!

Almost done? but the comparation in BrowserElementChildPreload doesn't work doc != content.document :/ Don't know why yet.

Add mozbrowseractivitydone event. The event name is bad but I don't have a better idea. Although I have no idea how to write mochitest for web activity...Fabrice, any clue?

Listen to mozbrowser event instead of mozChromeEvent

Comment on attachment 818395 [details] [diff] [review] 927310-gecko.patch Review of attachment 818395 [details] [diff] [review]: ----------------------------------------------------------------- For the tests, we have no activities mochitests yet. Maybe you can write some gaia integration tests? ::: dom/activities/src/ActivityWrapper.js @@ +69,5 @@ > + return; > + } > + let docshell = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) > + .getInterface(Ci.nsIWebNavigation);; > + Services.obs.notifyObservers(docshell, "activity-done", null); Instead of sending the docshell, why not send aWindow.document? Also, you need to remove the two new observers.

(In reply to Fabrice Desré [:fabrice] from comment #7) > Comment on attachment 818395 [details] [diff] [review] > 927310-gecko.patch > > Review of attachment 818395 [details] [diff] [review]: > ----------------------------------------------------------------- > > For the tests, we have no activities mochitests yet. Maybe you can write > some gaia integration tests? OK, but should be another bug. We have a meta bug for gaia system wide integration test and we plan to break down all what we need to write for all cross-app features. Activity definitely is a big part. > > ::: dom/activities/src/ActivityWrapper.js > @@ +69,5 @@ > > + return; > > + } > > + let docshell = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) > > + .getInterface(Ci.nsIWebNavigation);; > > + Services.obs.notifyObservers(docshell, "activity-done", null); > > Instead of sending the docshell, why not send aWindow.document? It's because neither aWindow.document or aWindow.content.document matches content.document in BrowserElementChildPreload...but the process id is the same without doubt between ActivityWrapper and BrowserElementChildPreload. After talking with Patrick(:kk1fff), he says Justin Lebar told him to compare using docShell. See http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#545

v2

Comment on attachment 818475 [details] 927310-gaia.patch I wonder if we should move all the mozChromeEvent events to mozbrowseractivity* events altogether? I know we only allow foreground app to make activity request currently through.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #10) > Comment on attachment 818475 [details] > 927310-gaia.patch > > I wonder if we should move all the mozChromeEvent events to > mozbrowseractivity* events altogether? I know we only allow foreground app > to make activity request currently through. I have a plan to move some more events into mozBrowserAPI but may need to discuss with others. including gUM recording notifications(recording-devices in mozChromeEvent), activitystart(open-app in mozChromeEvent)... Fabrice, any concern about doing this? I think enhancing mozBrowser events this is must-have in haida. Activitystart case doesn't block my work so that's why I don't do it in this bug, but it may matter if one day we have more than one foreground apps at one time. Some day.

Comment on attachment 818869 [details] [diff] [review] 927310-gecko-v2.patch Review of attachment 818869 [details] [diff] [review]: ----------------------------------------------------------------- r- because I don't think this really works. see the comment about notifyObservers(). ::: dom/activities/src/ActivityWrapper.js @@ +70,5 @@ > + } > + Services.obs.removeObserver(observer, "activity-error"); > + Services.obs.removeObserver(observer, "activity-success"); > + let docshell = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) > + .getInterface(Ci.nsIWebNavigation);; nit: double ';' at the end of the line. @@ +72,5 @@ > + Services.obs.removeObserver(observer, "activity-success"); > + let docshell = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) > + .getInterface(Ci.nsIWebNavigation);; > + Services.obs.notifyObservers(docshell, "activity-done", > + { success: (aTopic == 'activity-success' ? true : false) }); the 3rd argument of notifyObservers is a string (see https://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIObserverService.idl#61), not a JS object.

v3: correct success attr.

Comment on attachment 819093 [details] [diff] [review] 927310-gecko-v3.patch Review of attachment 819093 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nit addressed ::: dom/browser-element/BrowserElementChildPreload.js @@ +288,5 @@ > case 'ask-parent-to-rollback-fullscreen': > sendAsyncMsg('rollback-fullscreen'); > break; > + case 'activity-done': > + sendAsyncMsg('activitydone', { success: (data == 'activity-success' ? true : false) }); nit: { success: (data == 'activity-success') }

checkin patch

master https://github.com/mozilla-b2g/gaia/commit/f6dd2c5e2d1a1d79f0689a174227600028df2007