Currently the way we expose browser API to content is XPCNativeWrapper.unwrap(self._frameElement)[name] = function() { // ... }; This is ugly. We should make it a JS-implemented webidl

My draft patch currently defines a [NoInterfaceObject] in BrowserElement.webidl and HTMLIFrameElement implements BrowserElement. Then nsGenericHTMLFrameElement inherits from nsBrowserElement which will proxy the API to BrowserElementParent.js via nsIBrowserElementAPI. bz, I have one question. Currently we expose browser API only if the iframe has a mozbrowser attribute before it get bind to the DOM tree. It also checks if the iframe is a widget and expose only a subset of the API. Could the new dom binding do that? It seems the Func extended attribute only cares about the global object.

> It seems the Func extended attribute only cares about the global object. The problem for the use case you describe is that Web IDL attributes are defined on the prototype, not on instances. Things that want to expose different APIs are expected to use different prototypes. In addition to that, Web IDL assumes the set of exposed APIs on an object does not change during the object's lifetime, quite purposefully: the other way lies madness. I'm not sure how to reconcile that with the mutability of @mozbrowser on <iframe>. :(

One option for exposing things on instances would be with a resolve hook, I guess. But then you have to manually define the functions(s) involved...

How about turn browser API to be defined on the prototype instead? So an app with "browser" permission could see browser API exposed on <iframe> but calling them would throw unless the <iframe> is a mozbrowser?

That would certainly be easy to implement. So if we're ok with that setup, sounds good to me.

(In reply to Kan-Ru Chen [:kanru] from comment #4) > How about turn browser API to be defined on the prototype instead? So an app > with "browser" permission could see browser API exposed on <iframe> but > calling them would throw unless the <iframe> is a mozbrowser? We talked to Kan-Ru today about this. Perhaps it's time to start disentangling this from <iframe>. But for the record, we should not expose these methods on the HTMLIframeElement's prototype, because those will be visible to normal Web content.

Not if we exposed conditionally per comment 4.

(In reply to Boris Zbarsky [:bz] from comment #7) > Not if we exposed conditionally per comment 4. But we don't know what kind of iframe we're dealing with when the object is created, right?

We do know whether we're in an environment that has the "browser" permission. Which the Web at large does not.

(In reply to Boris Zbarsky [:bz] from comment #9) > We do know whether we're in an environment that has the "browser" > permission. Which the Web at large does not. Are you suggesting that in such an environment we put all of these methods on the prototype of all HTMLIframeElement, and throw if the element is not really a mozbrowser?

More precisely, Kan-Ru is suggesting that. I'm just saying that seems fine to me.

Heh, fair enough. :-) That seems like a bad solution to me, tbh. But I don't have a better suggestion. :( FWIW I and Kan-Ru discussed doing a new <webview> element mostly for this same reason. But I'm not sure what the timelines on that idea look like.

I think this is the short term solution to make mozbrowser better and easier to implement <webview> in the future. Sadly <iframe mozbrowser> was expected to be a short term solution too...

This patch changes the semantic of "embed-widgets" to reflect what was discussed. Junior, could you check that this doesn't break your use case.

Comment on attachment 8512401 [details] [diff] [review] Part 5. Widget should only require embed-widgets permission. Review of attachment 8512401 [details] [diff] [review]: ----------------------------------------------------------------- A widget should not be exposed to a privileged app by limited browser API. Since both "browser" and "embed-widgets" permission are privileged, IMO it should throw if widget embedders with "browser" and "embed-widgets" invoke a limited browser API. And tests should be adapted.

Comment on attachment 8512395 [details] [diff] [review] Part 1. Add BrowserElement.webidl and stub implementation Review of attachment 8512395 [details] [diff] [review]: ----------------------------------------------------------------- Kanru, maybe we could use this opportunity to use promises instead of DOMRequests? I know this will impact gaia quite a lot...

Comment on attachment 8512398 [details] [diff] [review] Part 3. Make BrowserElementParent implement nsIBrowserElementAPI and use it. Review of attachment 8512398 [details] [diff] [review]: ----------------------------------------------------------------- That looks mostly good to me. I'm not giving r+ until we agree on the 'return undefined' vs throw in a couple of methods. ::: dom/browser-element/BrowserElementParent.jsm @@ +540,3 @@ > this._sendAsyncMsg('set-visible', {visible: visible}); > this._frameLoader.visible = visible; > + }), nit: add new line @@ +547,5 @@ > + }), > + > + getActive: function() { > + if (!this._isAlive()) { > + return undefined; hm, is that correct? Should we rather throw? @@ +558,1 @@ > this._sendAsyncMsg("send-mouse-event", { I guess you tested (I didn't!) but is |this| binded to the right object there? @@ +826,5 @@ > }, > > + setInputMethodActive: function(isActive) { > + if (!this._isAlive()) { > + return undefined; here too, I'm not convinced returning undefined is the right thing.

Comment on attachment 8512399 [details] [diff] [review] Part 4. Rename BrowserElementParent.jsm to BrowserElementParent.js. Review of attachment 8512399 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/browser-element/BrowserElementParent.js @@ +716,5 @@ > + aOffset, aCount) { > + this.extListener.onDataAvailable(aRequest, aContext, aInputStream, > + aOffset, aCount); > + }, > + QueryInterface: XPCOMUtils.generateQI([Ci.nsIStreamListener, I guess it was there before, but there's a trailing whitespace.

Comment on attachment 8512401 [details] [diff] [review] Part 5. Widget should only require embed-widgets permission. Review of attachment 8512401 [details] [diff] [review]: ----------------------------------------------------------------- Fine, but why is this part of this bug?

(In reply to Fabrice Desré [:fabrice] from comment #25) > Comment on attachment 8512401 [details] [diff] [review] > Part 5. Widget should only require embed-widgets permission. > > Review of attachment 8512401 [details] [diff] [review]: > ----------------------------------------------------------------- > > Fine, but why is this part of this bug? Because the test was broken by patches. Note this patch needs some modification according to comment 21. We will make <iframe mozbrowser mozwidget> have the same set of APIs no matter the embedder has "browser" permission or not.

(In reply to Fabrice Desré [:fabrice] from comment #22) > Comment on attachment 8512395 [details] [diff] [review] > Part 1. Add BrowserElement.webidl and stub implementation > > Review of attachment 8512395 [details] [diff] [review]: > ----------------------------------------------------------------- > > Kanru, maybe we could use this opportunity to use promises instead of > DOMRequests? I know this will impact gaia quite a lot... I didn't want to make any API changes in this bug. Maybe this could be done after DOMRequests gain the "then" method?

(In reply to Fabrice Desré [:fabrice] from comment #23) > Comment on attachment 8512398 [details] [diff] [review] > Part 3. Make BrowserElementParent implement nsIBrowserElementAPI and use it. > > Review of attachment 8512398 [details] [diff] [review]: > ----------------------------------------------------------------- > > That looks mostly good to me. I'm not giving r+ until we agree on the > 'return undefined' vs throw in a couple of methods. > > ::: dom/browser-element/BrowserElementParent.jsm > @@ +540,3 @@ > > this._sendAsyncMsg('set-visible', {visible: visible}); > > this._frameLoader.visible = visible; > > + }), > > nit: add new line > > @@ +547,5 @@ > > + }), > > + > > + getActive: function() { > > + if (!this._isAlive()) { > > + return undefined; > > hm, is that correct? Should we rather throw? > > @@ +558,1 @@ > > this._sendAsyncMsg("send-mouse-event", { > > I guess you tested (I didn't!) but is |this| binded to the right object > there? Yes, the inner function is called by |fn.apply(this, arguments);| > @@ +826,5 @@ > > }, > > > > + setInputMethodActive: function(isActive) { > > + if (!this._isAlive()) { > > + return undefined; > > here too, I'm not convinced returning undefined is the right thing. The original code uses |return;| which is equal to |return undefined;| I was just making it explicit. There are several other places uses |return;| instead of throw, like in defineNoReturnMethod. IMO change it to throw is good as long as Gaia is fixed or not affected.

(In reply to Kan-Ru Chen [:kanru] from comment #27) > (In reply to Fabrice Desré [:fabrice] from comment #22) > > Comment on attachment 8512395 [details] [diff] [review] > > Part 1. Add BrowserElement.webidl and stub implementation > > > > Review of attachment 8512395 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > Kanru, maybe we could use this opportunity to use promises instead of > > DOMRequests? I know this will impact gaia quite a lot... > > I didn't want to make any API changes in this bug. Maybe this could be done > after DOMRequests gain the "then" method? I found bug 839838 has been landed for a while. We could do it in a follow-up or in this bug if Gaia could change to use the Promise pattern while this bug is being reviewed.

(In reply to Kan-Ru Chen [:kanru] from comment #26) > > Fine, but why is this part of this bug? > > Because the test was broken by patches. Note this patch needs some > modification according to comment 21. We will make <iframe mozbrowser > mozwidget> have the same set of APIs no matter the embedder has "browser" > permission or not. ok! > I didn't want to make any API changes in this bug. Maybe this could be done > after DOMRequests gain the "then" method? fine for me.

(In reply to Kan-Ru Chen [:kanru] from comment #29) > I found bug 839838 has been landed for a while. We could do it in a > follow-up or in this bug if Gaia could change to use the Promise pattern > while this bug is being reviewed. I think that's very unlikely :) The Browser API is widely used in Gaia. One possibility is that we move to using Promises when implementing a (preferably standardised [1]) <webview> element. That would make the transition in Gaia less risky as we can transition a few parts at a time. (In reply to Kan-Ru Chen [:kanru] from comment #13) > I think this is the short term solution to make mozbrowser better and easier > to implement <webview> in the future. Glad to hear this will make <webview> simpler! > Sadly <iframe mozbrowser> was expected > to be a short term solution too... Hah, yes. 1. http://benfrancis.github.io/webview/

Comment on attachment 8512395 [details] [diff] [review] Part 1. Add BrowserElement.webidl and stub implementation Shouldn't we use TouchEvent::PrefEnabled instead of an incorrect copy of it? The IDL for getVisible says null is never returned, but the implementation returns null. Same for download, purgeHistory, getScreenshot, getCanGoBack, getCanGoForward, getContentDimensions, setInputMethodActive. >+ CheckPermissions="browser", >+ CheckPermissions="input-manage"] I really doubt this does what you want. Have you looked at the generated code? I don't think we should put nsBrowserElement in the mozilla::dom namespace. Perhaps the mozilla namespace? I don't feel that strongly about this part.

(In reply to Boris Zbarsky [:bz] from comment #32) > Comment on attachment 8512395 [details] [diff] [review] > Part 1. Add BrowserElement.webidl and stub implementation > > Shouldn't we use TouchEvent::PrefEnabled instead of an incorrect copy of it? ok. > The IDL for getVisible says null is never returned, but the implementation > returns null. > > Same for download, purgeHistory, getScreenshot, getCanGoBack, > getCanGoForward, getContentDimensions, setInputMethodActive. They are only stubs in the first patch. In patch 2 they only return null when they throw. I could update patch 1 to always throw when fixing following > >+ CheckPermissions="browser", > >+ CheckPermissions="input-manage"] > > I really doubt this does what you want. Have you looked at the generated > code? I thought that works from the impression of reading the doc. But the yes, the generated code only test the last CheckPermissions attribute. > I don't think we should put nsBrowserElement in the mozilla::dom namespace. > Perhaps the mozilla namespace? I don't feel that strongly about this part. ok.

I'd opened a gaia bug to switch all browser API access in system app to .then() format. https://bugzilla.mozilla.org/show_bug.cgi?id=1092897

Comment on attachment 8516498 [details] [diff] [review] Part 1. Add BrowserElement.webidl and stub implementation v2 You could also "using namespace mozilla::dom;" in the .cpp and avoid those extra "dom::" prefixes. Either way. r=me

Comment on attachment 8516500 [details] [diff] [review] Part 2. Add nsIBrowserElementAPI.idl and implement nsBrowserElement. v2 Please file a bug on comm-central to include this in their manifests. I assume there's a reason for the set/get pair for "active" instead of using an attribute, both here and in the webidl? Might be good to document that reason, both places. Do we care about sendMouseEvent only being able to do integer CSS pixels? >+++ b/dom/html/nsBrowserElement.cpp >+class nsBrowserElement::BrowserShownObserver : public nsIObserver >+ void RemoveObserver(); This doesn't need to be public. >+ nsBrowserElement* mBrowserElement; // owned That would normally mean that this is an owning reference. But that's not the case here. What you're trying to say is that this is a weak reference, because mBrowserElement owns us. But _that_'s not right either, because it just holds a ref to us. Which means we can outlive mBrowserElement. Which means we need to null it out when that object dies and null-check it various places, no? And fix the comment to make it clear what it means. >+nsBrowserElement::nsBrowserElement() >+ mObserver = do_QueryObject(observer); Why is a QI needed there? > nsBrowserElement::SendTouchEvent(const nsAString& aType, This needs to throw if any of the sequence lengths doesn't match aCount, right? >+nsBrowserElement::Download(JSContext* aCx, >+ if (!ToJSValue(aCx, aOptions, &options)) { >+ aRv.Throw(NS_ERROR_OUT_OF_MEMORY); Need to return nullptr there too, no? That said, this will pass a content-compartment object to mBrowserElementAPI. Is that really what we want? I doubt it. And as a more general question, if we plan to implement mBrowserElementAPI in JS, did we consider making it a Web IDL callback interface matching the BrowserElement Web IDL interface? That would help with a lot of these impedance mismatches, I think.. > nsBrowserElement::GetScreenshot(uint32_t aWidth, >+ aWidth, aHeight, aMimeType.WasPassed() ? aMimeType.Value() : EmptyString(), Why not give mimeType a default value of "" in the IDL, and avoid needing this mess here? >+++ b/dom/html/nsBrowserElement.h >+ virtual bool IsOwnFrameLoader(nsIFrameLoader* aFrameLoader) = 0; Shouldn't this be protected? Also, can we really not do something better than broadcasting to all observers and then checking that the notification is for our frameloader? Followup bug on a better setup is probably OK if needed. >+ nsIFrameLoader* mFrameLoaderPtr; // owned No, this makes no sense to me at all. At the very least, please clearly document the ownership model, but until proven otherwise I don't believe this is safe.

Comment on attachment 8516501 [details] [diff] [review] Part 3. Make BrowserElementParent implement nsIBrowserElementAPI and use it. v2 r=me I guess, but I'm not really all that qualified to review this stuff, honestly.

Comment on attachment 8516502 [details] [diff] [review] Part 4. Rename BrowserElementParent.jsm to BrowserElementParent.js. This diff seems oddly full of changes, for a file rename. Was it generated with hg mv? If not, it should be. r=me modulo that.

Comment on attachment 8519862 [details] [diff] [review] Part 2. Add nsIBrowserElementAPI.idl and implement nsBrowserElement. v3 >+ // Weak reference to the browser element Please document who's responsible for nulling it out? > nsBrowserElement::BrowserShownObserver::Observe(nsISupports* aSubject, I don't understand the changes made to this function. What is the goal there? r- for this part for now, until that's explained. >+ virtual already_AddRefed<nsFrameLoader> GetFrameLoader() MOZ_OVERRIDE Why do you need this, if you already pulled it in via "using"? The rest looks ok, though I still think it would be better with the callback interface approach. Followup is ok for that.

(In reply to Boris Zbarsky [:bz] from comment #47) > Comment on attachment 8519862 [details] [diff] [review] > Part 2. Add nsIBrowserElementAPI.idl and implement nsBrowserElement. v3 > > >+ // Weak reference to the browser element > > Please document who's responsible for nulling it out? OK. nsBrowserElement has a reference to the BrowserShowObserver. nsBrowserElement's destructor is responsible to null out (via RemoveObserver()) the reference. > > nsBrowserElement::BrowserShownObserver::Observe(nsISupports* aSubject, > > I don't understand the changes made to this function. What is the goal > there? r- for this part for now, until that's explained. Because the browser element API needs the frameloader to initialize. We still use the observer to get notified when the frameloader is created. So we check if the frameloader created is ours, then initialize the browser element API. > >+ virtual already_AddRefed<nsFrameLoader> GetFrameLoader() MOZ_OVERRIDE > > Why do you need this, if you already pulled it in via "using"? nsIFrameLoaderOwner defines two GetFrameLoader() overloads. One is XPCOM style interface, the other one is C++ only. readonly attribute nsIFrameLoader frameLoader; [noscript, notxpcom] alreadyAddRefed_nsFrameLoader GetFrameLoader(); > The rest looks ok, though I still think it would be better with the callback > interface approach. Followup is ok for that. I think callback interface is better too. But I don't want to make too radical change to how browser element API is initialized in this patch. It was designed to interact with OOP and Nuwa process creation and changing it might be a mess. I'll do the cleanup in a followup.

Comment on attachment 8519862 [details] [diff] [review] Part 2. Add nsIBrowserElementAPI.idl and implement nsBrowserElement. v3 > OK. nsBrowserElement has a reference to the BrowserShowObserver. Yes, I read the code. The idea is to make it so other people don't have to. ;) > So we check if the frameloader created is ours Ah, I see now. I just misread this part. OK. r=me > nsIFrameLoaderOwner defines two GetFrameLoader() overloads. Sure, but so what? "using" pulls them both in, I'd think.

(In reply to Boris Zbarsky [:bz] from comment #49) > Comment on attachment 8519862 [details] [diff] [review] > Part 2. Add nsIBrowserElementAPI.idl and implement nsBrowserElement. v3 > > > OK. nsBrowserElement has a reference to the BrowserShowObserver. > > Yes, I read the code. The idea is to make it so other people don't have to. > ;) Right, I wrote what I intend to put in the comment :) > > So we check if the frameloader created is ours > > Ah, I see now. I just misread this part. OK. r=me > > > nsIFrameLoaderOwner defines two GetFrameLoader() overloads. > > Sure, but so what? "using" pulls them both in, I'd think. using pulls them both in, now GetFrameLoader() is ambiguous because nsBrowserElement also has GetFrameLoader(). I had to explicit redefine GetFrameLoader() to explicit choose nsElementFrameLoaderOwner::GetFrameLoader()

Ah, I see. That might also be worth a comment.

carryover r+

Rebase and only check OwnerIsWidget once. Check OwnerIsWidget once because APIs expect to return DOMRequest do not throw when they aren't in the DOM tree. (maybe we should throw instead?)

remote: https://hg.mozilla.org/integration/b2g-inbound/rev/e81aa95ac8d3 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/8505276285fa remote: https://hg.mozilla.org/integration/b2g-inbound/rev/f55ff90f2b6f remote: https://hg.mozilla.org/integration/b2g-inbound/rev/1b55f80e1ca6 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/7037d2b40d39 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/fb6a60700803 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/a3604ab2b3ca remote: https://hg.mozilla.org/integration/b2g-inbound/rev/920775ddfb85

https://hg.mozilla.org/integration/b2g-inbound/rev/6ccbc46c33db backouted

Relanded, try https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0253711e9ef2 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/78aacc35b4e7 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/c7c9170ab299 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/8ecfc1f41ddc remote: https://hg.mozilla.org/integration/b2g-inbound/rev/e4f1f09f150f remote: https://hg.mozilla.org/integration/b2g-inbound/rev/b3ff4ce20e67 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/59a47fafaccf remote: https://hg.mozilla.org/integration/b2g-inbound/rev/61fc517d0336 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/b1a6ecfca674

https://hg.mozilla.org/mozilla-central/rev/78aacc35b4e7 https://hg.mozilla.org/mozilla-central/rev/c7c9170ab299 https://hg.mozilla.org/mozilla-central/rev/8ecfc1f41ddc https://hg.mozilla.org/mozilla-central/rev/e4f1f09f150f https://hg.mozilla.org/mozilla-central/rev/b3ff4ce20e67 https://hg.mozilla.org/mozilla-central/rev/59a47fafaccf https://hg.mozilla.org/mozilla-central/rev/61fc517d0336 https://hg.mozilla.org/mozilla-central/rev/b1a6ecfca674

This broke 2 integration tests in gij 3: https://tbpl.mozilla.org/?tree=B2g-Inbound&showall=1&rev=b1a6ecfca674 Can you back out or do a quick fix?

(In reply to Gregor Wagner [:gwagner] from comment #58) > This broke 2 integration tests in gij 3: > https://tbpl.mozilla.org/?tree=B2g-Inbound&showall=1&rev=b1a6ecfca674 > > Can you back out or do a quick fix? The log is not very helpful... How do I reproduce it locally? Or if you can, could you provide the JavaScript Error message or stack?

The status bar on master is pretty broken right now and it seems that this patch caused some/all of the regression. Kanru can you back this out please?

cc'ing sheriffs. kweirso, tomcat, are you available also to help for backout?

Bug 1098145 needs to be backed out first in order to backout this cleanly. bug 1096778 also needs to be backed out from common-central.

All of this bug's commits backed out in https://hg.mozilla.org/mozilla-central/rev/d045286cff6e Bug 1098145 was backed out so I could get this backed out. I'll back out bug 1096778 once my comm-central clone finishes cloning.

And the comm-central patch is now backed out.

I'm seeing build failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=645273&repo=mozilla-central on the mozilla-central backouts. I clobbered mozilla-central and am retriggering the failed builds to see if that helps, but I'm going to have to run before I'll have a chance to see if they've built successfully. Hopefully that's all this takes.

(In reply to Wes Kocher (:KWierso) from comment #65) > I'm seeing build failures like > https://treeherder.mozilla.org/ui/logviewer.html#?job_id=645273&repo=mozilla- > central on the mozilla-central backouts. I clobbered mozilla-central and am > retriggering the failed builds to see if that helps, but I'm going to have > to run before I'll have a chance to see if they've built successfully. > Hopefully that's all this takes. Thanks Wes!

Clobbered retriggers still failing, here's my attempt at fixing this mess: remote: https://hg.mozilla.org/mozilla-central/rev/925353f53fd3 ^ Backout of my original backout that folded all of the individual backouts into a single commit. (Kanru guesses maybe | hg qbackout -s | doesn't deal with renames very nicely. With that un-backed out, everything should be in its original state. Here's individual backouts of each commit (still using | hg qbackout |, but without the -s flag to fold them all together): remote: https://hg.mozilla.org/mozilla-central/rev/3f6dd413b418 remote: https://hg.mozilla.org/mozilla-central/rev/fa6d6ca97c39 remote: https://hg.mozilla.org/mozilla-central/rev/4efb2b966b33 remote: https://hg.mozilla.org/mozilla-central/rev/cca85825b028 remote: https://hg.mozilla.org/mozilla-central/rev/a1aedd5a059f remote: https://hg.mozilla.org/mozilla-central/rev/0dd0ef8ec041 remote: https://hg.mozilla.org/mozilla-central/rev/f0f64e56694f remote: https://hg.mozilla.org/mozilla-central/rev/2a292d225ec0 Hopefully that's enough.

So it turns out the failure is not caused by this bug. I guess I could reland?

(In reply to Kan-Ru Chen [:kanru] from comment #68) > So it turns out the failure is not caused by this bug. I guess I could > reland? Can you test a build with it and stress-test the status-bar and the rocketbar? Or maybe give QA a build and they can help out? I am definitely fine with landing if there are no new regressions.

remote: https://hg.mozilla.org/integration/b2g-inbound/rev/6ccc4cc5d2d2 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/da57faca25cf remote: https://hg.mozilla.org/integration/b2g-inbound/rev/21a12d4f0f26 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/a0374263ed33 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/d64d706592af remote: https://hg.mozilla.org/integration/b2g-inbound/rev/199b84b1c0f6 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/6bdda743c0b4 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/0a6a9c3a6184 remote: https://hg.mozilla.org/integration/b2g-inbound/rev/389b2964076b

Yes, please make sure the following bug does not resurface when relanding: STR 1. Reboot the phone and wait until radio signal displays in status bar 2. Activate airplane mode and wait until the status bar displays the airplane icon 3. Launch an app Expected result Airplane mode is still displayed in status bar. Actual result Status bar is frozen, airplane mode is not displayed. If you go back to the homescreen, the status bar is in the right state. See video for details: http://mzl.la/1v9jh4y

https://hg.mozilla.org/mozilla-central/rev/6ccc4cc5d2d2 https://hg.mozilla.org/mozilla-central/rev/da57faca25cf https://hg.mozilla.org/mozilla-central/rev/21a12d4f0f26 https://hg.mozilla.org/mozilla-central/rev/a0374263ed33 https://hg.mozilla.org/mozilla-central/rev/d64d706592af https://hg.mozilla.org/mozilla-central/rev/199b84b1c0f6 https://hg.mozilla.org/mozilla-central/rev/6bdda743c0b4 https://hg.mozilla.org/mozilla-central/rev/0a6a9c3a6184