Closed Bug 1393189 Opened 7 years ago Closed 7 years ago

stylo: Assertion failure: IsResolved()

Categories

(Core :: CSS Parsing and Computation, defect, P2)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: truber, Assigned: xidorn)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(8 files)

testcase.html
119 bytes, text/html
Details
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1393189 part 2 - Have CounterStyle::GetStyleName return nsIAtom and make it const.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1393189 part 4 - Remove kListStyleKTable and CSS keywords only used in this table.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1393189 part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1393189 part 6 - Add crashtest for this bug.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1393189 part 0 - Reorder NS_STYLE_LIST_STYLE_* consts to match the order in nsCSSProps::kListStyleKTable.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Attached file testcase.html — 
The attached testcase causes an assertion in m-c rev 20170823-c86b7150523c with stylo enabled by pref.

Assertion failure: IsResolved(), at /home/worker/workspace/build/src/layout/style/CounterStyleManager.h:244
 #0: mozilla::CounterStylePtr::Get, at layout/style/CounterStyleManager.h:244
 #1: Gecko_CounterStyle_IsSingleString, at layout/style/CounterStyleManager.h:230
 #2: style::gecko_bindings::structs::root::mozilla::GeckoList::clone_list_style_type, at out/gecko_properties.rs:15365
 #3: style::properties::animated_properties::AnimationValue::from_computed_values, at out/properties.rs:84808
 #4: geckoservo::glue::Servo_ComputedValues_ExtractAnimationValue, at servo/ports/geckolib/glue.rs:742
 #5: mozilla::dom::KeyframeEffectReadOnly::EnsureBaseStyle, at dom/animation/KeyframeEffectReadOnly.cpp:566
 #6: mozilla::dom::KeyframeEffectReadOnly::EnsureBaseStyles, at dom/animation/KeyframeEffectReadOnly.cpp:530
 #7: mozilla::dom::KeyframeEffectReadOnly::DoUpdateProperties<const mozilla::ServoStyleContext>, at dom/animation/KeyframeEffectReadOnly.cpp:343
 #8: mozilla::dom::KeyframeEffectReadOnly::DoSetKeyframes<const mozilla::ServoStyleContext>, at dom/animation/KeyframeEffectReadOnly.cpp:321
 #9: nsAnimationManager::DoUpdateAnimations<ServoCSSAnimationBuilder>, at layout/style/nsAnimationManager.cpp:428
#10: nsAnimationManager::UpdateAnimations, at layout/style/nsAnimationManager.cpp:1040
#11: Gecko_UpdateAnimations, at layout/style/ServoBindings.cpp:673
#12: style::gecko::wrapper::{{impl}}::update_animations, at servo/components/style/gecko/wrapper.rs:1246
#13: style::context::SequentialTask<style::gecko::wrapper::GeckoElement>::execute<style::gecko::wrapper::GeckoElement>, at servo/components/style/context.rs:481
#14: style::context::{{impl}}::drop<style::gecko::wrapper::GeckoElement>, at servo/components/style/context.rs:601
#15: core::ptr::drop_in_place<style::context::SequentialTaskList<style::gecko::wrapper::GeckoElement>>, at src/libcore/ptr.rs:60
#16: core::ptr::drop_in_place<style::context::ThreadLocalStyleContext<style::gecko::wrapper::GeckoElement>>, at src/libcore/ptr.rs:60
#17: core::ptr::drop_in_place<[core::cell::RefCell<core::option::Option<style::context::ThreadLocalStyleContext<style::gecko::wrapper::GeckoElement>>>]>, at src/libcore/ptr.rs:60
#18: core::ptr::drop_in_place<alloc::boxed::Box<[core::cell::RefCell<core::option::Option<style::context::ThreadLocalStyleContext<style::gecko::wrapper::GeckoElement>>>]>>, at src/libcore/ptr.rs:60
#19: style::parallel::traverse_dom<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly>, at servo/components/style/parallel.rs:104
#20: geckoservo::glue::traverse_subtree, at servo/ports/geckolib/glue.rs:250
#21: geckoservo::glue::Servo_TraverseSubtree, at servo/ports/geckolib/glue.rs:290
#22: mozilla::ServoStyleSet::StyleDocument, at layout/style/ServoStyleSet.cpp:893
#23: nsCSSFrameConstructor::ConstructDocElementFrame, at layout/base/nsCSSFrameConstructor.cpp:2558
#24: nsCSSFrameConstructor::ContentRangeInserted, at layout/base/nsCSSFrameConstructor.cpp:8121
#25: nsCSSFrameConstructor::ContentInserted, at layout/base/nsCSSFrameConstructor.h:283
#26: mozilla::PresShell::Initialize, at layout/base/PresShell.cpp:1809
Flags: in-testsuite?
I think we're using P1 for fatal asserts? If not, please feel free to re-prioritize.
Assignee: nobody → dakatsuka
Status: NEW → ASSIGNED
Priority: -- → P1
Priority: P1 → P2
Assignee: dakatsuka → xidorn+moz
Note that Cam had a patch to fix it already IIRC
emilio told me that heycam's patch in bug 1394297 should fix this bug.

I'd still like to use this bug for refactoring how CounterStyleOrNone::from_gecko_value is implemented for a bit, though.
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

::: commit-message-0a7b4:1
(Diff revision 1)
> +Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included. r?dholbert
> +

Could you include an extra few lines here with a little more motivation/explanation of what you're doing here?

::: dom/base/nsGkAtomList.h:405
(Diff revision 1)
>  GK_ATOM(DOMSubtreeModified, "DOMSubtreeModified")
>  GK_ATOM(double_, "double")
> +GK_ATOM(disclosure_closed, "disclosure-closed")
> +GK_ATOM(disclosure_open, "disclosure-open")
>  GK_ATOM(drag, "drag")

These new lines aren't quite in the correct sorted position -- they want to be one line up (since "di" sorts before "do")

::: dom/base/nsGkAtomList.h:593
(Diff revision 1)
>  GK_ATOM(itemtype, "itemtype")
> +GK_ATOM(japanese_informal, "japanese-informal")
> +GK_ATOM(japanese_formal, "japanese-formal")
>  GK_ATOM(kbd, "kbd")

These are in the wrong sorted order. "f" is before "i" in the alphabet, so "japanese-formal" should be sorted before "japanese-informal".

Same goes for every other "informal"/"formal" pair in this patch -- there are three others, I think (korean-hanja-*, simp-chinese-*, trad-chinese-*)

