Open Bug 1638591 Opened 4 years ago Updated 4 years ago

Thunderbird should move elements away from XUL Layout.

Categories

(Thunderbird :: Mail Window Front End, task)

Tracking

(Not tracked)

People

(Reporter: ishikawa, Unassigned)

References

Details

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1621274 +++
In sovling the issue in bug 162174, it turns out that the issue is related XUL Layout.
The original stack dump is now being suppressed if the issue is related to XUL Layout.

In the dicussion, following question came up.:

Also, is there any chance that Thunderbird could move those elements away from XUL layout? You can see which elements they are using
frame->GetContent()->List() or frame->DumpFrameTreeLimited().

I think the suggestion makes sense now that XUL is being removed (gradually maybe)
AND I suspect a very strange slow down due to the edge case handling in the layout code.
This is especially true during mochitest Calendar testing.
[I mean the change in the calendar screen image sometimes is not as quick as the rest of the test. Now that stackdump is gone, the screen update is quicker definitely, but there are moments of silence during the calendar test. ]

This definitely should be a goal, maybe a medium-term(?).

The comment below is the original one.


I set platform as x86_64 and linux (this is my local PC), but I suspect this bug appears on all the platforms

During mochitest run of C-C TB FULL DEBUG version locally, I see many assertions of the following form. 498 times to be exact.:

498: Main Thread] ###!!! ASSERTION: Table inline-size is less than the sum of its columns' min inline-sizes: '!(aISizeType == BTLS_FINAL_ISIZE && aISize < guess_min)', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/layout/tables/BasicTableLayoutStrategy.cpp, line 758

Note the number 498 at the beginning of the line. That is added by my script to summarize the errors/warnings.

Now, interestingly another already reported(?) bug appears exactly same number of times.

498: Main Thread] ###!!! ASSERTION: didn't subtract all that we added: '(space == 0 || space == nscoord_MAX) && ((l2t == FLEX_PCT_LARGE) ? (-0.001f < basis.f && basis.f < 0.001f) : (basis.c == 0 || basis.c == nscoord_MAX))', file /NEW-SSD/NREF-COMM-CENTRAL/mozilla/layout/tables/BasicTableLayoutStrategy.cpp, line 983

This is mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=513106#c4

So I believe these two assertions are related.

498 is a bit too many.

We should investigate this.

The assertions can be found in other people's tyrserver runs as well.

For example,

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&fromchange=6c81ea172348b677edd90753b6f5b5be3afbce6a&selectedJob=292189682
https://firefoxci.taskcluster-artifacts.net/FZzfXi8FScuJpukxLvuuBA/0/public/logs/live_backing.log

You will notice that

"Table inline-size ..." assertion precedes "didn't subtract all that ..." assertion. They appear in pairs.

Severity: S3 → --
Priority: P3 → --

It would happen at some point, but gradually. Did you find out which element this case is about?

I obtain the list of elements of XUL Box when the assertion was hit during mochitest (with FULL DEBUG version of TB).
The list was obtained by the following wher trfame was the innermost surrounding XUL Box when the assertion was hit.

          tframe->DumpFrameTreeLimited();

I split the attachment in two.

The first one here shows the excerpt of the log. It shows the tests and the dump.

This is the list of elements of XUL Boxes by stripping the line prefix from the log.
(The order of listing is random.)

They are all different (I thought there may be duplicates, etc.) but when the assertion was hit, they are all different trees.

These are obtained by the following awk script after running mochitest with the my original verbose patch in Bug 1621274, which was further modified to add the list dump statement noted in the previous comment

#
# run this
# gawk -f this_script logfile > logfile-1.tmp
#
#
BEGIN {
  junk = 1;
  accummulate = 0;
  lines = "";
  hasseen = 0;
}

/TEST_START/ { print; next; }

/TEST_END/ { print; next; }

/Entering test/ {print; next; }
/Leaving test/ {print; next; }

# start accummulating
/{debug}:ASSERTION: ! \(aISizeType == BTLS_FINAL_ISIZE/ {
  accumulate = 1;
  print;
  lines = "";
  hasseen ++;
  next;
}

/{debug}:ASSERTION: \(space == 0/ {
  print "**** ENDING list"

  accumulate = 0;
  print;
  seen[lines] = 1;
  next;
}

accumulate {
  print;
  j = match($0, "\\)");
  t = substr($0, j+1)
  lines = lines "\n" t;
}


END {
  print "Ending .... ";
  print "hasseen = ", hasseen;
  uniqseen = 0;
  for( t in seen ) {
    uniqseen ++;
    print "----\n";
    print t;
    print "\n";
  }
  print "uniqseen =",   uniqseen;
}

Anyway, as far as I can tell, the lists are all related to "Box(calendar-minimonth)".
So that is where the effort to eliminate XUL Box dependency should be focused.

BTW, adding the dump slightly changed the timing behavior of TB and I think I see some timeouts due to the timing race or something. Interesting.

Type: defect → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: