1821603 - Thunderbird debug build test results not parsed due to huge amounts of ###!!! ASSERTION: writing-mode mismatch: 'aWritingMode.IgnoreSideways() == GetWritingMode().IgnoreSideways()

Status: RESOLVED FIXED

(Thunderbird :: General, defect, P1)

Description

[Magnus Melin [:mkmelin]]

All the debug mochitest results can't be viewed since the logs are too large. They are too large due to

###!!! ASSERTION: writing-mode mismatch: 'aWritingMode.IgnoreSideways() == GetWritingMode().IgnoreSideways()

E.g. https://firefoxci.taskcluster-artifacts.net/KR4DqUYESpeUBaSGsD6P6w/0/public/logs/live_backing.log

The stack indicates it's flexbox related, and showing up since switch to modern flexbox in bug 1820744.

Comment 1

[ISHIKAWA, Chiaki]

Attachment: Excerpt from the local log. The first assertion (before a test begins).

This hurts local test, too.
I have to stop mochitest only a minute or two by hitting CONTROL-C under linux after starting because the log alrady becomes 27GB in that time frame.

Does printing |mWritingMode.bits| of |param| and |GetwritingMOde()| help?

I am asking because the assertion is of the following form,

    NS_ASSERTION(param.IgnoreSideways() == GetWritingMode().IgnoreSideways(), \
                 "writing-mode mismatch"); 

and
IgnoreSidways() is defined as

#ifdef DEBUG
  // Used by CHECK_WRITING_MODE to compare modes without regard for the
  // StyleWritingMode::VERTICAL_SIDEWAYS or StyleWritingMode::TEXT_SIDEWAYS
  // flags.
  WritingMode IgnoreSideways() const {
    return WritingMode(
        mWritingMode.bits &
        ~(StyleWritingMode::VERTICAL_SIDEWAYS | StyleWritingMode::TEXT_SIDEWAYS)
             .bits);
  }
#endif

Well, I printed it anyway.
It seems that in the first part of local mochitest log,
we have 0x12 vs 0x00 or 0x00 vs 0x12 of mismatches.
The code to print the value. I rewrote the macro:

#define CHECK_WRITING_MODE(param)                                       \
  do {                                                                  \
    if (param.IgnoreSideways() != GetWritingMode().IgnoreSideways())   {  \
      NS_WARNING(nsPrintfCString(                                       \
                   "param.mWritingMode             (0x%08x)\n"          \
                   "GetWritingMode().mWritingMode  (0x%08x)\n",         \
                   (unsigned int) param.mWritingMode.bits,              \
                   (unsigned int) GetWritingMode().mWritingMode.bits).get()); \
    }                                                                   \
    NS_ASSERTION(param.IgnoreSideways() == GetWritingMode().IgnoreSideways(), \
                 "writing-mode mismatch");                              \
  } while (false) 

The first assertion happens BEFORE the test begins. (See the excerpt of the log attached.).

From the modified CHECK_WRITING_MODE(), I observed (from about 130K pairs recorded in the first few minutes),
0x00 vs 0x12 or 0x12 vs 0x00 of param.MwritingMode.bits and GetWritingMode().mWritingMode.bits occur.

Hope this helps.

Comment 2

[ISHIKAWA, Chiaki]

From the look of the enum values, I am tempted to think some RTL writing setting crept in?
0x12 = 0x10 | 0x02.

From WritingModes.h:

  /**
   * Absolute inline flow direction
   */
  enum InlineDir {
    eInlineLTR = 0x00,  // text flows horizontally left to right
    eInlineRTL = 0x02,  // text flows horizontally right to left
    eInlineTTB = 0x01,  // text flows vertically top to bottom
    eInlineBTT = 0x03,  // text flows vertically bottom to top
  };

  /**
   * Absolute block flow direction
   */
  enum BlockDir {
    eBlockTB = 0x00,  // horizontal lines stack top to bottom
    eBlockRL = 0x01,  // vertical lines stack right to left
    eBlockLR = 0x05,  // vertical lines stack left to right
  };

  /**
   * Line-relative (bidi-relative) inline flow direction
   */
  enum BidiDir {
    eBidiLTR = 0x00,  // inline flow matches bidi LTR text
    eBidiRTL = 0x10,  // inline flow matches bidi RTL text
  };

Comment 3

[ISHIKAWA, Chiaki]

Simply running locally build TB would hit this NS_ASSERT().
I can see there are class instances that disagree with the rendering direction(?) somehow.
I am not sure how this happens, but maybe lengthy gdb session by someone in the know should make it clear.