::: layout/style/BuiltinCounterStyleList.h:7
(Diff revision 1)
> +/* a list of all builtin counter styles */
> +
> +// none and decimal are not redefinable, so they should not be moved.
> +BUILTIN_COUNTER_STYLE(NONE, none)
> +BUILTIN_COUNTER_STYLE(DECIMAL, decimal)
> +// the following graphic styles are processed in a different way.
> +BUILTIN_COUNTER_STYLE(DISC, disc)
> +BUILTIN_COUNTER_STYLE(CIRCLE, circle)
> +BUILTIN_COUNTER_STYLE(SQUARE, square)
> +BUILTIN_COUNTER_STYLE(DISCLOSURE_CLOSED, disclosure_closed)

Could you include a comment to indicate what the args represent, so this list is less magical? (Like we do in e.g. the similar-but-larger nsCSSPropList.h file.)

Maybe something like:
// Notes on BUILTIN_COUNTER_STYLE args:
// 1st arg is a label that we can use in names of #define'd constants.
// 2nd arg is the nsGkAtoms field that represents the counter style's name.

::: layout/style/BuiltinCounterStyleList.h:9
(Diff revision 1)
> +// none and decimal are not redefinable, so they should not be moved.
> +BUILTIN_COUNTER_STYLE(NONE, none)
> +BUILTIN_COUNTER_STYLE(DECIMAL, decimal)
> +// the following graphic styles are processed in a different way.
> +BUILTIN_COUNTER_STYLE(DISC, disc)

What does "should not be moved" mean here? ("moved" how/where? Do you mean they can't be reordered to somewhere else in this list, i.e. they need to be first?)

Please clarify the wording here to be clearer.

::: layout/style/CounterStyleManager.h:100
(Diff revision 1)
>  protected:
>    int32_t mStyle;

Please add 'const' here:
  const int32_t mStyle;
...for good measure (while you make its getter 'constexpr' -- I think that helps prove that the getter's value really can be determined at compile-time. Or, even if it doesn't, it's good practice anyway, as long as this member-var isn't expected to change. :))

::: layout/style/CounterStyleManager.cpp:614
(Diff revision 1)
> +  // The atom for the name of the builtin counter style.
> +  // Extra indirection to point to nsGkAtoms members.
> +  nsIAtom** mName;

(1) Please declare this as 'nsIAtom** const' to indicate/validate that it's set in the init list and never changes.

(2) I didn't initially understand what the comment here meant, & why this needs to be a double-pointer.  Can you reword/extend this to clarify? "To point to nsGkAtoms" members is not really clear -- superficially, it seems like this could be simplified to a nsIAtom*.

(After some testing of me making that change: I think it's a double-pointer because we want to allow instances of this class to be instantiated by the compiler at compile-time, e.g. for the static_assert elsewhere in this patch, but we can't do that if this were a nsIAtom* whose value came from a "nsGkAtoms::foo" value, because nsGkAtoms::foo values are not themselves 'constexpr'. But we can use their address within the nsGkAtoms table -- that can be computed at compile time.  So -- I think I understand now, but only after modifying the code manually to try to make this a single-pointer and seeing how things broke.)

::: layout/style/nsStyleConsts.h:792
(Diff revision 1)
> -#define NS_STYLE_LIST_STYLE_SIMP_CHINESE_FORMAL   12
> -#define NS_STYLE_LIST_STYLE_TRAD_CHINESE_INFORMAL 13
> -#define NS_STYLE_LIST_STYLE_TRAD_CHINESE_FORMAL   14
> -#define NS_STYLE_LIST_STYLE_ETHIOPIC_NUMERIC      15
> -#define NS_STYLE_LIST_STYLE_DISCLOSURE_CLOSED     16
> -#define NS_STYLE_LIST_STYLE_DISCLOSURE_OPEN       17
> +#define NS_STYLE_LIST_STYLE_KOREAN_HANJA_FORMAL   12
> +#define NS_STYLE_LIST_STYLE_SIMP_CHINESE_INFORMAL 13
> +#define NS_STYLE_LIST_STYLE_SIMP_CHINESE_FORMAL   14
> +#define NS_STYLE_LIST_STYLE_TRAD_CHINESE_INFORMAL 15
> +#define NS_STYLE_LIST_STYLE_TRAD_CHINESE_FORMAL   16
> +#define NS_STYLE_LIST_STYLE_ETHIOPIC_NUMERIC      17
> -#define NS_STYLE_LIST_STYLE__MAX                  18
>  // These styles are handled as custom styles defined in counterstyles.css.

It looks like this section is just a reordering/renumbering (plus the removal of the now-unneeded __MAX value).

