Closed Bug 1359217 Opened 7 years ago Closed 7 years ago

Stylo: Opening DevTools inspector crashes during `inDOMUtils::GetCSSStyleRules`

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- disabled
firefox56 --- fixed

People

(Reporter: jryans, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

(Keywords: crash)

Attachments

(8 files, 8 obsolete files)

Bug 1359217 part 3 - Remove unused ErrorResult param from GetCssRulesInternal.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 4 - Make ServoStyleSheet::GetCssRulesInternal return ServoCSSRuleList.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 0 - Some fixup for later patches.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 1 - Make document nsIDocumentObserver::StyleRule* methods include the rule as parameter.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 1359217 part 2 - Include the import rule of child sheet when notifying StyleRuleAdded.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 5 - Fix reversed condition for inserting import rule.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1359217 part 7 - Add test for getCSSStyleRules.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
STR:

1. Go to http://example.com in a Stylo build
2. Open the DevTools inspector panel

ER:

No crash.

AR:

We crash while AddRef'ing a bad pointer, looks like line 305 from inDOMUtils.cpp, which is:

frame #4: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(this=<unavailable>, aElement=<unavailable>, aPseudo=<unavailable>, _retval=<unavailable>) at inDOMUtils.cpp:305 [opt]
   302 	      ServoStyleRule* rule;
   303 	      rawRulesToRules.Get(rawRule, &rule);
   304 	      MOZ_ASSERT(rule, "We should always be able to map a raw rule to a rule.");
-> 305 	      RefPtr<css::Rule> ruleObj(rule);
   306 	      rules->AppendElement(ruleObj, false);
   307 	    }
   308 	  }

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x1)
  * frame #0: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(nsIDOMElement*, nsAString const&, nsIArrayExtensions**) [inlined] mozilla::RefPtrTraits<mozilla::css::Rule>::AddRef(aPtr=0x0000000000000001) at RefPtr.h:37 [opt]
    frame #1: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(nsIDOMElement*, nsAString const&, nsIArrayExtensions**) [inlined] RefPtr<mozilla::css::Rule>::ConstRemovingRefPtrTraits<mozilla::css::Rule>::AddRef(aPtr=0x0000000000000001) at RefPtr.h:392 [opt]
    frame #2: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(nsIDOMElement*, nsAString const&, nsIArrayExtensions**) [inlined] RefPtr<mozilla::css::Rule>::RefPtr(aRawPtr=0x0000000000000001) at RefPtr.h:111 [opt]
    frame #3: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(nsIDOMElement*, nsAString const&, nsIArrayExtensions**) [inlined] RefPtr<mozilla::css::Rule>::RefPtr(aRawPtr=0x0000000000000001) at RefPtr.h:109 [opt]
    frame #4: 0x0000000104b7fd81 XUL`inDOMUtils::GetCSSStyleRules(this=<unavailable>, aElement=<unavailable>, aPseudo=<unavailable>, _retval=<unavailable>) at inDOMUtils.cpp:305 [opt]
    frame #5: 0x0000000101c43a9e XUL`NS_InvokeByIndex at xptcinvoke_asm_x86_64_unix.S:129
    frame #6: 0x00000001026b50e6 XUL`CallMethodHelper::Call() [inlined] CallMethodHelper::Invoke(this=0x00007fff5fbf8230) at XPCWrappedNative.cpp:2010 [opt]
    frame #7: 0x00000001026b50cf XUL`CallMethodHelper::Call(this=<unavailable>) at XPCWrappedNative.cpp:1329 [opt]
    frame #8: 0x00000001026b7217 XUL`XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) [inlined] XPCWrappedNative::CallMethod(ccx=<unavailable>) at XPCWrappedNative.cpp:1296 [opt]
    frame #9: 0x00000001026b71e6 XUL`XPC_WN_CallMethod(cx=<unavailable>, argc=<unavailable>, vp=0x00007fff5fbf83c8) at XPCWrappedNativeJSOps.cpp:983 [opt]
    frame #10: 0x0000122b192f27df
:bholley mentioned in today's meeting that :xidorn should look into this.
Flags: needinfo?(xidorn+moz)
Assignee: nobody → xidorn+moz
Priority: -- → P1
Fwiw, the problem is that the code that fills the rawRulesToRules hashtable is broken in multiple ways:

1)  It only walks document stylesheets, whereas the rule list can contain rules
    from UA and user sheets, as well as from the mAdditionalSheets bits in nsIDocument.
    I expect this is the main problem on example.com