Comment 4

[Intermittent Failures Robot]

39 failures in 3966 pushes (0.01 failures/push) were associated with this bug in the last 7 days.

This is the #34 most frequent failure this week.

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

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

Repository breakdown:

• comm-central: 39

Platform and build breakdown:

• windows11-32-2009-qr: 7
  • debug: 7
• windows10-32-2004-qr: 7
  • debug: 7
• linux1804-64-qr: 7
  • debug: 7
• windows11-64-2009-qr: 6
  • debug: 6
• windows10-64-2004-qr: 5
  • debug: 5
• macosx1015-64-qr: 7
  • debug: 7

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1821603&startday=2023-03-06&endday=2023-03-12&tree=all

Comment 5

[Kai Engert (:KaiE:)]

I think this needs to be a very high priority, we need treeherder bug logs for development.

Comment 6

[Kai Engert (:KaiE:)]

This is an assertion in Mozilla/Firefox core layout code, file layout/generic/WritingModes.h

Does anyone understand why this assertion affects Thunderbird, but apparently doesn't affect Firefox treeherder builds?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I can poke now I'm back from unexpected travel and have fast builds again.

Comment 8

[Kai Engert (:KaiE:)]

Attachment: Bug 1821603 - Disable chat scrollbar rule to prevent rtl/ltr assertions. r=freaktechnik

Comment 9

[Kai Engert (:KaiE:)]

Chiaki thanks for your analysis.

With the hack I've attached the assertions are gone for me.

The hack disables a forced rtl/ltr ordering.

I don't know what the correct fix would be. But if the code this hack disables is simply responsible for some placement on screen, maybe it could be checked in until we get a better fix?

Comment 10

[Kai Engert (:KaiE:)]

We'll land this after today's merge, and need to uplift to beta.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1821603 - Fix writing-mode mismatch in visibility: collapse handling. r=dholbert,TYLin

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Note that we backed out bug 1820534 from beta so you might want to instead back out your flexbox follow-ups

Comment 13

[Kai Engert (:KaiE:)]

Thanks Emilio, that's helpful information!

Comment 14

[ISHIKAWA, Chiaki]

(In reply to Kai Engert (:KaiE:) from comment #9)

Chiaki thanks for your analysis.

With the hack I've attached the assertions are gone for me.

The hack disables a forced rtl/ltr ordering.

I don't know what the correct fix would be. But if the code this hack disables is simply responsible for some placement on screen, maybe it could be checked in until we get a better fix?

You are welcome and it is great to hear it is gone for now.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)

Note that we backed out bug 1820534 from beta so you might want to instead back out your flexbox follow-ups

I see the root cause is temporarily backed out.
I think we are more prepared for strange rtl/ltr rendering issue in the future after this experience.

Comment 15

[ISHIKAWA, Chiaki]

(In reply to Kai Engert (:KaiE:) from comment #9)

Chiaki thanks for your analysis.

With the hack I've attached the assertions are gone for me.

The hack disables a forced rtl/ltr ordering.

I don't know what the correct fix would be. But if the code this hack disables is simply responsible for some placement on screen, maybe it could be checked in until we get a better fix?

Oops, I probably misunderstood.

We still need Kai's patch against |chat.css| in comment 9 to shut up the run time stack trace (or abort depending on the setting of XPCOM_DEBUG_BREAK), correct?

I downloaded the very latest M-C/C-C and rebuild TB, but
without the patch in comment 9, I still got 850 assertion hits simply by starting and stopping locally built debug version of TB.
(Or maybe the removal of bug Bug 1820534 may not have propagated to the visible M-C tree yet?)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d2ffc302c56
Fix writing-mode mismatch in visibility: collapse handling. r=TYLin

Comment 17

[ISHIKAWA, Chiaki]

(In reply to Pulsebot from comment #16)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d2ffc302c56
Fix writing-mode mismatch in visibility: collapse handling. r=TYLin

I now verified that DEBUG version of C-C TB does not produce the NS_ASSERT() dump any more after manually applying the patch
to layout/generic/nsFlexContainerFrame.cpp in comment 16.
(I downloaded the M-C tree a few hours ago, but it was not in there yet.)
This was true with or without Kai's patch to chat.css in comment 8.
So we may probably not have to patch css.patch at all now.
But I will leave the decision to someone in the know.

Thank you for fixing the issue so quickly (!)

Comment 18

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/2d2ffc302c56

Comment 19

[Martin Giger [:freaktechnik]]

Debug builds looking normal again.