Would it make sense to split the reordering/renumbering into a "part 0" patch here? (if it's a necessary-but-independent change from the rest of this patch)
Attachment #8902196 - Flags: review?(dholbert) → review-
Comment on attachment 8902197 [details]
Bug 1393189 part 2 - Have CounterStyle::GetStyleName return nsIAtom and make it const.

https://reviewboard.mozilla.org/r/173588/#review179254
Attachment #8902197 - Flags: review?(dholbert) → review+
Comment on attachment 8902198 [details]
Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable.

https://reviewboard.mozilla.org/r/173590/#review179256

::: layout/style/CounterStyleManager.cpp:2042
(Diff revision 1)
>    } else {
> -    int32_t type;
> -    nsDependentAtomString name(aName);
> -    nsCSSKeyword keyword = nsCSSKeywords::LookupKeyword(name);
> -    if (nsCSSProps::FindKeyword(keyword, nsCSSProps::kListStyleKTable, type)) {
> -      if (gBuiltinStyleTable[type].IsDependentStyle()) {
> -        data = new (mPresContext) DependentBuiltinCounterStyle(type, this);
> -      } else {
> +    for (const BuiltinCounterStyle& item : gBuiltinStyleTable) {
> +      if (item.GetStyleName() == aName) {
> +        int32_t style = item.GetStyle();
> +        data = item.IsDependentStyle()
> +          ? new (mPresContext) DependentBuiltinCounterStyle(style, this)
> +          : GetBuiltinStyle(style);
> +        break;

There's a comment above this section about how we're doing a case-insensitive comparison here.  (I think it was case-insensitive due to the nsCSSKeywords::LookupKeyword usage here -- since nsCSSKeywords uses a "nsStaticCaseInsensitiveNameTable" under the hood.)


Is the new code still doing a case-insensitive comparison somehow, or is this patch accidentally converting it to be case-sensitive?  (It looks like it's an exact case-sensitive comparison, because it's checking for nsIAtom pointer-equality now.)

So I think this might be wrong & need some fixing. If I'm mistaken, though, then please make sure it's clear (via a comment) how this is still doing the right sort of comparison.

::: layout/style/nsRuleNode.cpp:8112
(Diff revision 1)
>            break;
>          default:
> -          name = NS_Atomize(nsCSSProps::ValueToKeyword(
> +          name = CounterStyleManager::GetStyleNameFromType(intValue);
> -                  intValue, nsCSSProps::kListStyleKTable));

What sort of assurance do we have here that |intValue| is in the correct range of builtin values? (and that it won't trigger an out-of-bounds read in the function that we're calling now)  The old code (using nsCSSProps::ValueToKeyword) would handle this case gracefully-ish by producing the empty string, but now we'll do an out-of-bounds read (and fail an assertion in debug builds).  So we need to reassure ourselves that such out-of-bounds values aren't possible here, now that the possibility is getting more dangerous.

So: please include a comment here to explain how we know it's in-range (ideally mentioning whatever nsCSSParser function, etc., that gives us that guarantee)
Attachment #8902198 - Flags: review?(dholbert) → review-
Comment on attachment 8902199 [details]
Bug 1393189 part 4 - Remove kListStyleKTable and CSS keywords only used in this table.

https://reviewboard.mozilla.org/r/173592/#review179268
Attachment #8902199 - Flags: review?(dholbert) → review+
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

> (1) Please declare this as 'nsIAtom** const' to indicate/validate that it's set in the init list and never changes.
> 
> (2) I didn't initially understand what the comment here meant, & why this needs to be a double-pointer.  Can you reword/extend this to clarify? "To point to nsGkAtoms" members is not really clear -- superficially, it seems like this could be simplified to a nsIAtom*.
> 
> (After some testing of me making that change: I think it's a double-pointer because we want to allow instances of this class to be instantiated by the compiler at compile-time, e.g. for the static_assert elsewhere in this patch, but we can't do that if this were a nsIAtom* whose value came from a "nsGkAtoms::foo" value, because nsGkAtoms::foo values are not themselves 'constexpr'. But we can use their address within the nsGkAtoms table -- that can be computed at compile time.  So -- I think I understand now, but only after modifying the code manually to try to make this a single-pointer and seeing how things broke.)

I have a feeling that this is a usual pattern to use `nsIAtom**` and `&nsGkAtoms::` for statically initialized data. See for example nsMediaFeature [1] and many others if you search `&nsGkAtoms::` in our codebase [2], so I don't really think it's worth more explanation.

We need to use `&nsGkAtoms::` because value of items in `nsGkAtoms` are initialized at runtime. That is because we need to deduplicate when creating those static atoms, and we can only do that at runtime. See the static atom register code [3] for more details. (We can probably try to do them at build time via some script, I guess.)

[1] https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/layout/style/nsMediaFeatures.h#25
[2] https://searchfox.org/mozilla-central/search?q=%26nsGkAtoms%3A%3A
[3] https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/xpcom/ds/nsAtomTable.cpp#660
Comment on attachment 8902198 [details]
Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable.

https://reviewboard.mozilla.org/r/173590/#review179256

> There's a comment above this section about how we're doing a case-insensitive comparison here.  (I think it was case-insensitive due to the nsCSSKeywords::LookupKeyword usage here -- since nsCSSKeywords uses a "nsStaticCaseInsensitiveNameTable" under the hood.)
> 
> 
> Is the new code still doing a case-insensitive comparison somehow, or is this patch accidentally converting it to be case-sensitive?  (It looks like it's an exact case-sensitive comparison, because it's checking for nsIAtom pointer-equality now.)
> 
> So I think this might be wrong & need some fixing. If I'm mistaken, though, then please make sure it's clear (via a comment) how this is still doing the right sort of comparison.

This is because the parser should lowercase all name of predefined counter styles (including all builtin ones). I don't think it's worth a code comment itself because lowercasing predefined counter style itself is a higher level requirement from the spec, and not just for builtin ones here (but also other predefined non-builtin ones). But I would put the justification into the commit message for this change.

> What sort of assurance do we have here that |intValue| is in the correct range of builtin values? (and that it won't trigger an out-of-bounds read in the function that we're calling now)  The old code (using nsCSSProps::ValueToKeyword) would handle this case gracefully-ish by producing the empty string, but now we'll do an out-of-bounds read (and fail an assertion in debug builds).  So we need to reassure ourselves that such out-of-bounds values aren't possible here, now that the possibility is getting more dangerous.
> 
> So: please include a comment here to explain how we know it's in-range (ideally mentioning whatever nsCSSParser function, etc., that gives us that guarantee)

Actually there is a comment before this whole switch block stating that the value never comes from CSS, they are from attribute map. I'll add some reference in that comment for where the values can come from.
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

> I have a feeling that this is a usual pattern to use `nsIAtom**` and `&nsGkAtoms::` for statically initialized data. See for example nsMediaFeature [1] and many others if you search `&nsGkAtoms::` in our codebase [2], so I don't really think it's worth more explanation.
> 
> We need to use `&nsGkAtoms::` because value of items in `nsGkAtoms` are initialized at runtime. That is because we need to deduplicate when creating those static atoms, and we can only do that at runtime. See the static atom register code [3] for more details. (We can probably try to do them at build time via some script, I guess.)
> 
> [1] https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/layout/style/nsMediaFeatures.h#25
> [2] https://searchfox.org/mozilla-central/search?q=%26nsGkAtoms%3A%3A
> [3] https://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/xpcom/ds/nsAtomTable.cpp#660

> I don't really think it's worth more explanation.

I disagree.  My concern is that the current documentation implies that you always need a double-pointer in order to point to a nsGkAtom member.  And that's simply not true -- and hence, it is confusing.

Could you just expand this a little, maybe to something like:
  // Extra indirection to point to nsGkAtoms members (whose nsIAtom*
  // pointer-values are determined at runtime -- hence we can't capture them
  // as nsIAtom* and still be constructed as 'constexpr')
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

> > I don't really think it's worth more explanation.
> 
> I disagree.  My concern is that the current documentation implies that you always need a double-pointer in order to point to a nsGkAtom member.  And that's simply not true -- and hence, it is confusing.
> 
> Could you just expand this a little, maybe to something like:
>   // Extra indirection to point to nsGkAtoms members (whose nsIAtom*
>   // pointer-values are determined at runtime -- hence we can't capture them
>   // as nsIAtom* and still be constructed as 'constexpr')

It's true that you always need a double-pointer in order to point to a member of `nsGkAtoms`, because those members themselves are `nsIAtom*` :)
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

> It's true that you always need a double-pointer in order to point to a member of `nsGkAtoms`, because those members themselves are `nsIAtom*` :)

ehh... *strictly* true I suppose. :) But still quite ambiguous to a human reader.


(supposing foo->mPresShell is a pointer, and I said "This variable points to foo->mPresShell", a reader could easily [mis]understand me to be discussing a single-pointer rather than a double-pointer.)
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179210

> ehh... *strictly* true I suppose. :) But still quite ambiguous to a human reader.
> 
> 
> (supposing foo->mPresShell is a pointer, and I said "This variable points to foo->mPresShell", a reader could easily [mis]understand me to be discussing a single-pointer rather than a double-pointer.)

Well, when you are saying you need to point to a member, it clearly means you need a pointer to the member, not the object behind, and in this case, when the member itself has a pointer type, we need a double-pointer to point to it, otherwise you should use a different expression (e.g. "this variable points to the same pres shell as foo->mPresShell, rather than "this variable points to mPresShell member of foo"). So I don't think this is that confusing.
Comment on attachment 8902488 [details]
Bug 1393189 part 0 - Reorder NS_STYLE_LIST_STYLE_* consts to match the order in nsCSSProps::kListStyleKTable.

https://reviewboard.mozilla.org/r/174072/#review179336
Attachment #8902488 - Flags: review?(dholbert) → review+
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review179338

::: layout/style/CounterStyleManager.cpp:615
(Diff revisions 1 - 2)
>    // Extra indirection to point to nsGkAtoms members.
> -  nsIAtom** mName;
> +  nsIAtom** const mName;

I still think an extra parenthetical comment here would be helpful to clarify the motivation, to clue the reader into the fact that (a) this class's instances are intended to be evaluated at compile-time,  (b) nsGkAtoms:: pointer-values are determined at runtime, (c) therefore we need an extra layer of indirection.

(Each of these facts is available somewhere in the codebase, but not together and not very close to this line, which is why this reasoning is non-obvious.)

I guess I won't tick "open an issue" because I don't want to hold the review on this, but I'd still encourage you to add some sort of helpful comment like this.
Attachment #8902196 - Flags: review?(dholbert) → review+
Comment on attachment 8902201 [details]
Bug 1393189 part 6 - Add crashtest for this bug.

https://reviewboard.mozilla.org/r/173596/#review179340
Attachment #8902201 - Flags: review?(cam) → review+
Comment on attachment 8902200 [details]
Bug 1393189 part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions.

https://reviewboard.mozilla.org/r/173594/#review179342

::: servo/components/style/gecko/values.rs:480
(Diff revision 2)
>                                       symbols.as_ptr(), symbols.len() as u32) };
>              }
>          }
>      }
>  
>      /// Convert Gecko CounterStylePtr to CounterStyleOrNone.