2)  It doesn't walk through group rules other than MEDIA_RULE.
3)  It doesn't walk through child sheets (@import).

In general, it seems very fragile and somewhat expensive.  It would be much better if we could directly map back from a RawServoStyleRule* to its DOM representation...
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #2)
> 1)  It only walks document stylesheets, whereas the rule list can contain rules
>     from UA and user sheets, as well as from the mAdditionalSheets bits in nsIDocument.
>     I expect this is the main problem on example.com

Hmmm, probably we should just walk through all of them and create the CSSOM wrapper objects.

> 2)  It doesn't walk through group rules other than MEDIA_RULE.
> 3)  It doesn't walk through child sheets (@import).

Others are not implemented yet... but shouldn't be too hard to fix.

> In general, it seems very fragile and somewhat expensive.  It would be much
> better if we could directly map back from a RawServoStyleRule* to its DOM
> representation...

It seems to be fragile because of lack of some unimplemented things...

And yeah, it is expensive. If we want to make it cheaper, we may want to cache the hash table somehow, but that would make things more fragile since we need to be careful about when we need to rebuild that map.

Maybe we can store the pointer of ServoStyleRule in the servo side StyleRule, then set / clear it when construct / destruct the corresponding wrapper object.

We would still need to walk through the CSSOM tree to build up the objects when someone hasn't been constructed. But that is probably cheaper than walking through every time?
If the first one is the only relevant one here, we can probably just fix the collecting code in this bug. If 2 and 3 are also relevant, I probably should implement those bits first.
Depends on: 1352968, 1355394
Flags: needinfo?(xidorn+moz)
> It seems to be fragile because of lack of some unimplemented things...

No, it's fragile because it's duplicating logic found elsewhere (actual "find all the matching rules" code) and will do bad things if the two ever get out of sync for any reason.

> Maybe we can store the pointer of ServoStyleRule in the servo side StyleRule

That would make a lot more sense.

> We would still need to walk through the CSSOM tree to build up the objects

Why?  Why can't we just trigger construction of a ServoStyleRule for a servo-side StyleRule directly?

> If the first one is the only relevant one here, we can probably just fix the collecting code in this bug.

For purposes of fixing this crash, the important part is that this code be in sync with the code that actually does rule matching.  If stylo doesn't implement non-@media grouping rules and doesn't implement @import (I mean on the matching level, not the CSSOM level!), then my item (1) is the only relevant one.  If it does implement those, then items (2) and (3) are relevant.
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #5)
> > It seems to be fragile because of lack of some unimplemented things...
> 
> No, it's fragile because it's duplicating logic found elsewhere (actual
> "find all the matching rules" code) and will do bad things if the two ever
> get out of sync for any reason.
> 
> > Maybe we can store the pointer of ServoStyleRule in the servo side StyleRule
> 
> That would make a lot more sense.
> 
> > We would still need to walk through the CSSOM tree to build up the objects
> 
> Why?  Why can't we just trigger construction of a ServoStyleRule for a
> servo-side StyleRule directly?

That means we would need to construct lots of additional objects which are not used at all in majority of cases. Also having Servo objects own Gecko refcounted objects, especially those can involve cycle reference, has the danger of either leaking or double free, so I think we should avoid it as much as possible. We've done that for CSSFontFaceRule with some trick (see bug 1345696 part 6), but I think we should eventually move away from that.

Also, if we don't walk through the CSSOM tree to link up the whole tree, but instead just construct the separate objects, we may need more complicated mechanism to setup their stylesheet pointer, otherwise changing on those rules may not trigger restyle correctly.
> That means we would need to construct lots of additional objects which are not used at all 

No, I meant why can't we have a method that takes a servo-side StyleRule and constructs the ServoStyleRule?  I'm not saying to do that construction eagerly.

I agree that some other CSSOM-walking may be involved.  What I am suggesting is that we start with the rule we want to construct and construct things as needed (its parent stylesheet, etc), not try to start at the "root" of the CSSOM and walk our way to this rule.  That should be much harder to mess up, I would think.
Because you don't know what are the parent rule and stylesheet of a StyleRule without walking through the tree. That object doesn't have any up pointer.
Ah.  That's ... really annoying.

In that case we do need to do the walk but at the very least:

1)  We need to be _extremely_ sure that it considers all possible sources of style rules.  We need to have some sort of pretty exhaustive tests for it, etc.

