1102210 - crash in nsAndroidHistory::RemovePendingVisitURI(nsIURI*)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Flaviu Cos]

This bug was filed from the Socorro interface and is 
report bp-1a2f4f14-550d-4021-a657-e6b892141120.
=============================================================
Environment: 
Device: HTC One M7 (Android 4.2.2);
Build: Nightly 36.0a1 (2014-10-20);

Steps to reproduce:
1. Go to bookmarks panel;
2. Long tap and choose 'Open in New Tab' option on a bookmark;
3. Repeat step 3 for a few more bookmarks.

Expected result:
All the bookmarks are successfully opened in new tab.

Actual result:
Firefox crashes.

Stack trace:
0	libxul.so	nsAndroidHistory::RemovePendingVisitURI(nsIURI*)	mobile/android/components/build/nsAndroidHistory.cpp
1	libxul.so	nsAndroidHistory::VisitURI(nsIURI*, nsIURI*, unsigned int)	mobile/android/components/build/nsAndroidHistory.cpp
2	libxul.so	nsDocShell::AddURIVisit(nsIURI*, nsIURI*, nsIURI*, unsigned int, unsigned int)	docshell/base/nsDocShell.cpp
3	libxul.so	nsDocShell::OnRedirectStateChange(nsIChannel*, nsIChannel*, unsigned int, unsigned int)	docshell/base/nsDocShell.cpp
4	libxul.so	nsDocLoader::AsyncOnChannelRedirect(nsIChannel*, nsIChannel*, unsigned int, nsIAsyncVerifyRedirectCallback*)	uriloader/base/nsDocLoader.cpp
5	libxul.so	nsAsyncRedirectVerifyHelper::DelegateOnChannelRedirect(nsIChannelEventSink*, nsIChannel*, nsIChannel*, unsigned int)	netwerk/base/src/nsAsyncRedirectVerifyHelper.cpp
6	libxul.so	nsAsyncRedirectVerifyHelper::Run()	netwerk/base/src/nsAsyncRedirectVerifyHelper.cpp
7	libxul.so	nsThread::ProcessNextEvent(bool, bool*)	xpcom/threads/nsThread.cpp
8	libxul.so	NS_ProcessNextEvent(nsIThread*, bool)	xpcom/glue/nsThreadUtils.cpp
9	libxul.so	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)	ipc/glue/MessagePump.cpp
10	libxul.so	MessageLoop::RunInternal()	ipc/chromium/src/base/message_loop.cc
11	libxul.so	MessageLoop::Run()	ipc/chromium/src/base/message_loop.cc
12	libxul.so	nsBaseAppShell::Run()	widget/nsBaseAppShell.cpp
13	libxul.so	nsAppStartup::Run()	toolkit/components/startup/nsAppStartup.cpp
14	libxul.so	XREMain::XRE_mainRun()	toolkit/xre/nsAppRunner.cpp
15	libxul.so	XREMain::XRE_main(int, char**, nsXREAppData const*)	toolkit/xre/nsAppRunner.cpp
16	libxul.so	XRE_main	toolkit/xre/nsAppRunner.cpp
17	libxul.so	GeckoStart	toolkit/xre/nsAndroidStartup.cpp
18	libmozglue.so	Java_org_mozilla_gecko_mozglue_GeckoLoader_nativeRun	mozglue/android/APKOpen.cpp
Ø 19	libdvm.so	libdvm.so@0x20392	
Ø 20	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 21	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x5999c0	
Ø 22	libdvm.so	libdvm.so@0x532cd	
Ø 23	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x5999be	
24	libmozglue.so	Java_org_mozilla_gecko_mozglue_GeckoLoader_loadNSSLibsNative	mozglue/android/APKOpen.cpp
25		@0xbffffffe	
Ø 26	libc.so	libc.so@0x118f1	
Ø 27	libc.so	libc.so@0xcff3	
Ø 28	libdvm.so	libdvm.so@0x55fb5	
Ø 29	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4d57f6	
Ø 30	libdvm.so	libdvm.so@0x7ea01	
Ø 31	dalvik-heap (deleted)	dalvik-heap (deleted)@0x10de	
Ø 32	libdvm.so	libdvm.so@0x2cb86	
Ø 33	libdvm.so	libdvm.so@0x693bb	
Ø 34	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x347e	
Ø 35	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42f2	
Ø 36	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 37	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42de	
Ø 38	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x5eb06f	
Ø 39	libdvm.so	libdvm.so@0x76d17	
Ø 40	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x5eb06f	
Ø 41	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 42	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x5eb06f	
Ø 43	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x727ffe	
Ø 44	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0xfada6	
Ø 45	dalvik-heap (deleted)	dalvik-heap (deleted)@0x122aaab9	
Ø 46	libdvm.so	libdvm.so@0x55ebb	
Ø 47	dalvik-heap (deleted)	dalvik-heap (deleted)@0x122aaab9	
Ø 48	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42de	
Ø 49	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42de	
Ø 50	libdvm.so	libdvm.so@0x5311f	
Ø 51	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 52	libdvm.so	libdvm.so@0x55cf9	
Ø 53	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42de	
Ø 54	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x3b160a	
Ø 55	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 56	libdvm.so	libdvm.so@0x204fe	
Ø 57	libdvm.so	libdvm.so@0x29822	
Ø 58	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 59	libdvm.so	libdvm.so@0x2ee56	
Ø 60	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 61	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4c3f06	
Ø 62	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x598fe0	
Ø 63	libdvm.so	libdvm.so@0x693bb	
Ø 64	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 65	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4c3f06	
Ø 66	libdvm.so	libdvm.so@0x693e5	
Ø 67	libdvm.so	libdvm.so@0x5c3dd	
Ø 68	data@[e-mail]@classes.dex	data@[e-mail]@classes.dex@0x676545	
Ø 69	dalvik-heap (deleted)	dalvik-heap (deleted)@0x122aaab9	
Ø 70	libdvm.so	libdvm.so@0x5c33b	
Ø 71	libdvm.so	libdvm.so@0x5c33b	
Ø 72	libc.so	libc.so@0xe4ba	
Ø 73	libc.so	libc.so@0xdba6

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Of course, aLastVisitedURI can be null. Patch coming.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: history-crash v0.1