Update this comment now that we can return a String too?
Attachment #8902200 - Flags: review?(cam) → review+
Comment on attachment 8902198 [details]
Bug 1393189 part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable.

https://reviewboard.mozilla.org/r/173590/#review179344

::: layout/style/CounterStyleManager.cpp:2035
(Diff revision 2)
>    // It is intentional that the predefined names are case-insensitive
>    // but the user-defined names case-sensitive.
>    StyleSetHandle styleSet = mPresContext->StyleSet();
>    nsCSSCounterStyleRule* rule = styleSet->CounterStyleRuleForName(aName);
>    if (rule) {
>      MOZ_ASSERT(rule->Name() == aName);
>      data = new (mPresContext) CustomCounterStyle(aName, this, rule);
>    } else {
> -    int32_t type;
> -    nsDependentAtomString name(aName);
> +    for (const BuiltinCounterStyle& item : gBuiltinStyleTable) {
> +      if (item.GetStyleName() == aName) {

Thanks for adding the explanation in the commit message!

This preexisting code-comment is becoming confusing, though.  Could you also update this code-comment to keep it correct? It becomes stale/misleading as of this patch. 

(It suggests that the code that follows it will do a different sort of comparison [case-sensitivity-wise] in the two branches, but the code clearly does *not* do that.  Rather, both branches are now **case-sensitive** despite what this comment says -- and we're depending on earlier code to have adjusted the case for us, elsewhere, so that the case-sensitivity simply doesn't make a difference in one of the branches.)
Attachment #8902198 - Flags: review?(dholbert) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7aee62c706ab
part 0 - Reorder NS_STYLE_LIST_STYLE_* consts to match the order in nsCSSProps::kListStyleKTable. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d292ce405097
part 1 - Statically initialize builtin style table with their name atom included. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/1200482baf04
part 2 - Have CounterStyle::GetStyleName return nsIAtom and make it const. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/986b0a6f7173
part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d0cff01a73a0
part 4 - Remove kListStyleKTable and CSS keywords only used in this table. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/370357882bef
part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions. r=heycam
https://hg.mozilla.org/integration/autoland/rev/b0cc5205f6ed
part 6 - Add crashtest for this bug. r=heycam
Backed out for crashing e.g. in chrome's devtools/client/memory/test/chrome/test_List_01.html or losing connection while running many test suites, both on Windows:

servo part: https://hg.mozilla.org/integration/autoland/rev/adfc31a890bbdd1c8fbb365b8912094bf095a776

Gecko part:
https://hg.mozilla.org/integration/autoland/rev/48c3b47b97ac76c353cd2390730626750283b809
https://hg.mozilla.org/integration/autoland/rev/4960488f939ec95fb8152774d0230bbce7f9d242
https://hg.mozilla.org/integration/autoland/rev/657c8ee01fcfe0a2403ec1f705fd785b988ffa1d
https://hg.mozilla.org/integration/autoland/rev/095109d7ba80f68446fdb8b7b58949d5fb4fb5d4
https://hg.mozilla.org/integration/autoland/rev/381d831f79a182fcf6533482992bf63891c788ff
https://hg.mozilla.org/integration/autoland/rev/e949b2b4653449fd032c0ad26d43bcc6e8499ac0
https://hg.mozilla.org/integration/autoland/rev/8d70c25e8e2236a111b46b5fddeb9faba65b7d5d

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b0cc5205f6ed2d92ed4cbc7b6c75de2538cb6309&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Failure log devtools/client/memory/test/chrome/test_List_01.html: https://treeherder.mozilla.org/logviewer.html#?job_id=126979636&repo=autoland

23:41:59     INFO - TEST-START | devtools/client/memory/test/chrome/test_List_01.html
23:42:01     INFO - TEST-INFO | Main app process: exit 1
23:42:01     INFO - Buffered messages finished
23:42:01    ERROR - TEST-UNEXPECTED-FAIL | devtools/client/memory/test/chrome/test_List_01.html | application terminated with exit code 1
23:42:01     INFO - runtests.py | Application ran for: 0:00:11.977000
23:42:01     INFO - zombiecheck | Reading PID log: c:\users\cltbld~1.001\appdata\local\temp\tmpp1fdsipidlog
23:42:01     INFO - mozcrash Downloading symbols from: https://queue.taskcluster.net/v1/task/D6om9YiITCiCjAwx-41qGw/artifacts/public/build/target.crashreporter-symbols.zip
23:42:07     INFO - mozcrash Copy/paste: C:\slave\test\build\win32-minidump_stackwalk.exe c:\users\cltbld~1.001\appdata\local\temp\tmpjfkvyt.mozrunner\minidumps\514cf0d8-9c13-4a81-8116-948df53b10d9.dmp c:\users\cltbld~1.001\appdata\local\temp\tmpvrr_hb
23:42:12     INFO - mozcrash Saved minidump as C:\slave\test\build\blobber_upload_dir\514cf0d8-9c13-4a81-8116-948df53b10d9.dmp
23:42:12     INFO - mozcrash Saved app info as C:\slave\test\build\blobber_upload_dir\514cf0d8-9c13-4a81-8116-948df53b10d9.extra
23:42:12     INFO - PROCESS-CRASH | devtools/client/memory/test/chrome/test_List_01.html | application crashed [@ nsBlockFrame::SetInitialChildList(mozilla::layout::FrameChildListID,nsFrameList &)]
23:42:12     INFO - Crash dump filename: c:\users\cltbld~1.001\appdata\local\temp\tmpjfkvyt.mozrunner\minidumps\514cf0d8-9c13-4a81-8116-948df53b10d9.dmp
23:42:12     INFO - Operating system: Windows NT
23:42:12     INFO -                   6.2.9200 
23:42:12     INFO - CPU: amd64
23:42:12     INFO -      family 6 model 30 stepping 5
23:42:12     INFO -      8 CPUs
23:42:12     INFO - 
23:42:12     INFO - GPU: UNKNOWN
23:42:12     INFO - 
23:42:12     INFO - Crash reason:  EXCEPTION_ACCESS_VIOLATION_READ
23:42:12     INFO - Crash address: 0x20
23:42:12     INFO - Process uptime: 11 seconds
23:42:12     INFO - 
23:42:12     INFO - Thread 0 (crashed)
23:42:12     INFO -  0  xul.dll!nsBlockFrame::SetInitialChildList(mozilla::layout::FrameChildListID,nsFrameList &) [nsBlockFrame.cpp:b0cc5205f6ed : 7114 + 0x9]
23:42:12     INFO -     rax = 0x0000000000000000   rdx = 0x000000c32d24b4c8
23:42:12     INFO -     rcx = 0x000007fa95694d80   rbx = 0x000000c32d24b400
23:42:12     INFO -     rsi = 0x0000000000000000   rdi = 0x000000c32d24b760
23:42:12     INFO -     rbp = 0x000000c3058aced9   rsp = 0x000000c3058acde0
23:42:12     INFO -      r8 = 0x000000c32940d048    r9 = 0x0000000000400080
23:42:12     INFO -     r10 = 0x0000000000000000   r11 = 0x0000000000000001
23:42:12     INFO -     r12 = 0x000000c3058adc20   r13 = 0x000000c32c61f280
23:42:12     INFO -     r14 = 0x000000c3058acfc0   r15 = 0x000000c32d24b760
23:42:12     INFO -     rip = 0x000007fa93ec0bd4
23:42:12     INFO -     Found by: given as instruction pointer in context
23:42:12     INFO -  1  xul.dll!nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState &,nsIContent *,nsContainerFrame *,nsContainerFrame *,nsStyleContext *,nsContainerFrame * *,nsFrameItems &,nsIFrame *,PendingBinding *) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 12464 + 0x15]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ace10   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e40caa
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  2  xul.dll!nsCSSFrameConstructor::ConstructNonScrollableBlockWithConstructor(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItem &,nsContainerFrame *,nsStyleDisplay const *,nsFrameItems &,nsBlockFrame * (*)(nsIPresShell *,nsStyleContext *)) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 5109 + 0x65]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058acf20   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e4309d
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  3  xul.dll!nsCSSFrameConstructor::ConstructNonScrollableBlock(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItem &,nsContainerFrame *,nsStyleDisplay const *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 5073 + 0x25]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058acfa0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42f8d
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  4  xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem &,nsFrameConstructorState &,nsContainerFrame *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 4017 + 0x20]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058acff0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e41f55
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  5  xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItemList::Iterator &,nsContainerFrame *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 6410 + 0x23]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad280   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42bf5
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  6  xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItemList &,nsContainerFrame *,bool,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 11065 + 0x18]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad300   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42ca1
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  7  xul.dll!nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState &,nsIContent *,nsStyleContext *,nsContainerFrame *,bool,nsFrameItems &,bool,PendingBinding *,nsIFrame *) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 11379 + 0x23]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad360   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e61c92
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  8  xul.dll!nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState &,nsIContent *,nsContainerFrame *,nsContainerFrame *,nsStyleContext *,nsContainerFrame * *,nsFrameItems &,nsIFrame *,PendingBinding *) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 12460 + 0x43]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad600   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e40c95
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO -  9  xul.dll!nsCSSFrameConstructor::ConstructNonScrollableBlockWithConstructor(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItem &,nsContainerFrame *,nsStyleDisplay const *,nsFrameItems &,nsBlockFrame * (*)(nsIPresShell *,nsStyleContext *)) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 5109 + 0x65]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad710   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e4309d
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 10  xul.dll!nsCSSFrameConstructor::ConstructNonScrollableBlock(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItem &,nsContainerFrame *,nsStyleDisplay const *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 5073 + 0x25]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad790   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42f8d
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 11  xul.dll!nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem &,nsFrameConstructorState &,nsContainerFrame *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 4017 + 0x20]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ad7e0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e41f55
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 12  xul.dll!nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItemList::Iterator &,nsContainerFrame *,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 6410 + 0x23]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ada70   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42bf5
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 13  xul.dll!nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState &,nsCSSFrameConstructor::FrameConstructionItemList &,nsContainerFrame *,bool,nsFrameItems &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 11065 + 0x18]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058adaf0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e42ca1
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 14  xul.dll!nsCSSFrameConstructor::ContentAppended(nsIContent *,nsIContent *,bool,bool,TreeMatchContext *) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 7882 + 0x4f]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058adb50   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e44bb9
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 15  xul.dll!nsCSSFrameConstructor::CreateNeededFrames(nsIContent *,TreeMatchContext &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 7400 + 0x1d]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058adfb0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e487c8
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 16  xul.dll!nsCSSFrameConstructor::CreateNeededFrames(nsIContent *,TreeMatchContext &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 7422 + 0xf]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae080   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e488ed
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 17  xul.dll!nsCSSFrameConstructor::CreateNeededFrames(nsIContent *,TreeMatchContext &) [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 7422 + 0xf]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae150   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e488ed
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 18  xul.dll!nsCSSFrameConstructor::CreateNeededFrames() [nsCSSFrameConstructor.cpp:b0cc5205f6ed : 7441 + 0x10]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae220   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e489d1
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 19  xul.dll!mozilla::GeckoRestyleManager::ProcessPendingRestyles() [GeckoRestyleManager.cpp:b0cc5205f6ed : 540 + 0x11]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae450   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e278b7
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 20  xul.dll!mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) [PresShell.cpp:b0cc5205f6ed : 4183 + 0x1c]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae480   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e16d16
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 21  xul.dll!nsRefreshDriver::Tick(__int64,mozilla::TimeStamp) [nsRefreshDriver.cpp:b0cc5205f6ed : 1921 + 0x29]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058ae4d0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0a817
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 22  xul.dll!mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver *,__int64,mozilla::TimeStamp) [nsRefreshDriver.cpp:b0cc5205f6ed : 337 + 0x23]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aeb40   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0b3fd
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 23  xul.dll!mozilla::RefreshDriverTimer::TickRefreshDrivers(__int64,mozilla::TimeStamp,nsTArray<RefPtr<nsRefreshDriver> > &) [nsRefreshDriver.cpp:b0cc5205f6ed : 307 + 0x25]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aeba0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0b721
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 24  xul.dll!mozilla::RefreshDriverTimer::Tick(__int64,mozilla::TimeStamp) [nsRefreshDriver.cpp:b0cc5205f6ed : 328 + 0x24]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aec00   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0a1c6
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 25  xul.dll!mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) [nsRefreshDriver.cpp:b0cc5205f6ed : 770 + 0x29]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aec90   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0993c
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 26  xul.dll!mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) [nsRefreshDriver.cpp:b0cc5205f6ed : 683 + 0x1a]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aecf0   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0b61b
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 27  xul.dll!mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() [nsRefreshDriver.cpp:b0cc5205f6ed : 529 + 0x1d]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aed40   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93e0950f
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 28  xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:b0cc5205f6ed : 1039 + 0x9]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058aed90   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa926c420d
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 29  xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:b0cc5205f6ed : 521 + 0xd]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058af440   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa926c333f
23:42:12     INFO -     Found by: call frame info
23:42:12     INFO - 30  xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [MessagePump.cpp:b0cc5205f6ed : 97 + 0xa]
23:42:12     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:12     INFO -     rsp = 0x000000c3058af470   r12 = 0x000000c3058adc20
23:42:12     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:12     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa929f5e56
23:42:12     INFO -     Found by: call frame info
23:42:13     INFO - 31  xul.dll!MessageLoop::RunHandler() [message_loop.cc:b0cc5205f6ed : 319 + 0xf]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af4d0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa929d8693
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 32  xul.dll!MessageLoop::Run() [message_loop.cc:b0cc5205f6ed : 299 + 0x8]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af500   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa929d8396
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 33  xul.dll!nsBaseAppShell::Run() [nsBaseAppShell.cpp:b0cc5205f6ed : 158 + 0xd]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af550   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93c4d3f7
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 34  xul.dll!nsAppShell::Run() [nsAppShell.cpp:b0cc5205f6ed : 230 + 0x8]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af580   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa93c9bb27
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 35  xul.dll!nsAppStartup::Run() [nsAppStartup.cpp:b0cc5205f6ed : 288 + 0xa]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af5b0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa94a4a8cf
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 36  xul.dll!XREMain::XRE_mainRun() [nsAppRunner.cpp:b0cc5205f6ed : 4645 + 0xb]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af5e0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa94ab6147
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 37  xul.dll!XREMain::XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [nsAppRunner.cpp:b0cc5205f6ed : 4809 + 0x8]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af870   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa94ab4253
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 38  xul.dll!XRE_main(int,char * * const,mozilla::BootstrapConfig const &) [nsAppRunner.cpp:b0cc5205f6ed : 4904 + 0x12]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058af950   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fa94ab3cec
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 39  firefox.exe!do_main [nsBrowserApp.cpp:b0cc5205f6ed : 236 + 0x21]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058afaf0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007f7b86218e0
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 40  firefox.exe!NS_internal_main(int,char * *,char * *) [nsBrowserApp.cpp:b0cc5205f6ed : 309 + 0xd]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058afcb0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007f7b8621410
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 41  firefox.exe!wmain [nsWindowsWMain.cpp:b0cc5205f6ed : 115 + 0x14]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058afd20   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007f7b8621b97
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 42  firefox.exe!__scrt_common_main_seh [exe_common.inl : 253 + 0x22]
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058afd80   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007f7b864c271
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 43  kernel32.dll!BaseThreadInitThunk + 0x1a
23:42:13     INFO -     rbx = 0x000000c32d24b400   rbp = 0x000000c3058aced9
23:42:13     INFO -     rsp = 0x000000c3058afdc0   r12 = 0x000000c3058adc20
23:42:13     INFO -     r13 = 0x000000c32c61f280   r14 = 0x000000c3058acfc0
23:42:13     INFO -     r15 = 0x000000c32d24b760   rip = 0x000007fab03f167e
23:42:13     INFO -     Found by: call frame info
23:42:13     INFO - 44  ntdll.dll!RtlUserThreadStart + 0x21
23:42:13     INFO -     rsp = 0x000000c3058afdf0   rip = 0x000007fab23ac3f1
23:42:13     INFO -     Found by: stack scanning
23:42:13     INFO - 45  KERNELBASE.dll!GetLegacyComposition + 0x1180
23:42:13     INFO -     rsp = 0x000000c3058afe20   rip = 0x000007faaf4709d0
23:42:13     INFO -     Found by: stack scanning


