Created attachment 8850131 [details] testcase.html The attached testcase asserts in mozilla-central rev e03e0c60462. Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch), at /home/worker/workspace/build/src/layout/generic/nsGridContainerFrame.cpp:474 #01: nsGridContainerFrame::Reflow at layout/generic/nsGridContainerFrame.cpp:6196 #02: nsBlockReflowContext::ReflowBlock at layout/generic/nsBlockReflowContext.cpp:307 #03: nsBlockFrame::ReflowBlockFrame at layout/generic/nsBlockFrame.cpp:3470 #04: nsBlockFrame::ReflowLine at layout/generic/nsBlockFrame.cpp:2831 #05: nsBlockFrame::ReflowDirtyLines at layout/generic/nsBlockFrame.cpp:2370 #06: nsBlockFrame::Reflow at layout/generic/nsBlockFrame.cpp:1250 #07: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:900 #08: nsCanvasFrame::Reflow at layout/generic/nsCanvasFrame.cpp:722 #09: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:900 #10: nsHTMLScrollFrame::ReflowScrolledFrame at layout/generic/nsGfxScrollFrame.cpp:556 #11: nsHTMLScrollFrame::ReflowContents at layout/generic/nsGfxScrollFrame.cpp:686 #12: nsHTMLScrollFrame::Reflow at layout/generic/nsGfxScrollFrame.cpp:1041 #13: nsContainerFrame::ReflowChild at layout/generic/nsContainerFrame.cpp:943 #14: mozilla::ViewportFrame::Reflow at layout/generic/ViewportFrame.cpp:330 #15: mozilla::PresShell::DoReflow at layout/generic/ReflowOutput.h:282
Assignee: nobody → mats
OS: Unspecified → All
Priority: -- → P2
Hardware: Unspecified → All
Summary: Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch) → [css-grid] Assertion failure: mIter.isSome() || mArray->Length() == aGridItemCount (grid item count mismatch)
I can't reproduce the bug in a Linux64 debug build from the exact same rev as yours (which is currently m-c tip), using a fresh profile.
Daniel, can you reproduce this assertion?
Sorry, my clownshoes are showing -- I forgot that the build I tested in had "--disable-debug" in its mozconfig, so of course assertions were disabled. I can reproduce this in a Linux64 debug build that I just downloaded from treeherder -- the "target.tar.bz2" tarball here: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=01125b5142e587fb8d8bd40c8dbaf6aaef71a9f9&filter-searchStr=linux%20x64%20debug%20tc%20B&selectedJob=85835007 (Direct link: https://queue.taskcluster.net/v1/task/MuZ5TB9zRb2g0XjQ-uSi2g/runs/0/artifacts/public/build/target.tar.bz2 ) Mats, if your tested local build is definitely a debug build & you still can't reproduce, maybe try that ^^ tarball as an extra sanity check?
Flags: needinfo?(jschwartzentruber) → needinfo?(mats)
Ah, the plot thickens -- my *locally-built* enable-debug/disable-optimize build does not reproduce the issue. The main distinction between my just-tested local build & the downloaded tarball from comment 4 is "disable-optimize" (local bulid) vs "enable-optimize" (treeherder build -- technically unspecified, but this is the default). So maybe this depends on some subtly-different initialization / out-of-order instructions that only happen in enable-optimize builds...?
Thanks Daniel, I indeed have disable-optimize in my .mozconfig. I'll rebuild with enable-optimize and see if that helps.
Thanks Daniel. I only try taskcluster builds usually (mozilla-central.latest.firefox.linux64-debug). I didn't realize those are optimized.
Nope, that build didn't reproduce it either. Hmm, it might be a gcc/clang difference...
Yep, now I can reproduce it. gcc + enable-optimize did it.
Created attachment 8850517 [details] [diff] [review] fix It's just a bogus assertion. When mArray is used it contains all grid container children, including placeholders. aGridItemCount always excludes placeholders.
Attachment #8850517 - Flags: review?(dholbert)
(BTW, I'm amazed how much faster enable-optimize makes the build, even with enable-debug. It feels like an order of magnitude or so. It does make it less debuggable though, with gdb frequently displaying <optimized out> when trying to print variables. It seems worth investigating if adding -Og and/or -ggdb helps with that though...)
(In reply to Mats Palmgren (:mats) from comment #9) > Yep, now I can reproduce it. gcc + enable-optimize did it. Glad to hear it. I can't reproduce with *clang* + enable-optimize, FWIW. (I left that building overnight to test in the morning, & just tried it. But I guess that's just because clang & gcc perform different optimizations.
D'oh, I just now noticed the "CLANG_CRASH_BUG" exclusion-ifdef around the condition -- that definitely explains why I'm not hitting it. :) (I'm using clang 3.8.1 on this machine, which satisfies the CLANG_CRASH_BUG condition.)
Comment on attachment 8850517 [details] [diff] [review] fix Review of attachment 8850517 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8850517 - Flags: review?(dholbert) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/3670c556728b [css-grid] Fix a bogus assertion. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/18e9870067e4 [css-grid] Crashtest.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
I'm going to assume that this isn't worth backporting. Feel free to set things back to affected and make the requests if you feel otherwise, though.
status-firefox52: --- → wontfix
status-firefox53: --- → wontfix
status-firefox54: --- → wontfix
status-firefox-esr52: --- → wontfix
Version: Trunk → 52 Branch
I agree. It's a debug-only change, so there's no user benefit that we'd gain from uplifting this.
...though actually, this is in a piece of code that I want to move around over in Bug 812687 (and I'll want to uplift the patches over there). So it would make for an easier/cleaner uplift over there, if we uplifted this bug's patches over here. I think we don't need to bother with uplift approval over here, though -- this bug only changes the inside of a MOZ_ASSERT expression, so it's debug-only and hence NPOTB.
Uplifted to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/08002a9d9e3ae337bdd23bf01432ba64251b880e https://hg.mozilla.org/releases/mozilla-aurora/rev/f09f78db26a594b76139cee19a2487f9509d92ee ...and beta: https://hg.mozilla.org/releases/mozilla-beta/rev/27126f140a2cf381f070afe905cf03659be8cd98 https://hg.mozilla.org/releases/mozilla-beta/rev/5ec9b3104a4cc38b1d28e6f49f0c0dc61642314e ...per comment 20. (I'd like it to as easy as possible for bug 812687 to be uplifted, and this bug's NPOTB changes are a requirement for that).
status-firefox53: wontfix → fixed
status-firefox54: wontfix → fixed
You need to log in before you can comment on or make changes to this bug.