This should fix the crash. I'm going to try and make a test for this stuff too.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8526094 [details] [diff] [review]
history-crash v0.1

I am still fighting with some tests, but wanted to move this along to fix the crash

Comment 4

[Richard Newman [:rnewman]]

Comment on attachment 8526094 [details] [diff] [review]
history-crash v0.1

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

Oops.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: historyservice-tests v0.1

This is similar to the testTrackingProtection approach. I again use a promise-based helper to handle page loads, but also add a promise-based sleep to help get the timing right.

As with the promise-based load helper, the promise-based sleep seems to be used a lot in our tests. As with the tracking protection tests, I switched from using the deprecated Promise.jsm approach to the new JS Promise approach.

Three tests are run:
1. Simple HTML file. No redirects, so one history visit is expected.
2. 301 redirect via an SJS file. The 301 redirect should be ignored, so one history visit is expected.
3. JS redirection via an SJS file. The redirection is not hidden, so we see two history visits: the JS redirector and the final HTML page.

The pending history visits are flushed after at most three (3) seconds, so we need to wait for longer than that. I picked four (4) seconds and it works fine locally.

Try run: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c1daf1e823a2

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Mark Finkle (:mfinkle) from comment #5)

> Try run:
> https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c1daf1e823a2

That one did not run robocop tests.

This one does:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e64f6ec423b6

Android 2.3 RC3 and Android 4.0 RC5 are both green \o/

Comment 7

[Aaron Train [:aaronmt]]

rank #3 on trunk

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Landed the crash fix:
https://hg.mozilla.org/integration/fx-team/rev/1c41bf100a3d

Waiting to close until we get the test.

Comment 9

[Richard Newman [:rnewman]]

Comment on attachment 8526432 [details] [diff] [review]
historyservice-tests v0.1

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

Bless you. You are the test king.

If this runs and passes, I'm happy.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Test:
https://hg.mozilla.org/integration/fx-team/rev/65205034ed90

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/1c41bf100a3d
https://hg.mozilla.org/mozilla-central/rev/65205034ed90

Comment 12

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in Firefox for Android 36.0a1 (2014-11-23)
Device: Asus Transformer Pad TF300T (Android 4.2.1)