Failure log connection lost: https://treeherder.mozilla.org/logviewer.html#?job_id=126975530&repo=autoland

06:23:32     INFO -  TEST-START | dom/canvas/test/webgl-conf/generated/test_conformance__extensions__webgl-compressed-texture-size-limit.html
06:23:32     INFO -  TEST-SKIP | dom/canvas/test/webgl-conf/generated/test_conformance__extensions__webgl-compressed-texture-size-limit.html | took 0ms
06:23:32     INFO -  Running manifest: dom\canvas\test\webgl-conf\generated-mochitest.ini
06:23:32     INFO -  Z:\task_1504070343\build\tests\bin\pk12util.exe: PKCS12 IMPORT SUCCESSFUL
06:23:32     INFO -  MochitestServer : launching [u'Z:\\task_1504070343\\build\\tests\\bin\\xpcshell.exe', '-g', 'Z:\\task_1504070343\\build\\application\\firefox', '-v', '170', '-f', 'Z:\\task_1504070343\\build\\tests\\bin\\components\\httpd.js', '-e', "const _PROFILE_PATH = 'c:\\\\users\\\\genericworker\\\\appdata\\\\local\\\\temp\\\\tmpigf5zc.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '127.0.0.1'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', 'Z:\\task_1504070343\\build\\tests\\mochitest\\server.js']
06:23:32     INFO -  runtests.py | Server pid: 2716
06:23:32     INFO -  runtests.py | Websocket server pid: 3192
06:23:32     INFO -  runtests.py | SSL tunnel pid: 716
06:23:32     INFO -  Couldn't convert chrome URL: chrome://branding/locale/brand.properties
06:23:32     INFO -  [2716] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 85
06:23:32     INFO -  [2716] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 85
06:23:32     INFO -  [2716] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file z:/build/build/src/dom/media/CubebUtils.cpp, line 370
06:23:32     INFO -  runtests.py | Running with e10s: False
06:23:32     INFO -  runtests.py | Running tests: start.
06:23:32     INFO -  Application command: Z:\task_1504070343\build\application\firefox\firefox.exe -marionette -foreground -profile c:\users\genericworker\appdata\local\temp\tmpigf5zc.mozrunner
06:23:32     INFO -  runtests.py | Application pid: 816
06:23:32     INFO -  TEST-INFO | started process GECKO(816)
06:23:33     INFO -  GECKO(816) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\genericworker\appdata\local\temp\tmpigf5zc.mozrunner\runtests_leaks.log
06:23:33     INFO -  GECKO(816) | [816] WARNING: Failed to load startupcache file correctly, removing!: file z:/build/build/src/startupcache/StartupCache.cpp, line 218
06:23:33     INFO -  GECKO(816) | 1504074213609	addons.xpi	WARN	Error parsing extensions state: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [amIAddonManagerStartup.readStartupData]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm :: loadExtensionState :: line 1505"  data: no] Stack trace: loadExtensionState()@resource://gre/modules/addons/XPIProvider.jsm:1505 < getInstallState()@resource://gre/modules/addons/XPIProvider.jsm:1540 < checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3075 < startup()@resource://gre/modules/addons/XPIProvider.jsm:2142 < callProvider()@resource://gre/modules/AddonManager.jsm:263 < _startProvider()@resource://gre/modules/AddonManager.jsm:731 < startup()@resource://gre/modules/AddonManager.jsm:898 < startup()@resource://gre/modules/AddonManager.jsm:3082 < observe()@jar:file:///Z:/task_1504070343/build/application/firefox/omni.ja!/components/addonManager.js:65
06:23:33     INFO -  GECKO(816) | [816] WARNING: This method is lossy. Use GetCanonicalPath !: file z:/build/build/src/xpcom/io/nsLocalFileWin.cpp, line 3571
06:23:35     INFO -  GECKO(816) | ++DOCSHELL 18AE5400 == 1 [pid = 816] [id = {a34f893c-ed4a-429f-99c6-18283f30b5c7}]
06:23:35     INFO -  GECKO(816) | ++DOMWINDOW == 1 (17CC3400) [pid = 816] [serial = 1] [outer = 00000000]
06:23:35     INFO -  GECKO(816) | ++DOMWINDOW == 2 (17CC3C00) [pid = 816] [serial = 2] [outer = 17CC3400]
06:23:35     INFO -  GECKO(816) | [816] WARNING: 'NS_FAILED(GetAccentColor(unused))', file z:/build/build/src/widget/windows/nsLookAndFeel.cpp, line 450
06:23:35     INFO -  GECKO(816) | [816] WARNING: DWM not enabled, falling back to software vsync: file z:/build/build/src/gfx/thebes/gfxWindowsPlatform.cpp, line 1904
06:23:39     INFO -  GECKO(816) | 1504074219758	Marionette	INFO	Enabled via --marionette
06:23:39     INFO -  GECKO(816) | ++DOCSHELL 0DE7D800 == 2 [pid = 816] [id = {9b2a089a-1782-42f3-ad96-c764ab59ec27}]
06:23:39     INFO -  GECKO(816) | ++DOMWINDOW == 3 (0DE7DC00) [pid = 816] [serial = 3] [outer = 00000000]
06:23:39     INFO -  GECKO(816) | ++DOMWINDOW == 4 (0DE7E400) [pid = 816] [serial = 4] [outer = 0DE7DC00]
06:23:39     INFO -  GECKO(816) | ++DOCSHELL 1BF2E800 == 3 [pid = 816] [id = {d7c64ba5-e816-44a8-8a77-f510bc83d565}]
06:23:39     INFO -  GECKO(816) | ++DOMWINDOW == 5 (1BF2EC00) [pid = 816] [serial = 5] [outer = 00000000]
06:23:39     INFO -  GECKO(816) | ++DOMWINDOW == 6 (1BF2F400) [pid = 816] [serial = 6] [outer = 1BF2EC00]
06:23:40     INFO -  GECKO(816) | ++DOMWINDOW == 7 (1C9F1C00) [pid = 816] [serial = 7] [outer = 0DE7DC00]
06:23:40     INFO -  GECKO(816) | ++DOCSHELL 1D144400 == 4 [pid = 816] [id = {f83ec8e3-60c6-46cd-8230-a0213bdb4455}]
06:23:40     INFO -  GECKO(816) | ++DOMWINDOW == 8 (1D144C00) [pid = 816] [serial = 8] [outer = 00000000]
06:23:40     INFO -  GECKO(816) | ++DOCSHELL 17CC5000 == 5 [pid = 816] [id = {2485be94-70cf-4905-b4a9-0ea3f79000c8}]
06:23:40     INFO -  GECKO(816) | ++DOMWINDOW == 9 (1CB48400) [pid = 816] [serial = 9] [outer = 00000000]
06:23:40     INFO -  GECKO(816) | ++DOMWINDOW == 10 (1D4CE400) [pid = 816] [serial = 10] [outer = 1CB48400]
06:23:40     INFO -  GECKO(816) | ++DOMWINDOW == 11 (18C41C00) [pid = 816] [serial = 11] [outer = 1CB48400]
06:25:33     INFO -  Traceback (most recent call last):
06:25:33     INFO -    File "Z:\task_1504070343\build\tests\mochitest\runtests.py", line 2635, in doTests
06:25:33     INFO -      marionette_args=marionette_args,
06:25:33     INFO -    File "Z:\task_1504070343\build\tests\mochitest\runtests.py", line 2165, in runApp
06:25:33     INFO -      self.marionette.start_session(timeout=port_timeout)
06:25:33     INFO -    File "Z:\task_1504070343\build\venv\lib\site-packages\marionette_driver\decorators.py", line 28, in _
06:25:34     INFO -      m._handle_socket_failure()
06:25:34     INFO -    File "Z:\task_1504070343\build\venv\lib\site-packages\marionette_driver\decorators.py", line 23, in _
06:25:34     INFO -      return func(*args, **kwargs)
06:25:34     INFO -    File "Z:\task_1504070343\build\venv\lib\site-packages\marionette_driver\marionette.py", line 1203, in start_session
06:25:34     INFO -      self.protocol, _ = self.client.connect()
06:25:34     INFO -    File "Z:\task_1504070343\build\venv\lib\site-packages\marionette_driver\transport.py", line 223, in connect
06:25:34     INFO -      self.sock.connect((self.addr, self.port))
06:25:34     INFO -    File "c:\mozilla-build\python\Lib\socket.py", line 228, in meth
06:25:34     INFO -      return getattr(self._sock,name)(*args)
06:25:34     INFO -  error: [Errno 10061] No connection could be made because the target machine actively refused it
06:25:34     INFO -  0 ERROR Automation Error: Received unexpected exception while running application
Flags: needinfo?(xidorn+moz)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fcfaa1a9b106
Revert "Backed out changeset 11803fe39188 as being the servo side of bug 1393189. r=backout on a CLOSED TREE"
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/fb56ee6c4eb7
Revert "Backed out changeset 11803fe39188 as being the servo side of bug 1393189. r=backout on a CLOSED TREE"
OK, so this is a bug on MSVC 2015 that, constexpr object would have their vtable pointer being null. The workaround is to make it const rather than constexpr for MSVC 2015.

Will re-land the patches soon.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2de8fc839ce462f31a257496ef7165812dcdbe32
Flags: needinfo?(xidorn+moz)
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0f581ed201f
part 0 - Reorder NS_STYLE_LIST_STYLE_* consts to match the order in nsCSSProps::kListStyleKTable. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/f0481aa56d17
part 1 - Statically initialize builtin style table with their name atom included. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/92f380250d23
part 2 - Have CounterStyle::GetStyleName return nsIAtom and make it const. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/c69cd397ec43
part 3 - Replace all uses of nsCSSProps::kListStyleKTable with gBuiltinStyleTable. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/e2e8b462c00e
part 4 - Remove kListStyleKTable and CSS keywords only used in this table. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/ea351997a8aa
part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions. r=heycam
https://hg.mozilla.org/integration/autoland/rev/02d4f687f0e6
part 6 - Add crashtest for this bug. r=heycam
I believe this has lead to a regression in my local builds where the assertion in CounterStylePtr& oeprator=(CounterStyle* aCounterStyle) is firing.. It appears to have an aCoutnerStyle object with an invalid vtable ptr.
Comment on attachment 8902196 [details]
Bug 1393189 part 1 - Statically initialize builtin style table with their name atom included.

https://reviewboard.mozilla.org/r/173586/#review180172

::: layout/style/CounterStyleManager.cpp:966
(Diff revision 4)
>    }
>  }
>  
> +// MSVC 2015 has a bug that vtable pointer of constexpr objects is null,
> +// which would cause startup crash. So const is used instead here for
> +// pre-2017 version of MSVC to workaround this issue.

