1435803 - Intermittent GECKO(1031) | Assertion failure: !PresContext()->HasPendingMediaQueryUpdates() (Someone forgot to update media queries?), at /builds/worker/workspace/build/src/layout/base/GeckoRestyleManager.cpp:566

Status: RESOLVED FIXED

(Core :: Layout, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: archaeopteryx [at] coole-files.de

https://treeherder.mozilla.org/logviewer.html#?job_id=160422406&repo=mozilla-central

https://queue.taskcluster.net/v1/task/QeKpQ2dzR5mnIXpkzS8AhA/runs/0/artifacts/public/logs/live_backing.log

[task 2018-02-05T17:41:06.325Z] 17:41:06     INFO - GECKO(1031) | Assertion failure: !PresContext()->HasPendingMediaQueryUpdates() (Someone forgot to update media queries?), at /builds/worker/workspace/build/src/layout/base/GeckoRestyleManager.cpp:566
[task 2018-02-05T17:41:06.326Z] 17:41:06     INFO - GECKO(1031) | #01: mozilla::GeckoRestyleManager::ProcessPendingRestyles [layout/base/GeckoRestyleManager.cpp:573]
[task 2018-02-05T17:41:06.328Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.328Z] 17:41:06     INFO - GECKO(1031) | #02: mozilla::GeckoRestyleManager::RebuildAllStyleData [layout/base/GeckoRestyleManager.cpp:479]
[task 2018-02-05T17:41:06.328Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.328Z] 17:41:06     INFO - GECKO(1031) | #03: nsPresContext::RebuildAllStyleData [layout/base/RestyleManagerInlines.h:33]
[task 2018-02-05T17:41:06.328Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.330Z] 17:41:06     INFO - GECKO(1031) | #04: nsPresContext::DoChangeCharSet [layout/base/nsPresContext.cpp:1134]
[task 2018-02-05T17:41:06.330Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.331Z] 17:41:06     INFO - GECKO(1031) | #05: CharSetChangingRunnable::Run [layout/base/nsPresContext.cpp:146]
[task 2018-02-05T17:41:06.331Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.332Z] 17:41:06     INFO - GECKO(1031) | #06: mozilla::SchedulerGroup::Runnable::Run [xpcom/threads/SchedulerGroup.cpp:395]
[task 2018-02-05T17:41:06.332Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.333Z] 17:41:06     INFO - GECKO(1031) | #07: nsThread::ProcessNextEvent [mfbt/Maybe.h:100]
[task 2018-02-05T17:41:06.334Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.341Z] 17:41:06     INFO - GECKO(1031) | #08: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:517]
[task 2018-02-05T17:41:06.341Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.343Z] 17:41:06     INFO - GECKO(1031) | #09: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:98]
[task 2018-02-05T17:41:06.344Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.346Z] 17:41:06     INFO - GECKO(1031) | #10: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
[task 2018-02-05T17:41:06.347Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.348Z] 17:41:06     INFO - GECKO(1031) | #11: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:298]
[task 2018-02-05T17:41:06.349Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.351Z] 17:41:06     INFO - GECKO(1031) | #12: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:159]
[task 2018-02-05T17:41:06.352Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.353Z] 17:41:06     INFO - GECKO(1031) | #13: XRE_RunAppShell [toolkit/xre/nsEmbedFunctions.cpp:873]
[task 2018-02-05T17:41:06.354Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.356Z] 17:41:06     INFO - GECKO(1031) | #14: mozilla::ipc::MessagePumpForChildProcess::Run [ipc/glue/MessagePump.cpp:269]
[task 2018-02-05T17:41:06.357Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.358Z] 17:41:06     INFO - GECKO(1031) | #15: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
[task 2018-02-05T17:41:06.360Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.361Z] 17:41:06     INFO - GECKO(1031) | #16: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:298]
[task 2018-02-05T17:41:06.362Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.363Z] 17:41:06     INFO - GECKO(1031) | #17: XRE_InitChildProcess [toolkit/xre/nsEmbedFunctions.cpp:703]
[task 2018-02-05T17:41:06.363Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.364Z] 17:41:06     INFO - GECKO(1031) | #18: content_process_main [ipc/contentproc/plugin-container.cpp:63]
[task 2018-02-05T17:41:06.365Z] 17:41:06     INFO - 
[task 2018-02-05T17:41:06.366Z] 17:41:06     INFO - GECKO(1031) | #19: main [browser/app/nsBrowserApp.cpp:280]
[task 2018-02-05T17:41:06.367Z] 17:41:06     INFO -

Comment 1

[Intermittent Failures Robot]

16 failures in 154 pushes (0.104 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* autoland: 10
* mozilla-central: 4
* mozilla-inbound: 2

Platform breakdown:
* linux64-stylo-disabled: 8
* linux32-stylo-disabled: 7
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1435803&startday=2018-02-07&endday=2018-02-07&tree=all

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to OrangeFactor Robot from comment #1)
> Platform breakdown:
> * linux64-stylo-disabled: 8
> * linux32-stylo-disabled: 7

These are all debug-mochitest-browser-chrome-e10s-14

> * linux32: 1

This is debug-mochitest-media-e10s-2.  (I wonder if that has stylo disabled?)

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm going to assume this is a regression from bug 1435939.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Except for the single isolated occurrence 2 days earlier (!!), this failure becoming high frequency does appear to match the landing of bug 1435939, based on:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1435803&entireHistory=true&tree=all

Comment 5

[Emilio Cobos Álvarez (:emilio)]

That bug landed 11 hours ago, but this bug was filed 2 days ago, so sounds unlikely... But chances are that bug made it more frequent, yeah.

Looking at the stack it makes sense, Gecko reconstructs all style data synchronously, instead of async like Stylo does, so there's nothing preventing that assertion to get hit from one of the direct callers of RebuildAllStyleData.

There wasn't anything preventing that assertion to fail either before bug 1430844, which added it.

I'll take a look to see if there's an easy fix, otherwise disabling that assertion on the old style system is probably an ok solution for now.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1435803: Loosen an assertion that may not hold on the old style system.

Review commit: https://reviewboard.mozilla.org/r/218622/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218622/

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Feel free to land it if it's urgent, since it's late for me and I'm heading out to bed :)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8949254 [details]
Bug 1435803: Loosen an assertion that may not hold on the old style system.

https://reviewboard.mozilla.org/r/218622/#review224426

r=dbaron, although I didn't really page all of this in.  (In particular, it's not entirely clear to me why mDoRebuildAllStyleData is a useful condition other than "it happens to be when the assertion fires".)

I'd also note that I had an unlanded patch in my tree that might have fixed some related issues: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/0c03eded4deb/flush-media-feature-values .

::: layout/base/GeckoRestyleManager.cpp:570
(Diff revision 1)
> +  // point is probably not worth the effort. Also, the restyles the media query
> +  // changes require will be processed afterwards anyway, so it's probably not a
> +  // correctness issue, just not as efficient as it could be.

I'm not entirely convinced that this is true, given things like bug 1089417 comment 21, etc., but it's been too long for me to remember the details, or whether that setup is still true.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #8)
> Comment on attachment 8949254 [details]
> Bug 1435803: Loosen an assertion that may not hold on the old style system.
> 
> https://reviewboard.mozilla.org/r/218622/#review224426
> 
> r=dbaron, although I didn't really page all of this in.  (In particular,
> it's not entirely clear to me why mDoRebuildAllStyleData is a useful
> condition other than "it happens to be when the assertion fires".)
> 
> I'd also note that I had an unlanded patch in my tree that might have fixed
> some related issues:
> https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/
> 0c03eded4deb/flush-media-feature-values .

