Tab count not visible

VERIFIED FIXED in Firefox 14

Status

()

Firefox for Android
General
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: Jaclyn Fu, Assigned: Margaret)

Tracking

Trunk
Firefox 15
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox14 verified, firefox15 verified, blocking-fennec1.0 +)

Details

(Whiteboard: [Engagement] [PMM])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 621731 [details]
screenshot

Web page or screen you were on when you saw the issue: all

Steps to reproduce:
1. Opened Aurora
2. Tap on tab to open new tab
3. Have multiple tabs opened

What you expected: able to see tab count

Comment 1

6 years ago
I see this on my galaxy nexus with ICS today also, see attached additional screenshot.

Comment 2

6 years ago
Created attachment 621775 [details]
screenshot of empty tab
(Assignee)

Comment 3

6 years ago
If either of you see this again, could you attach logs?
(Reporter)

Comment 4

6 years ago
Quick instructions on how to find logs? :)
(Assignee)

Comment 5

6 years ago
(In reply to Jaclyn Fu from comment #4)
> Quick instructions on how to find logs? :)

https://wiki.mozilla.org/Mobile/Testdrivers_Program/How_To/File_a_Bug#If_possible.2C_get_a_Log :)

Comment 6

6 years ago
Created attachment 621811 [details]
log of tab increment issue (sometime around 11:09/4:09)

Updated

6 years ago
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → +
(Assignee)

Updated

6 years ago
Assignee: nobody → margaret.leibovic
(Assignee)

Comment 7

6 years ago
Looking at the logs, nothing jumps out at me as causing the problem. Unfortunately, we don't currently have much logging around updating the tab count (maybe I should write a patch to add some).

Can anyone figure out steps to reproduce this? Michelle or Jaclyn, do you remember what you were doing when this happened?
Keywords: qawanted
Although not the steps reported; another way to get into this state is via changing the system language while the Fennec activity is currently running (i.e, Settings -> Language & input -> Language -> Fran├žais), and switch back to Fennec.
(Assignee)

Comment 9