This is the cause of my issue. It appears 2017 still makes vtable pointers of constexpr objects be null.
Flags: needinfo?(xidorn+moz)
I can confirm that changing the ifdefs to just !defined(_MSC_VER) fixes the issues.
(In reply to Bas Schouten (:bas.schouten) from comment #55)
> I can confirm that changing the ifdefs to just !defined(_MSC_VER) fixes the
> issues.

It works fine for me locally, and I'm using MSVC2017. My guess is that, this bug existed in some early version of MSVC2017, and after some updates, it gets fixed.

My local version is Visual Studio 15.3.3, whose corresponding cl version is 19.11.25507.1. You may want to upgrade you MSVC install to latest version.

Also, I guess we can change the version check from 1910 to 1911. Could you confirm if that works for you?
Flags: needinfo?(xidorn+moz)
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox-esr52: --- → unaffected
Flags: in-testsuite? → in-testsuite+
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #56)
> (In reply to Bas Schouten (:bas.schouten) from comment #55)
> > I can confirm that changing the ifdefs to just !defined(_MSC_VER) fixes the
> > issues.
> 
> It works fine for me locally, and I'm using MSVC2017. My guess is that, this
> bug existed in some early version of MSVC2017, and after some updates, it
> gets fixed.
> 
> My local version is Visual Studio 15.3.3, whose corresponding cl version is
> 19.11.25507.1. You may want to upgrade you MSVC install to latest version.
> 
> Also, I guess we can change the version check from 1910 to 1911. Could you
> confirm if that works for you?

I ran into the same issue as Bas. I see I have an update for VS 2017 so I'll give that a try.

I can also confirm that changing the version check from 1910 to 1911 also fixes the issue for me.
Bug 1424281 just made VS2017 15.6 the minimum VS version for Firefox 61, so we can remove this constexpr/const workaround needed for VS2015 and early VS2017 versions. Bug 1445105 is the meta bug tracking VS workarounds that can be removed now that we're using 15.6.

https://hg.mozilla.org/mozilla-central/rev/f0481aa56d17#l4.78
See Also: → 1445105
Depends on: 1448436
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: