1321969 - Crash in nsGlobalWindow::GetLocalStorage

Status: RESOLVED FIXED

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

Description

[Marcia Knous [:marcia]]

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.

Comment 1

[Nicholas Nethercote [inactive]]

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

Comment 2

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: top crash

Comment 3

[Andrew McCreight [:mccr8]]

(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.

Comment 4

[Dave Huseby [: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.

Comment 5

[Dave Huseby [:huseby]]

Attachment: Bug_1321969.patch

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.

Comment 6

[Dave Huseby [:huseby]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c63641337fce413271ac197808ae421f26a0d446

Comment 7

[Marcia Knous [:marcia]]

Tracking 53+ for this top crash.

Comment 8

[Tanvi Vyas[:tanvi]]

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!

Comment 9

[Dave Huseby [:huseby]]

Attachment: Bug_1321969.patch

modified to comment the assert out with a comment.  thanks tanvi.

Comment 10

[Dave Huseby [:huseby]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e5b89c6ff566c625651b6f0705a969c513a20f9

Comment 11

[Nicholas Nethercote [inactive]]

#6 topcrash in Nightly 20161209030212. Will be good to get a fix soon :)

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/127edcc85d6f
diagnostic assert triggering top crash in nightly. r=bkelly

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/127edcc85d6f

Comment 15

[Ting-Yu Chou [:ting] (away)]

Still see the signature nsGlobalWindow::GetSessionStorage on crash-stats. I don't have permission to read bug 1323262, are you handling it there?

Comment 16

[(no longer active)]

(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.

Comment 17

[Ting-Yu Chou [:ting] (away)]

Filed bug 1325514.