6 years ago
(In reply to Aaron Train [:aaronmt] from comment #8)
> Although not the steps reported; another way to get into this state is via
> changing the system language while the Fennec activity is currently running
> (i.e, Settings -> Language & input -> Language -> Fran├žais), and switch back
> to Fennec.

Thanks for that - I'll look into what happens there. Before seeing this comment, I wrote this...

So, doing some code investigation, this could only happen if we both hide the number (mTabsCount.setVisibility(View.GONE)) *and* we make the tab icon image invisibile (mTabs.setImageLevel(0)).

We call mTabs.setImageLevel(0) during initialization, and mTabsCount is hidden to begin with, so I suppose we could get into this state if we somehow never call updateTabCount or updateTabCountAndAnimate, but that seems unlikely.

The calls to mTabs.setImageLevel() in updateTabCountAndAnimate will never call it with 0, so those can't be guilty.

The only other explanation is that we're calling updateTabCount(0), since that would call mTabs.setImageLevel(0) and hide mTabsCount. There are two places this could happen:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserToolbar.java#377
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tabs.java#298

The first will only happen on devices that use ActionBar, and since this happened on Jaclyn's gingerbread device, that call in Tabs is all that's left.

Jaclyn and Michelle, do you remember if this happened after coming back to the app after it had been killed in the background?

Brian, is there any way that Tabs.getCount() could return 0 at the end of session restore?
(Assignee)

Comment 10

6 years ago
(Brian, see above comment)
(Reporter)

Comment 11

6 years ago
I think it was after I came back to the app after it was killed
(In reply to Margaret Leibovic [:margaret] from comment #9)
> Brian, is there any way that Tabs.getCount() could return 0 at the end of
> session restore?

In the screenshot, it looks like the page has loaded and there's a title - basically, everything seems to be working (excluding the tab counter). I don't think it's possible for a page to be shown if there are no tabs.

Maybe Session:RestoreEnd isn't getting called somehow? That would be one explanation why everything else would be working. Restore failures should be caught - we have a try/catch that sends a "fail" message using notifyObservers(). But that function first calls _clearCache() before notifying observers (http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#920), so that might somehow be failing.

Comment 13

6 years ago
Same here, the counter disappeared after restarting from a crash.
From Michelle's log:

05-07 22:59:42.591: INFO/GeckoTabs(15724): Got message: Session:RestoreBegin
05-07 22:59:42.599: INFO/GeckoApp(15724): Got message: Gecko:Ready
05-07 22:59:42.599: DEBUG/GeckoLayerClient(15724): Screen-size changed to (720,1184)
05-07 22:59:42.599: DEBUG/GeckoLayerClient(15724): Window-size changed to (720,1038)
05-07 22:59:42.607: INFO/GeckoLayerClient(15724): Showing checks: true
05-07 22:59:42.615: INFO/Gecko(15724): Detected osrelease `3.0.8-gda6252b'
05-07 22:59:42.615: INFO/Gecko(15724): JITs are not broken
05-07 22:59:42.654: INFO/GeckoTabs(15724): Got message: Tab:Added
05-07 22:59:42.654: INFO/GeckoTabs(15724): Received message from Gecko: 42983835 - Tab:Added
05-07 22:59:42.662: INFO/GeckoTabs(15724): Added a tab with id: 1
05-07 22:59:42.693: INFO/GeckoTabs(15724): Got message: Tab:Added
05-07 22:59:42.693: INFO/GeckoTabs(15724): Received message from Gecko: 42983867 - Tab:Added
05-07 22:59:42.693: INFO/GeckoTabs(15724): Added a tab with id: 2
05-07 22:59:42.716: INFO/GeckoTabs(15724): Got message: Tab:Added
05-07 22:59:42.716: INFO/GeckoTabs(15724): Received message from Gecko: 42983894 - Tab:Added
05-07 22:59:42.716: INFO/GeckoTabs(15724): Added a tab with id: 3
05-07 22:59:42.740: INFO/GeckoTabs(15724): Got message: Tab:Added
05-07 22:59:42.740: INFO/GeckoTabs(15724): Received message from Gecko: 42983919 - Tab:Added
05-07 22:59:42.740: INFO/GeckoTabs(15724): Added a tab with id: 4
05-07 22:59:42.748: INFO/GeckoTabs(15724): Got message: SessionHistory:New
05-07 22:59:42.763: ERROR/GeckoConsole(15724): [JavaScript Error: "[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsISHEntry.adoptBFCacheEntry]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: jar:jar:file:///data/app/org.mozilla.fennec_aurora-1.apk!/omni.ja!/components/SessionStore.js :: _deserializeHistoryEntry :: line 747"  data: no]" {file: "jar:jar:file:///data/app/org.mozilla.fennec_aurora-1.apk!/omni.ja!/components/SessionStore.js" line: 747}]
05-07 22:59:42.771: INFO/GeckoPanZoomController(15724): Got message: Preferences:Data
Keywords: qawanted
Note that there's no corresponding Session:RestoreEnd above.

_restoreHistory() is called in an async callback (NetUtil.asyncFetch()), so that explains why the error isn't caught.
(Assignee)

Comment 16

6 years ago
Created attachment 622819 [details] [diff] [review]
patch to catch exception

Brian, thanks for figuring that out!

This patch just catches the exception, which will prevent SessionStore.js from dying. However, I'd like to figure out why adoptBFCacheEntry is failing, and if that will cause us other problems down the road.
Attachment #622819 - Flags: review?(mark.finkle)
Attachment #622819 - Flags: feedback?(justin.lebar+bug)
      let matchingEntry = aDocIdentMap[aEntry.docIdentifier];
      if (!matchingEntry) {
        matchingEntry = {shEntry: shEntry, childDocIdents: childDocIdents};
        aDocIdentMap[aEntry.docIdentifier] = matchingEntry;
      } else {
        shEntry.adoptBFCacheEntry(matchingEntry);   <------
        childDocIdents = matchingEntry.childDocIdents;
      }

Surely this should be adoptBFCacheEntry(matchingEntry.shEntry).  This looks right in nsSessionStore.js (the desktop version).
Attachment #622819 - Flags: feedback?(justin.lebar+bug)
Comment on attachment 622819 [details] [diff] [review]
patch to catch exception

Let's fix the typo first. I think we might want to add a try/catch in the code that calls _deserializeHistory, to keep the entire system from ending up horked if something goes wrong.
Attachment #622819 - Flags: review?(mark.finkle) → review-
(Assignee)

Comment 19

6 years ago
Created attachment 622900 [details] [diff] [review]
patch

Thank you, Justin! I was comparing with the desktop version earlier, but somehow missed that.

I don't think we need the try/catch anymore, since this shouldn't be throwing anymore with this fix. I hope we don't have other silly mistakes like this :/
Attachment #622900 - Flags: review?(mark.finkle)
Attachment #622900 - Flags: review?(mark.finkle) → review+
(Assignee)

Updated

6 years ago
Attachment #622819 - Attachment is obsolete: true
(Assignee)

Comment 20

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e8b484241e9
Target Milestone: --- → Firefox 15
(Assignee)

Comment 21

6 years ago
Comment on attachment 622900 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): n/a
User impact if declined: session restore can die, resulting in blank tabs counter (and likely session restore bustage)
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk typo fix
String changes made by this patch: n/a
Attachment #622900 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/8e8b484241e9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Attachment #622900 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 23

6 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/7bc24d1f43bf
status-firefox14: --- → fixed
I cannot reproduce this bug on the latest Nightly and Aurora builds. Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-30)
Firefox 14.0a2 (2012-05-30)

Device: Galaxy Nexus
OS: Android 4.0.2
Status: RESOLVED → VERIFIED
status-firefox14: fixed → verified
status-firefox15: --- → verified
You need to log in before you can comment on or make changes to this bug.