2)  We need to have some sort of dirty flag so we don't redo the walk when not needed.  Otherwise we're going to seriously regress inspector performance, and it's already not great.
Bug 1357716 has a patch that changes this from a crash to a warning, but doesn't fix the underlying problem of missing ServoStyleRules.
Depends on: 1357716
Why have this table rather than store a raw pointer to the C++ ServoStyleRule object in the Rust StyleRule object?  Is it to save memory when we don't need this mapping?  (Having the pointer stored in the StyleRule seems safer, since we don't have to worry about clearing entries from the table in response to the "style sheet/rule removed" notifications.)
That's a good question. I've never thought about that way.

We sill need to walk through the CSSOM tree, so an observer like that is necessary anyway to avoid walk through the tree for every lookup.

Storing a raw pointer of the wrapper object in Rust StyleRule would cost an additional word for StyleRule and an additional FFI call to set that pointer. Both of them would be there regardless of whether we need it. But they probably don't matter a lot. The good part of doing that is, we don't need to invalidate data when rule/sheet is removed, so we can avoid certain map rebuilding.

There isn't really anything unsafe, I suppose, because we shouldn't be queried for a raw rule pointer which we haven't updated.

Since we need to walk through the CSSOM tree anyway, I have a feeling that collecting things into a unified map is probably simpler than storing the pointer in Rust side, codewise. And doing a C++ side-only change makes it easier for landing as well :)

Since the mapping itself isn't very complicated (on top of the observer we need anyway), I suggest we go this way for now. And if it turns out that this isn't safe enough and/or there are cases the performance would benefit a lot from storing the pointer inside Rust object, we can change to that way then.

WDYT?
Flags: needinfo?(cam)
I can probably try implementing the different way, and see how that would look like.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #17)
> We sill need to walk through the CSSOM tree, so an observer like that is
> necessary anyway to avoid walk through the tree for every lookup.

Do we need an observer for that?  We could just do a CSSOM tree walk if we happen to find any Rust StyleRule object that had a null ServoStyleRule pointer.

> Storing a raw pointer of the wrapper object in Rust StyleRule would cost an
> additional word for StyleRule and an additional FFI call to set that
> pointer.

The additional FFI calls wouldn't be needed for inDOMUtils::GetCSSRules, since Servo_Element_GetStyleRuleList could be made to return the ServoStyleRule pointers directly.

> Both of them would be there regardless of whether we need it. But
> they probably don't matter a lot. The good part of doing that is, we don't
> need to invalidate data when rule/sheet is removed, so we can avoid certain
> map rebuilding.

Yeah that's true.

> There isn't really anything unsafe, I suppose, because we shouldn't be
> queried for a raw rule pointer which we haven't updated.

Maybe.  It makes me kind of nervous to have a big table of raw pointers, and to ensure that we are calling StyleRuleRemoved etc. all at the right times...

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #18)
> I can probably try implementing the different way, and see how that would
> look like.

