804811 - [browser] device becomes unresponsive after hitting back webpage and forward webpage a couple of times

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P2)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

## Environment :
Otoro phone, build 2012-10-23
Taken from default.xml in b2g-distro: 
* "platform_build" revision= db539a3bd139c93c09b0cd1c3f9396b74d68717c
* "gaia" revision= 0b8ec9b8c16429dc35453dbb7b9342fab3dd18fb 
* "releases-mozilla-aurora" revision= f58edfde05cb708f8a2c440d338f2e429aaf372b
* "gonk-misc" revision= db0c751715f4696515735eb1e0dc5df7a40eb81d

  
## Repro :
1. launch browser on b2g device
2. go to http://people.mozilla.com/~nhirata/html_tp
3. zoom in to be able to read
4. hit the parent directory at the top 
5. go back a web page
6. go forward a web page
7. repeat 5/6 two more times
8. wait til screen blacks out
9. try waking device

## Expected :
1. device wakes

## Actual :
1. device is still blacked out; have to pull out batteries in order to reset the device.

## Note :
found after finding bug 804808

Comment 1

[Ben Francis [:benfrancis]]

As discussed in triage, please renominate if you can find steps to reliably reproduce this as we couldn't easily reproduce.

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I was able to reproduce this again using the steps listed.  Unfortunately, making the phone sleep using the sleep button doesn't cause the bug to occur.  you have to wait for it to do a screen black out.  Going to try to figure out a better set of steps.

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Better steps :
1. launch browser on b2g device
2. go to http://people.mozilla.com/~nhirata/html_tp
3. hit the parent directory at the top 
4. zoom in to be able to read
5. hit the back arrow to go back a page; a good indication is that the lettering looks blurry
6. wait til screen blacks out
7. try waking device

You have to wait for it to time out... you can't just hit the sleep button.  even if you do hit the sleep button, you'll notice it's not responsive at step 6.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Renoming.  I'm able to consistantly repro on both unagi and otoro:

Video : http://www.youtube.com/watch?v=ua0FyT9al7M&feature=youtube_gdata_player

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

another better steps:
1. launch browser on b2g device
2. search something like 'test'
3. click first search result
4. hit the back arrow to go back
5. wait until screen blacks out
6. try waking device

something wrong in nsEventDispatcher::Dispatch. I'm investigating it.

Comment 6

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

shell.js cannot receive the 'sleep-button' event. After tracing in gdb, I found preVisitor.mCanHandle in nsEventDispatcher.cpp:581 is false, so the event cannot  be dispatched successful.

I tried to set mCanHandle = true in gdb, but there is another problem which item->mChild is null in nsEventDispatch.cpp:279.

this is a gecko issue, change Component to General.

Comment 7

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

There is some additional information. When I launch the browser app and try to reproduce this issue, there are only two point changing mCanHandle to false.

Breakpoint 6, nsHTMLInputElement::PreHandleEvent (this=0x49b64f20, aVisitor=...)
    at /home/yurenju/src/B2G/gecko/content/html/content/src/nsHTMLInputElement.cpp:2020
2020    aVisitor.mCanHandle = false;


Breakpoint 2, nsHTMLButtonElement::PreHandleEvent (this=0x47cd6200, aVisitor=...)
    at /home/yurenju/src/B2G/gecko/content/html/content/src/nsHTMLButtonElement.cpp:262
262   aVisitor.mCanHandle = false;

Comment 8

[Alex Keybl [:akeybl]]

Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".

Comment 9

[David Scravaglieri [:scravag]]

Andrew, as it seems to be a gecko issue, could you assign someone please ?

Comment 10

[Doug Turner (:dougt)]

olli, can you take a look at this?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Only if there is step to reproduce which doesn't require any b2g device.

Is something relying on DOM events to always go to chrome code?
That is risky. Bug 329509 won't be fixed in FF18 or FF 19.

But I don't understand how browsing web pages can affect to chrome. I thought web pages
run in a different process than chrome.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

So, is the case that browser app runs in the chrome process and focus is in location bar.
Then for some reason the <input> element which is used for implementing location bar is disabled?

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

And if so, the real fix would be to not run browser in the chrome process, and
probably the easiest fix would be to not disable input element ever.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Or at least when disabling input element, make sure to move focus elsewhere first.

Comment 15

[Doug Turner (:dougt)]

After talking to Olli, it sounds like the platform changes to fix this will be very risky.  I think it might be possible to fix this in gaia code (by confirming, and if it turns out to be the case, not disabling the input elements in chrome).

Comment 16

[Ben Francis [:benfrancis]]

