Closed Bug 1333001 Opened 3 years ago Closed 3 years ago

Assertion failure: value, at BindingUtils.h:900

Categories

(Core :: DOM: Core & HTML, defect)

53 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox53 + fixed
firefox54 --- fixed

People

(Reporter: cbook, Assigned: bzbarsky)

References

(Blocks 1 open bug, )

Details

(Keywords: assertion, crash)

Attachments

(3 files)

Attached file stack
Found by bughunter and reproduced on latest windows debug trunk tinderbox build.

Steps to reproduce:
-> Load https://indianstudyhub.com/logical-reasoning/topics/

Crashes instantly opt and debug builds at least on windows

Crash id: https://crash-stats.mozilla.com/report/index/53eda3ae-1f11-4d10-9724-e315e2170123
[Tracking Requested - why for this release]:
53 - bughunter opt/debug crash

Smaug, Boris, is this something for you ?
Flags: needinfo?(bzbarsky)
Flags: needinfo?(bugs)
This is totally mine.  In bug 851892 I wrote code that assumes that the mMedia of a css::ImportRule is never null (and convinced myself by code inspection that this was the case).  But clearly this isn't the case, for some reason.
Assignee: nobody → bzbarsky
Blocks: 851892
Flags: needinfo?(bzbarsky)
Flags: needinfo?(bugs)
I spent a while trying to write a test (that is, produce a situation in which an
@import rule with no child sheet is being cloned in a crashtest), and have not
succeeded so far....
Attachment #8829547 - Flags: review?(cam)
Comment on attachment 8829547 [details] [diff] [review]
Ensure that ImportRule in fact always has a non-null mMedia

Approval Request Comment
[Feature/Bug causing the regression]: Bug 851892
[User impact if declined]: Crashes on some sites
[Is this code covered by automated tests?]: No; couldn't figure out a sane way
   to reproduce in our test harnesses.  :(
[Has the fix been verified in Nightly?]: In my build, yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Adds a missing "copy the member while
  cloning" thing.
[String changes made/needed]: None.
Attachment #8829547 - Flags: approval-mozilla-aurora?
Comment on attachment 8829547 [details] [diff] [review]
Ensure that ImportRule in fact always has a non-null mMedia

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

Managed to get a crashtest working using a chrome: URL, like you mention in bug 1326509.
Attachment #8829547 - Flags: review?(cam) → review+
Attached patch crashtestSplinter Review
Attachment #8829713 - Flags: review?(bzbarsky)
Comment on attachment 8829713 [details] [diff] [review]
crashtest

r=me.  Thank you!
Attachment #8829713 - Flags: review?(bzbarsky) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/65a5cc2a2ef6
Ensure that ImportRule in fact always has a non-null mMedia.  r=heycam
https://hg.mozilla.org/integration/mozilla-inbound/rev/63dce15dd3d4
Crashtest. r=bzbarsky
https://hg.mozilla.org/mozilla-central/rev/65a5cc2a2ef6
https://hg.mozilla.org/mozilla-central/rev/63dce15dd3d4
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Flags: in-testsuite+
Version: unspecified → 53 Branch
Track 53+ as this crash is reproducible on real site.
Comment on attachment 8829547 [details] [diff] [review]
Ensure that ImportRule in fact always has a non-null mMedia

Fix a crash. Aurora53+.
Attachment #8829547 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.