Closed Bug 1102210 Opened 7 years ago Closed 7 years ago

crash in nsAndroidHistory::RemovePendingVisitURI(nsIURI*)


(Firefox for Android Graveyard :: Awesomescreen, defect)

Not set


(firefox36 verified)

Firefox 36
Tracking Status
firefox36 --- verified


(Reporter: cos_flaviu, Assigned: mfinkle)


(Keywords: crash)

Crash Data


(2 files)

This bug was filed from the Socorro interface and is 
report bp-1a2f4f14-550d-4021-a657-e6b892141120.
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	nsAndroidHistory::RemovePendingVisitURI(nsIURI*)	mobile/android/components/build/nsAndroidHistory.cpp
1	nsAndroidHistory::VisitURI(nsIURI*, nsIURI*, unsigned int)	mobile/android/components/build/nsAndroidHistory.cpp
2	nsDocShell::AddURIVisit(nsIURI*, nsIURI*, nsIURI*, unsigned int, unsigned int)	docshell/base/nsDocShell.cpp
3	nsDocShell::OnRedirectStateChange(nsIChannel*, nsIChannel*, unsigned int, unsigned int)	docshell/base/nsDocShell.cpp
4	nsDocLoader::AsyncOnChannelRedirect(nsIChannel*, nsIChannel*, unsigned int, nsIAsyncVerifyRedirectCallback*)	uriloader/base/nsDocLoader.cpp
5	nsAsyncRedirectVerifyHelper::DelegateOnChannelRedirect(nsIChannelEventSink*, nsIChannel*, nsIChannel*, unsigned int)	netwerk/base/src/nsAsyncRedirectVerifyHelper.cpp
6	nsAsyncRedirectVerifyHelper::Run()	netwerk/base/src/nsAsyncRedirectVerifyHelper.cpp
7	nsThread::ProcessNextEvent(bool, bool*)	xpcom/threads/nsThread.cpp
8	NS_ProcessNextEvent(nsIThread*, bool)	xpcom/glue/nsThreadUtils.cpp
9	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)	ipc/glue/MessagePump.cpp
10	MessageLoop::RunInternal()	ipc/chromium/src/base/
11	MessageLoop::Run()	ipc/chromium/src/base/
12	nsBaseAppShell::Run()	widget/nsBaseAppShell.cpp
13	nsAppStartup::Run()	toolkit/components/startup/nsAppStartup.cpp
14	XREMain::XRE_mainRun()	toolkit/xre/nsAppRunner.cpp
15	XREMain::XRE_main(int, char**, nsXREAppData const*)	toolkit/xre/nsAppRunner.cpp
16	XRE_main	toolkit/xre/nsAppRunner.cpp
17	GeckoStart	toolkit/xre/nsAndroidStartup.cpp
18	Java_org_mozilla_gecko_mozglue_GeckoLoader_nativeRun	mozglue/android/APKOpen.cpp
Ø 19	
Ø 20	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 21	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x5999c0	
Ø 22	
Ø 23	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x5999be	
24	Java_org_mozilla_gecko_mozglue_GeckoLoader_loadNSSLibsNative	mozglue/android/APKOpen.cpp
25		@0xbffffffe	
Ø 26	
Ø 27	
Ø 28	
Ø 29	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4d57f6	
Ø 30	
Ø 31	dalvik-heap (deleted)	dalvik-heap (deleted)@0x10de	
Ø 32	
Ø 33	
Ø 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@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x5eb06f	
Ø 39	
Ø 40	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x5eb06f	
Ø 41	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 42	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x5eb06f	
Ø 43	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x727ffe	
Ø 44	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0xfada6	
Ø 45	dalvik-heap (deleted)	dalvik-heap (deleted)@0x122aaab9	
Ø 46	
Ø 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	
Ø 51	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 52	
Ø 53	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3e42de	
Ø 54	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x3b160a	
Ø 55	dalvik-heap (deleted)	dalvik-heap (deleted)@0x46b44e	
Ø 56	
Ø 57	
Ø 58	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 59	
Ø 60	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 61	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4c3f06	
Ø 62	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x598fe0	
Ø 63	
Ø 64	dalvik-LinearAlloc (deleted)	dalvik-LinearAlloc (deleted)@0x3f43ce	
Ø 65	dalvik-heap (deleted)	dalvik-heap (deleted)@0x4c3f06	
Ø 66	
Ø 67	
Ø 68	data@app@org.mozilla.fennec-1.apk@classes.dex	data@app@org.mozilla.fennec-1.apk@classes.dex@0x676545	
Ø 69	dalvik-heap (deleted)	dalvik-heap (deleted)@0x122aaab9	
Ø 70	
Ø 71	
Ø 72	
Ø 73
Of course, aLastVisitedURI can be null. Patch coming.
Assignee: nobody → mark.finkle
This should fix the crash. I'm going to try and make a test for this stuff too.
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
Attachment #8526094 - Flags: review?(rnewman)
Comment on attachment 8526094 [details] [diff] [review]
history-crash v0.1

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

Attachment #8526094 - Flags: review?(rnewman) → review+
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:
Attachment #8526432 - Flags: review?(rnewman)
(In reply to Mark Finkle (:mfinkle) from comment #5)

> Try run:

That one did not run robocop tests.

This one does:

Android 2.3 RC3 and Android 4.0 RC5 are both green \o/
rank #3 on trunk
Landed the crash fix:

Waiting to close until we get the test.
Keywords: leave-open
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.
Attachment #8526432 - Flags: review?(rnewman) → review+
Verified as fixed in Firefox for Android 36.0a1 (2014-11-23)
Device: Asus Transformer Pad TF300T (Android 4.2.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.