It's completely beyond me how disabling an input element in HTML can prevent the phone from being woken up, but the input element used by the address bar never gets disabled in the browser app so this seems like a red herring.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Hmm, then more information is needed here.

Note 'disabled' in this case means either disabled=true, or user-input: none; or user-input: disabled;

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Ah, not input element, but button element.
There is back button element which gets disabled.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Attachment: b2g only hack

This is horrible, and slows down especially form control elements' event handling
a bit. But this is the safest approach I can think of.

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Feedback from any b2g/gaia developer would be very welcome. Someone needs to test the patch on the
device.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

And even that patch is a bit regression risky (not for Firefox ofc but for gaia/b2g).

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Especially make sure <input type="file"> works still in b2g. (I don't know what kind of UI it has there.)

Comment 23

[Ben Francis [:benfrancis]]

It's going to take me a while to get a Unagi build up and running to test this patch with so if anyone gets the chance to test this before me that would be great.

Comment 24

[Ben Francis [:benfrancis]]

OK, Fernando flashed my device with a build with this patch applied and it seems I can still reproduce the bug :(

Simpler STR:
* Open browser app
* Navigate to any web page with hyperlinks
* Click a hyperlink to enable the back button
* Click the back button to go back and disable the back button
* Wait for the device to go to sleep on its own
* Press the power button to wake up the device

Expected:
* The device wakes up

Actual:
* The event gets eaten by a big disabled input monster and the device is condemned to darkness for eternity.

There are over a hundred occurrences of "disabled" in Gaia so it would be great if we could fix this on the platform side.

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

The patch should fix all the form-element-is-disabled problems; the app in which the element is
won't get the event, but chrome code does get it.

But ok, then the bug is something else than what I thought or b2g doesn't set-up chrome event listener
properly (mozapp/mozbrowser setup in chrome process is wrong?), or the interesting event listener
isn't in chrome but somewhere in non-privileged code.

Comment 26

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

I tried the patch both on b2g-desktop and on a device and it appears to work for me.

I also found a new steps to reproduce (which is shorter):

Better steps :
1. launch browser on b2g device
2. go to http://people.mozilla.com/~nhirata/html_tp
3. hit the parent directory at the top 
4. zoom in to be able to read
5. hit the back arrow to go back a page; a good indication is that the lettering looks blurry
6. hit the home button

- Expected result:
 The app is closed and you see the homescreen

- Actual results:
 Nothing happens

Comment 27

[Ben Francis [:benfrancis]]

Comment on attachment 684017 [details] [diff] [review]
b2g only hack

I can confirm that this patch fixes the bug. The device doesn't slip into a coma when it goes to sleep.

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 684017 [details] [diff] [review]
b2g only hack

Jst, since you reviewed Bug 234455 only about 6.5 years ago, you obviously
remember all this stuff :)
Want to review this über-hackish patch? The idea is to
check if event dispatch would stop on disabled form control element, and if so,
dispatch to chrome instead. A bit slow and and quite horrible, but
I couldn't figure out anything safer for now.

I'll try to figure out how to fix bug 329509, but that will happen
for FF20 at earliest.

Comment 29

[Johnny Stenback (:jst)]

Comment on attachment 684017 [details] [diff] [review]
b2g only hack

+#ifdef MOZ_B2G
+            // Horrible hack for B2G to propage events even from

s/propage/propagate/

r=jst, but yeah, please try to come up with a real fix for this problem after v1!

Comment 30

[Olli Pettay [:smaug][[e-mail]]]

On which branches do we need this? The presshell code is a bit different on beta, so I'd
need to prepare a new patch.

Comment 31

[Johnny Stenback (:jst)]

We need this on beta.

Comment 32

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Fixed typo

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 685708 [details] [diff] [review]
Fixed typo

This applies to aurora and m-c

Comment 34

[Olli Pettay [:smaug][[e-mail]]]

er, looks like that applies cleanly to m-c only.

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

Attachment: for beta, b2g only

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA
User impact if declined: This bug happens
Testing completed (on m-c, etc.): about to land the patch to m-c
Risk to taking this patch (and alternatives if risky): the change is b2g only because it is risky
String or UUID changes made by this patch: NA

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 685708 [details] [diff] [review]
Fixed typo

[Approval Request Comment]
See the comments about beta

Comment 37

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/514b3edf5783

Comment 38

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 685708 [details] [diff] [review]
Fixed typo

Looks like this doesn't need approval.

Comment 39

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/60030e0525d3
https://hg.mozilla.org/releases/mozilla-beta/rev/f003643b1096

Comment 40

[Olli Pettay [:smaug][[e-mail]]]

Filed Bug 816340 to fix this in a less hacky way.