Maybe... The only thing that cares about that on stylo, afaict, is ProcessPendingRestyles, of which there are two callers now (three in Gecko). One is sound (FlushPendingNotifications), the other one (FlushThrottledStyles) is bug 1436625.

Other than that we should update media queries correctly.

> ::: layout/base/GeckoRestyleManager.cpp:570
> (Diff revision 1)
> > +  // point is probably not worth the effort. Also, the restyles the media query
> > +  // changes require will be processed afterwards anyway, so it's probably not a
> > +  // correctness issue, just not as efficient as it could be.
> 
> I'm not entirely convinced that this is true, given things like bug 1089417
> comment 21, etc., but it's been too long for me to remember the details, or
> whether that setup is still true.

At least in the Stylo case it would be, but maybe on the old style system it's not... Still given this is not an exactly new problem and this is nearly permafailing according to Aryx I'm going to land this for now, I think.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8949254 [details]
Bug 1435803: Loosen an assertion that may not hold on the old style system.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218622/diff/1-2/

Comment 11

[Emilio Cobos Álvarez (:emilio)]

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #8)
> Comment on attachment 8949254 [details]
> Bug 1435803: Loosen an assertion that may not hold on the old style system.
> 
> https://reviewboard.mozilla.org/r/218622/#review224426
> 
> r=dbaron, although I didn't really page all of this in.  (In particular,
> it's not entirely clear to me why mDoRebuildAllStyleData is a useful
> condition other than "it happens to be when the assertion fires".)
> 
> I'd also note that I had an unlanded patch in my tree that might have fixed
> some related issues:
> https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/
> 0c03eded4deb/flush-media-feature-values .

fwiw, I think the ContentStateChanged / AttributeChanged callers on this patch would need some thought, since FlushPendingMediaQueryUpdates can call script. Also, the FlushUserFontSet one should not be needed now that we call that right after flushing pending media query updates.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8949254 [details]
Bug 1435803: Loosen an assertion that may not hold on the old style system.

https://reviewboard.mozilla.org/r/218622/#review224426

> I'm not entirely convinced that this is true, given things like bug 1089417 comment 21, etc., but it's been too long for me to remember the details, or whether that setup is still true.

I've mentioned that bug comment in the patch.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d186a51615a6
Loosen an assertion that may not hold on the old style system. r=dbaron

Comment 14

[Mozilla Autoland]

We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/d186a51615a6

Comment 16

[Intermittent Failures Robot]

50 failures in 125 pushes (0.4 failures/push) were associated with this bug yesterday.    

Repository breakdown:
* mozilla-inbound: 23
* autoland: 16
* mozilla-central: 10
* try: 1

Platform breakdown:
* linux64-stylo-disabled: 28
* linux32-stylo-disabled: 20
* windows7-32: 1
* linux32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1435803&startday=2018-02-08&endday=2018-02-08&tree=all

Comment 17

[Intermittent Failures Robot]

76 failures in 702 pushes (0.108 failures/push) were associated with this bug in the last 7 days. 

This is the #12 most frequent failure this week. 

** This failure happened more than 75 times this week! Resolving this bug is a very high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 1 week, the affected test(s) may be disabled. **   

Repository breakdown:
* autoland: 28
* mozilla-inbound: 27
* mozilla-central: 15
* try: 6

Platform breakdown:
* linux64-stylo-disabled: 39
* linux32-stylo-disabled: 33
* linux32: 3
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1435803&startday=2018-02-05&endday=2018-02-11&tree=all

Comment 18

[Intermittent Failures Robot]

2 failures in 660 pushes (0.003 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* autoland: 2

Platform breakdown:
* linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1435803&startday=2018-05-07&endday=2018-05-13&tree=trunk