1173180 - "TypeError: this.selectedTab is null" in test logs before startup hang

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Geoff Brown [:gbrown]]

Bug 1054292 collects Android mochitest failures caused by timeouts. Logcats for most (all?) recent failures reported there appear to show a browser startup hang. Ignoring log messages from the add-on updater and health reporter, one of the last messages seen in all those logcats is:

14:06:03     INFO -  06-05 13:59:22.990 E/GeckoConsole(  619): [JavaScript Error: "TypeError: this.selectedTab is null" {file: "chrome://browser/content/browser.js" line: 1859}]

http://hg.mozilla.org/mozilla-central/annotate/3d11cb4f31b9/mobile/android/chrome/content/browser.js#l1859


I don't see this error in logcats for successful mochitest runs.

Comment 1

[Geoff Brown [:gbrown]]

Failure in more context:

4:06:03     INFO -  06-05 13:59:20.850 I/GeckoConsole(  619): Locale:OS: en-US
14:06:03     INFO -  06-05 13:59:20.880 I/GeckoConsole(  619): New OS locale.
14:06:03     INFO -  06-05 13:59:21.010 I/GeckoConsole(  619): Default intl.accept_languages = en-US, en
14:06:03     INFO -  06-05 13:59:21.060 I/GeckoConsole(  619): Setting intl.accept_languages to en-us,en
14:06:03     INFO -  06-05 13:59:21.110 W/GeckoEventDispatcher(  619): No listeners for Pref:Change
14:06:03     INFO -  06-05 13:59:22.190 I/GeckoConsole(  619): Sending snapshot message.
...
14:06:03     INFO -  06-05 13:59:22.609 I/GeckoDisplayPort(  619): Set strategy VelocityBiasStrategy mult=2.0, threshold=5.1200004, reverse=0.2, dangerBaseX=1.0, dangerBaseY=1.0, dangerIncrX=0.0, dangerIncrY=0.0
...
14:06:03     INFO -  06-05 13:59:22.990 E/GeckoConsole(  619): [JavaScript Error: "TypeError: this.selectedTab is null" {file: "chrome://browser/content/browser.js" line: 1859}]

vs typical success:

06-09 15:21:18.318   629   640 I GeckoConsole: Locale:OS: en-US
06-09 15:21:18.428   629   640 I GeckoConsole: New OS locale.
06-09 15:21:18.649   629   640 I GeckoConsole: Default intl.accept_languages = en-US, en
06-09 15:21:18.728   629   640 I GeckoConsole: Setting intl.accept_languages to en-us,en
06-09 15:21:18.738   629   640 W GeckoEventDispatcher: No listeners for Pref:Change
06-09 15:21:18.778   629   629 D GeckoToolbar: onTabChanged: TITLE
06-09 15:21:18.828   629   629 D GeckoBrowserApp: BrowserApp.onTabChanged: 0: TITLE
06-09 15:21:19.781   629   640 I GeckoConsole: Sending snapshot message.
...
06-09 15:21:21.528   629   638 D dalvikvm: Trying to load lib /data/data/org.mozilla.fennec/lib/libmozglue.so 0x40518dd8
06-09 15:21:21.538   629   638 D dalvikvm: Shared lib '/data/data/org.mozilla.fennec/lib/libmozglue.so' already loaded in same CL 0x40518dd8
...
06-09 15:21:23.118   629   640 D GeckoTabs: handleMessage: Tab:Added
06-09 15:21:23.428   629   640 D GeckoTabs: handleMessage: Content:StateChange
06-09 15:21:23.438   629   629 D GeckoToolbar: onTabChanged: START
06-09 15:21:23.438   629   629 D GeckoBrowserApp: BrowserApp.onTabChanged: 0: START

Comment 2

[Geoff Brown [:gbrown]]

:margaret -- I wonder if you might be able to offer any insight?

Comment 3

[:Margaret Leibovic]

If the line numbers are correct, this would point to this viewport logic:
http://hg.mozilla.org/mozilla-central/annotate/e101c589c242/mobile/android/chrome/content/browser.js#l1859

Maybe we should just add a null check? But I wouldn't want to cause more problems if we fail to update the viewport size.

kats, do you know if it's expected that we wouldn't have a selected tab when we receive this Viewport:FixedMarginsChanged message?

Comment 4

[Kartikaya Gupta (email:[e-mail])]

I think adding a null check is fine - it shouldn't cause any issues. However I doubt it will fix the startup hang either; a JS exception at that point should just abort the handling of the FixedMarginsChanged message and continue with the usual browser startup. It may be that the hang and the exception both have the same underlying root cause which is something else. Still, feel free to add the null check if you want to eliminate that as a cause.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

FWIW, selectedTab being null before it gets initialized is a common problem in Java land and we always have null checks.

Comment 6

[Geoff Brown [:gbrown]]

Attachment: add selectedTab null check

Thanks everyone!

Best to include the gViewportMargins setting in the if, or not?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8620980 [details] [diff] [review]
add selectedTab null check

Review of attachment 8620980 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/chrome/content/browser.js
@@ +1855,5 @@
>          break;
>  
>        case "Viewport:FixedMarginsChanged":
> +        if (this.selectedTab) {
> +          gViewportMargins = JSON.parse(aData);

Move the gViewportMargins assignment out of the condition, I think we still need that.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85c0975e7d92

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/85c0975e7d92

Comment 10

[Geoff Brown [:gbrown]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #4)
> I doubt it will fix the startup hang either; a JS exception at that point
> should just abort the handling of the FixedMarginsChanged message and
> continue with the usual browser startup. It may be that the hang and the
> exception both have the same underlying root cause which is something else.

It looks like you were right -- these failures continue in bug 1054292, just as before but now without these TypeErrors.