Yeah, I wouldn't mind seeing how it goes.  Thanks!
Flags: needinfo?(cam)
(In reply to Cameron McCormack (:heycam) from comment #19)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #17)
> > We sill need to walk through the CSSOM tree, so an observer like that is
> > necessary anyway to avoid walk through the tree for every lookup.
> 
> Do we need an observer for that?  We could just do a CSSOM tree walk if we
> happen to find any Rust StyleRule object that had a null ServoStyleRule
> pointer.

Hmmm, that might be a good idea. So Servo_Element_GetStyleRuleList would fail if any style rule doesn't have wrapper constructed, and then we walk through the tree and call it again.

If we don't use observer, we can remove part 1, which is good.

But that approach is approximately same as walking through all styles every time any style rule changes, which is probably not very optimal, especially given that user can add / remove style rules in inspector directly.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #20)
> But that approach is approximately same as walking through all styles every
> time any style rule changes, which is probably not very optimal, especially
> given that user can add / remove style rules in inspector directly.

That's probably not an issue, as far as we keep our current behavior that we always create wrapper object for newly inserted rule.

Hmmm... okay, it sounds like we can actually get rid of the observer.
Attachment #8876981 - Attachment is obsolete: true
Attachment #8876981 - Flags: review?(bzbarsky)
Attachment #8876982 - Attachment is obsolete: true
Attachment #8876982 - Flags: review?(cam)
Attachment #8876983 - Attachment is obsolete: true
Attachment #8876983 - Flags: review?(cam)
Attachment #8876984 - Attachment is obsolete: true
Attachment #8876984 - Flags: review?(cam)
So this is the alternative approach proposed by heycam. It is fine, although it somehow spreads things into different places rather then keep them inside a single module.

This doesn't include any optimization, so any addition of style sheet or media / supports rule may trigger a full walk through all stylesheets when we need to list it. This is probably not very bad, because those things shouldn't happen a lot during using devtools.

A simple optimization would be putting a flag into stylesheet to indicate that all style rules inside has been constructed, then there are two ways to use this flag:
1. Use this flag to fast skip stylesheets we don't need to walk through again, and unset the flag when any change is made.
2. Try to keep the flag persists after it is set, which means aggressively walk through any new rule added.

The second one is probably slightly more complicated than the first one. I guess the first one is simple enough so maybe we should do it (except that I didn't find a good place to put this flag).
Comment on attachment 8877401 [details]
Bug 1359217 part 5 - Force all style rules to be constructed when failed to get style rule list.

https://reviewboard.mozilla.org/r/148784/#review153310

::: layout/inspector/inDOMUtils.cpp:274
(Diff revision 1)
> -    Servo_Element_GetStyleRuleList(element, &ruleList);
> +    auto getRuleList = [&ruleList, element]() {
> +      return Servo_Element_GetStyleRuleList(element, &ruleList);
> +    };
> +    if (!getRuleList()) {

Nit: Probably easier to read as:

  if (!Servo_Element_GetStyleRuleList(element, &ruleList)) {
    ...
    DebugOnly<bool> success =
      Servo_Element_GetStyleRuleList(element, &ruleList);
    MOZ_ASSERT ...
  }

::: layout/inspector/inDOMUtils.cpp:278
(Diff revision 1)
>      nsTArray<const ServoStyleRule*> ruleList;
> -    Servo_Element_GetStyleRuleList(element, &ruleList);
> +    auto getRuleList = [&ruleList, element]() {
> +      return Servo_Element_GetStyleRuleList(element, &ruleList);
> +    };
> +    if (!getRuleList()) {
> +      nsIDocument* doc = element->GetOwnerDocument();

Not important, but I usually see OwnerDoc() instead of GetOwnerDocument().
Attachment #8877401 - Flags: review?(cam) → review+
Comment on attachment 8877398 [details]
Bug 1359217 part 2 - Change Servo_Element_GetStyleRuleList to return list of Gecko's ServoStyleRule pointers.

https://reviewboard.mozilla.org/r/148778/#review153312
Attachment #8877398 - Flags: review?(cam) → review+
Comment on attachment 8877399 [details]
Bug 1359217 part 3 - Remove unused ErrorResult param from GetCssRulesInternal.

https://reviewboard.mozilla.org/r/148780/#review153314
Attachment #8877399 - Flags: review?(cam) → review+
Comment on attachment 8877400 [details]
Bug 1359217 part 4 - Make ServoStyleSheet::GetCssRulesInternal return ServoCSSRuleList.

https://reviewboard.mozilla.org/r/148782/#review153318
Attachment #8877400 - Flags: review?(cam) → review+
Comment on attachment 8877401 [details]
Bug 1359217 part 5 - Force all style rules to be constructed when failed to get style rule list.

https://reviewboard.mozilla.org/r/148784/#review153310

> Nit: Probably easier to read as:
> 
>   if (!Servo_Element_GetStyleRuleList(element, &ruleList)) {
>     ...
>     DebugOnly<bool> success =
>       Servo_Element_GetStyleRuleList(element, &ruleList);
>     MOZ_ASSERT ...
>   }

Actually I changed from this way to using lambda, so that I don't duplicate the `GetStyleRuleList` function call... This call would have one more parameter in bug 1372464. I guess I'm fine changing back.
Comment on attachment 8877397 [details]
Bug 1359217 part 1 - Put a pointer of Gecko wrapper object into Servo's StyleRule.

https://reviewboard.mozilla.org/r/148776/#review153732
Attachment #8877397 - Flags: review?(manishearth) → review+
I just wanted to say that, even though it's probably too late, that backpointer really feels like a hack, and that growing all the style rules just because of CSSOM is not great either... Though the alternatives weren't pretty I guess :(.
The alternative is probably slower but shouldn't really be unsafe. That has better modularization, and doesn't add unnecessary cost when devtools is not used...

We would see if that matters. The old patches should be kept by MozReview I suppose, so we can always try the alternative if it turns out we don't want the extra overhead from this approach.
Let's try the old approach but make it a little safer.
Attachment #8877397 - Attachment is obsolete: true
Attachment #8877398 - Attachment is obsolete: true
Attachment #8877401 - Attachment is obsolete: true
Comment on attachment 8877872 [details]
Bug 1359217 part 0 - Some fixup for later patches.

https://reviewboard.mozilla.org/r/149292/#review153786
Attachment #8877872 - Flags: review?(cam) → review+
Comment on attachment 8877874 [details]
Bug 1359217 part 2 - Include the import rule of child sheet when notifying StyleRuleAdded.

https://reviewboard.mozilla.org/r/149296/#review153788
Attachment #8877874 - Flags: review?(cam) → review+
Comment on attachment 8877875 [details]
Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping.

https://reviewboard.mozilla.org/r/149298/#review153790

::: layout/inspector/ServoStyleRuleMap.cpp:49
(Diff revision 1)
> +
> +void
> +ServoStyleRuleMap::StyleSheetAdded(StyleSheet* aStyleSheet,
> +                                   bool aDocumentSheet)
> +{
> +  if (!IsEmpty()) {

It feels a little strange to me to use IsEmpty() to determine whether we should be updating the table, rather than having a separate boolean.  But I guess in practice we will never have a document with no style sheets / rules at all...

::: layout/inspector/ServoStyleRuleMap.cpp:107
(Diff revision 1)
> +    }
> +  }

Please add a default case that asserts, so we remember to update this if we add a new rule type.  (aStyleRule->Type() isn't an enum so we can't rely on the compiler complaining about missing cases.)

::: layout/inspector/ServoStyleRuleMap.cpp:155
(Diff revision 1)
> +  nsAutoCString url;
> +  aSheet->GetSheetURI()->GetSpec(url);

What is this for?

::: layout/inspector/inDOMUtils.cpp:288
(Diff revision 1)
> -      const RawServoStyleRule* rawRule = rawRuleList.ElementAt(j);
> -      ServoStyleRule* rule = nullptr;
> +      nsAutoCString text;
> +      Servo_StyleRule_Debug(rawRule, &text);

Remove this?
Attachment #8877875 - Flags: review?(cam) → review+
Can we also have some tests that adding and removing sheets and rules of various types will give us the right results from inDOMUtils.getCSSStyleRules.
Comment on attachment 8877875 [details]
Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping.

https://reviewboard.mozilla.org/r/149298/#review153790

> What is this for?

Oops... Probably some debug code which I forgot to remove...

> Remove this?

Same here...
Comment on attachment 8877875 [details]
Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping.

https://reviewboard.mozilla.org/r/149298/#review153790

> It feels a little strange to me to use IsEmpty() to determine whether we should be updating the table, rather than having a separate boolean.  But I guess in practice we will never have a document with no style sheets / rules at all...

We should never... UA sheets should always apply.
That test should probably be added into layout/inspector/tests, which we haven't enabled for Stylo yet...
Depends on: 1373193
Comment on attachment 8877873 [details]
Bug 1359217 part 1 - Make document nsIDocumentObserver::StyleRule* methods include the rule as parameter.

https://reviewboard.mozilla.org/r/149294/#review154114

We used to pass this in until bug 1232852...

r=me
Attachment #8877873 - Flags: review?(bzbarsky) → review+
Depends on: 1373484
Since inspector mochitests haven't been enabled for stylo (maybe tracked in bug 1357715?), the newly added test wouldn't actually run on stylo. I verified locally that it passes.

(Oh, actually, I should probably add a test for checking that we get rules from UA sheet as well somehow.)
The new test doesn't actually work for stylo until either bug 1373484 or bug 1373193 gets fixed, and this bug isn't going to be completely fixed until bug 1373193 is fixed, even patches here land.

But since we don't run inspector mochitests at the moment, I guess we can safely land patches here without fixing the other two bugs :P
Comment on attachment 8878963 [details]
Bug 1359217 part 5 - Fix reversed condition for inserting import rule.

https://reviewboard.mozilla.org/r/150208/#review154886
Attachment #8878963 - Flags: review?(cam) → review+
Comment on attachment 8878964 [details]
Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping.

https://reviewboard.mozilla.org/r/150210/#review154888
Attachment #8878964 - Flags: review?(cam) → review+
Comment on attachment 8878965 [details]
Bug 1359217 part 7 - Add test for getCSSStyleRules.

https://reviewboard.mozilla.org/r/150212/#review154890

::: layout/inspector/tests/test_getCSSStyleRules.html:14
(Diff revision 2)
> +<iframe id="test"></iframe>
> +<pre id="log">
> +<script>
> +/**
> + * This test checks that getCSSStyleRules returns correct style set in
> + * various cases. To avoid affection from UA sheets, most of the tests

"effects from" or maybe "being affected by"

::: layout/inspector/tests/test_getCSSStyleRules.html:38
(Diff revision 2)
> +  if (rules.length != rulesContent.length) {
> +    return;
> +  }

Should we do

  is(rules.length, rulesContent.length, "...");

here?
Attachment #8878965 - Flags: review?(cam) → review+
Comment on attachment 8878965 [details]
Bug 1359217 part 7 - Add test for getCSSStyleRules.

https://reviewboard.mozilla.org/r/150212/#review154890

> Should we do
> 
>   is(rules.length, rulesContent.length, "...");
> 
> here?

Oh, I thought I did that check... Probably got removed accidentally in last refactoring...
Attachment #8877875 - Attachment is obsolete: true
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 4f8a65ace6de -d 302e0f95efed: rebasing 402511:4f8a65ace6de "Bug 1359217 part 0 - Some fixup for later patches. r=heycam"
rebasing 402512:f386288eb8ae "Bug 1359217 part 1 - Make document nsIDocumentObserver::StyleRule* methods include the rule as parameter. r=bz"
rebasing 402513:608a2356af83 "Bug 1359217 part 2 - Include the import rule of child sheet when notifying StyleRuleAdded. r=heycam"
merging layout/style/ServoStyleSheet.cpp
rebasing 402514:649167219f84 "Bug 1359217 part 3 - Remove unused ErrorResult param from GetCssRulesInternal. r=heycam"
merging layout/style/CSSStyleSheet.cpp
merging layout/style/CSSStyleSheet.h
merging layout/style/ServoStyleSheet.cpp
merging layout/style/ServoStyleSheet.h
merging layout/style/StyleSheet.cpp
rebasing 402515:ab0330226dd2 "Bug 1359217 part 4 - Make ServoStyleSheet::GetCssRulesInternal return ServoCSSRuleList. r=heycam"
merging layout/style/ServoStyleSheet.cpp
merging layout/style/ServoStyleSheet.h
merging layout/style/StyleSheet.cpp
rebasing 402516:355cc23da7a4 "Bug 1359217 part 5 - Fix reversed condition for inserting import rule. r=heycam"
rebasing 402517:f62b44da84f1 "Bug 1359217 part 6 - Add ServoStyleRuleMap to handle style rule mapping. r=heycam"
merging layout/inspector/inDOMUtils.cpp
merging layout/style/ServoStyleSet.cpp
merging layout/style/ServoStyleSet.h
warning: conflicts while merging layout/style/ServoStyleSet.cpp! (edit, then use 'hg resolve --mark')
warning: conflicts while merging layout/style/ServoStyleSet.h! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d36b68e3873e
part 0 - Some fixup for later patches. r=heycam
https://hg.mozilla.org/integration/autoland/rev/afa17cc4b6fd
part 1 - Make document nsIDocumentObserver::StyleRule* methods include the rule as parameter. r=bz
https://hg.mozilla.org/integration/autoland/rev/8b0c62d6e9b7
part 2 - Include the import rule of child sheet when notifying StyleRuleAdded. r=heycam
https://hg.mozilla.org/integration/autoland/rev/4ff725ffaebc
part 3 - Remove unused ErrorResult param from GetCssRulesInternal. r=heycam
https://hg.mozilla.org/integration/autoland/rev/678196c7004f
part 4 - Make ServoStyleSheet::GetCssRulesInternal return ServoCSSRuleList. r=heycam
https://hg.mozilla.org/integration/autoland/rev/935eb0620c15
part 5 - Fix reversed condition for inserting import rule. r=heycam
https://hg.mozilla.org/integration/autoland/rev/a06509631afb
part 6 - Add ServoStyleRuleMap to handle style rule mapping. r=heycam
https://hg.mozilla.org/integration/autoland/rev/9b82992bcacb
part 7 - Add test for getCSSStyleRules. r=heycam
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: