Closed Bug 1321969 Opened 3 years ago Closed 3 years ago

Crash in nsGlobalWindow::GetLocalStorage

Categories

(Core :: DOM: Security, defect, P1, critical)

53 Branch
Unspecified
Windows 8
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 + fixed

People

(Reporter: marcia, Assigned: huseby)

References

(Blocks 1 open bug)

Details

(Keywords: crash, topcrash, Whiteboard: [OA])

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is 
report bp-585caff3-7151-4128-a52c-c44a52161203.
=============================================================

Seen while looking at nightly crash stats - this one started using the 20161202030204 build: http://bit.ly/2fWBrzO.

Possible regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd4cdcc9ad6c45dad8b8d8c0d40e459db2bca8a1&tochange=f65ad27efe839ce9df0283840a1a40b4bbc9ead0

Bug 1316075 I think touched code in this area. ni on Dave.
Flags: needinfo?(huseby)
In Nightly 20161202030204 this is the #1 crash once shutdown hang crashes are excluded, with 67 occurrences.

It started in Nightly 20161202030204. Regression window for that build includes bug 1316075:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd4cdcc9ad6c45dad8b8d8c0d40e459db2bca8a1&tochange=f65ad27efe839ce9df0283840a1a40b4bbc9ead0
[Tracking Requested - why for this release]: top crash
Keywords: topcrash
(In reply to Marcia Knous [:marcia - use ni] from comment #0)
> Bug 1316075 I think touched code in this area. ni on Dave.

Yeah, this is crashing on a release assertion that that patch added.
Crash Signature: [@ nsGlobalWindow::GetLocalStorage] → [@ nsGlobalWindow::GetLocalStorage] [@ nsGlobalWindow::GetSessionStorage]
Flags: needinfo?(huseby)
Bug 1316075 is the problem. The fix for now is to comment it out while I investigate why the values aren't in sync there.
Attached patch Bug_1321969.patch (obsolete) — Splinter Review
I have no idea why the PB flag and the OA aren't the same in that spot.  This patch removes it for now while I investigate.
Assignee: nobody → huseby
Status: NEW → ASSIGNED
Attachment #8817461 - Flags: review?(ehsan)
Crash Signature: [@ nsGlobalWindow::GetLocalStorage] [@ nsGlobalWindow::GetSessionStorage] → [@ nsGlobalWindow::GetLocalStorage] [@ nsGlobalWindow::GetSessionStorage] [@ nsWindowWatcher::OpenWindowInternal]
Tracking 53+ for this top crash.
Comment on attachment 8817461 [details] [diff] [review]
Bug_1321969.patch

Can we comment it out instead?  Along with a comment that it will be added back in bug number X, where X is a new or an existing bug where we will figure out the issues and fix them?

Thanks for taking this quickly Dave!
Priority: -- → P1
Whiteboard: [OA]
See Also: → 1322760
modified to comment the assert out with a comment.  thanks tanvi.
Attachment #8817461 - Attachment is obsolete: true
Attachment #8817461 - Flags: review?(ehsan)
Attachment #8817712 - Flags: review?(amarchesini)
Attachment #8817712 - Flags: review?(amarchesini) → review?(bkelly)
#6 topcrash in Nightly 20161209030212. Will be good to get a fix soon :)
Comment on attachment 8817712 [details] [diff] [review]
Bug_1321969.patch

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

LGTM as long as we are following up to fix it.

I wonder if this is a case where you simply don't have a docshell any more so the IsPrivateBrowsing() returns false.  Or maybe if its not the current inner window you can get different attributes on the docshell?
Attachment #8817712 - Flags: review?(bkelly) → review+
Keywords: checkin-needed
Depends on: 1316075
Blocks: 1322760
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/127edcc85d6f
diagnostic assert triggering top crash in nightly. r=bkelly
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/127edcc85d6f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Blocks: 1323262
Still see the signature nsGlobalWindow::GetSessionStorage on crash-stats. I don't have permission to read bug 1323262, are you handling it there?
Flags: needinfo?(huseby)
(In reply to Ting-Yu Chou [:ting] from comment #15)
> Still see the signature nsGlobalWindow::GetSessionStorage on crash-stats. I
> don't have permission to read bug 1323262, are you handling it there?

No, it should be filed separately I think.
Filed bug 1325514.
Flags: needinfo?(huseby)
You need to log in before you can comment on or make changes to this bug.