Closed Bug 1137906 Opened 9 years ago Closed 9 years ago

[Accessibility] crash during FxA login with screen reader on - mozilla::a11y::xpcAccessible::GetState(unsigned int*, unsigned int*)

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla39
Project Flags:
blocking-b2g 2.2+
Tracking Flags:
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- verified

People

(Reporter: ychung, Assigned: ting)

References

(
URL
)

Details

(Keywords: crash, verifyme, Whiteboard: [3.0-Daily-Testing], [b2g-crash])

Crash Data

Attachments

(2 files, 1 obsolete file)

logcat_20150227_CrashWithScreenReader.txt
1.10 MB, text/plain
Details
patch v2
1.07 KB, patch
surkov
: review+
bajaj
: approval-mozilla-b2g37+
Details | Diff | Splinter Review 
This bug was filed from the Socorro interface and is 
report bp-9bd0c138-b20b-4e2e-aea3-10e632150227.
=============================================================
When the user tries to enter password for FFOX account with screen reader on.

Repro Steps:
1) Update a Flame to 20150227010229.
2) Enable Screen Reader on Settings > Accessibility.
3) Go to Settings > Firefox Accounts > Create Account or Sign In.
4) Enter an email address, and select Next.
5) Tap on the password input field to enter password.

Actual:
Keyboard never shows up, and the crash happens.

Expected:
The keyboard pops up, so the user can enter the password.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150227010229
Gaia: 7512026a377271a0cade12d70846557f0bc7781c
Gecko: c7968255c1ea
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 5/5
See attached: logcat
This issue also reproduces on Flame 2.2.

Result: The crash occurs. 

Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150227002521
Gaia: eb6a5ac9081d3962198e0f4520b0743d716d7a27
Gecko: c8a38dcfbebc
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
==========================================

This issue is unable to reproduce on Flame 2.1. The user is unable to type in the email field at all when trying to log in with FxA.

Device: Flame 2.1  (KK, 319mb, full flash)
Build ID: 20150227002636
Gaia: 5d3479fdd438412adee4452720856b6b771fe5cd
Gecko: 0390c73a827b
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.2: --- → affected
Flags: needinfo?(ktucker)
Crash stack shows:

Frame 	Module 	Signature 	Source
0 	libxul.so 	mozilla::a11y::xpcAccessible::GetState(unsigned int*, unsigned int*) 	accessible/xpcom/xpcAccessible.cpp
1 	libxul.so 	NS_InvokeByIndex 	xpcom/reflect/xptcall/md/unix/xptcinvoke_arm.cpp
2 	libxul.so 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	js/xpconnect/src/XPCWrappedNative.cpp
3 	libxul.so 	XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp
4 	libxul.so 	js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) 	js/src/jscntxtinlines.h
5 	libxul.so 	js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
6 	libxul.so 	js::jit::DoCallFallback 	js/src/jit/BaselineIC.cpp
7 		@0xb29f57e2
blocking-b2g: --- → 2.2?
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing], [b2g-crash]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Here's a video for the crash: http://youtu.be/QyhbPECMkDg
URL: http://youtu.be/QyhbPECMkDg
Yura, can you take a look please?
triage: crash. let's block.
blocking-b2g: 2.2? → 2.2+
Ting-Yu, you may want to take a look at this.
Flags: needinfo?(janus926)
I guess you're looking for :TYLin?
Flags: needinfo?(janus926) → needinfo?(tlin)
Sorry that I am unfamiliar with the Accessibility module.
Flags: needinfo?(tlin) → needinfo?(whuang)
Spoke with :ting and he'll take a look first.
Flags: needinfo?(whuang)
ni :ting for notice, thanks.
Flags: needinfo?(janus926)
I see only "Color Filter" in Accessibility menu, are there anything else I should set?
(In reply to Ting-Yu Chou [:ting] from comment #12)
> I see only "Color Filter" in Accessibility menu, are there anything else I
> should set?

Just learnt from Arthur I need to enable that from developer menu. But I don't know how to control the device then...
Flags: needinfo?(janus926)
(In reply to Ting-Yu Chou [:ting] from comment #13)
> Just learnt from Arthur I need to enable that from developer menu. But I
> don't know how to control the device then...

Learnt how to control it, will try to reproduce tomorrow.
GetState() is called on a xpcAccessibleHyperText belongs to the xpcAccessibleDocument of keyboard.html for entering account, which has been shutdown already.

I tried to get JS stack, but calling DumpJSStack() prints nothings and cx->currentScript(0x0, 1)->filename() makes the process exited.
(In reply to Ting-Yu Chou [:ting] from comment #15)
> GetState() is called on a xpcAccessibleHyperText belongs to the
> xpcAccessibleDocument of keyboard.html for entering account, which has been
> shutdown already.

Correction: index.html of keyboard app
(In reply to Ting-Yu Chou [:ting] from comment #15)
> I tried to get JS stack, but calling DumpJSStack() prints nothings and
> cx->currentScript(0x0, 1)->filename() makes the process exited.

The JS stack:

03-06 04:13:37.510 31020 31020 I Gecko   : Utils.jsm | getState getState@resource://gre/modules/accessibility/Utils.jsm:272:15
03-06 04:13:37.510 31020 31020 I Gecko   : isAliveAndVisible@resource://gre/modules/accessibility/Utils.jsm:400:19
03-06 04:13:37.510 31020 31020 I Gecko   : VisualPresenter_viewportChanged@resource://gre/modules/accessibility/Presentation.jsm:168:9
03-06 04:13:37.510 31020 31020 I Gecko   : Presentation_viewportChanged@resource://gre/modules/accessibility/Presentation.jsm:725:13
03-06 04:13:37.510 31020 31020 I Gecko   : handleEvent@resource://gre/modules/accessibility/EventManager.jsm:122:22
03-06 04:13:37.510 31020 31020 D x       : 0xab607428 xpcAccessible::GetState 0xafe633c8
The issue can be reproduced when you activate the keyboard at 2nd time, you don't need to go through the Fx account settings.
A more accurate STR:

1. Enable screen reader
2. Activate keyboard
3. Press any key to make it focused (border wrapped)
4. Deactivate keyboard
5. Activate keyboard

My current understanding is: When deactivate keyboard, the corresponding xpcAccessibleyDocument is shutdown and its cached accessibles are cleared, but VisualPresenter still keeps the accessible of the last pressed key in _displayedAccessibles. Next time when keyboard is activated, a resize event comes and VisualPresenter_viewportChanged() will try to get the state of the dead accessible.
Attached patch patch v1 (obsolete) — — Splinter Review 

        Attachment #8574542 -
        Flags: review?(surkov.alexander)

    
    

    
  
Comment on attachment 8574542 [details] [diff] [review]
patch v1

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

thanks for fixing this, r=me

::: accessible/xpcom/xpcAccessibleDocument.cpp
@@ +199,5 @@
> +ShutdownEnumerator(const Accessible* aKey, xpcAccessibleGeneric* aValue,
> +                   void* aUnused)
> +{
> +  aValue->Shutdown();
> +  return PL_DHASH_NEXT;

not sure which way is more perfromant: iterate all and then remove all or iterate and then remove by one but please make sure to consider this

        Attachment #8574542 -
        Flags: review?(surkov.alexander) → review+

    
    

    
  
> ::: accessible/xpcom/xpcAccessibleDocument.cpp
> @@ +199,5 @@
> > +ShutdownEnumerator(const Accessible* aKey, xpcAccessibleGeneric* aValue,
> > +                   void* aUnused)
> > +{
> > +  aValue->Shutdown();
> > +  return PL_DHASH_NEXT;
> 
> not sure which way is more perfromant: iterate all and then remove all or
> iterate and then remove by one but please make sure to consider this

I'd be suprised if it wasn't faster to do the removals in the same pass.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v2
        — 
          — Splinter Review
      

    


  
You're right, clear() actually walks through the entries. Change to shutdown and remove in one pass.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be79084d8473
Assignee: nobody → janus926

        Attachment #8574542 -
        Attachment is obsolete: true
Status: NEW → ASSIGNED

        Attachment #8575053 -
        Flags: review?(surkov.alexander)

        Attachment #8575053 -
        Flags: review?(surkov.alexander) → review+

    
  
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/048ac1585bb6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago

          status-firefox39:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

    
    

    
  
Comment on attachment 8575053 [details] [diff] [review]
patch v2

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): seems it's there since the code was created
User impact if declined: crash when activate keyboard 2nd time
Testing completed: crashtest/mochitest on windows, linux and b2g emulator
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: n/a

        Attachment #8575053 -
        Flags: approval-mozilla-b2g37?

    
    

    
  
Adding qawanted to verify this on nightly mozilla-central.
Keywords: qawanted, 
          
            verifyme

    
  
QA Contact: ychung

        Attachment #8575053 -
        Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+

    
  
Blocks: 1142764

    
    

    
  
This crash no longer occurs on Flame Master. However, I had another crash while following the STR for this crash, which is filed as bug 1142747, and a graphic issue after logging in, bug 1142764.

Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150312010235
Gaia: 0c4e8b0b330757e261b031b7e7f326ef419c9808
Gecko: 5334d2bead3e
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
=================================

Leaving verifyme for 2.2 uplift & verification.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]

          status-b2g-master:
          fixed → verified
Flags: needinfo?(ktucker)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: