Panorama triggering WARNING: Overflowed nscoord_MAX in conversion to nscoord

RESOLVED FIXED

Status

Firefox Graveyard
Panorama
P3
normal
RESOLVED FIXED
7 years ago
2 years ago

People

(Reporter: zpao, Assigned: imphil)

Tracking

Trunk
x86
Mac OS X

Details

Attachments

(3 attachments, 1 obsolete attachment)

I've been seeing this for a long time now but it's harmless, so I held off on reporting, but here I am now.

I caught it in GDB while resizing a group with a single tab and here's the JS stack:

0 iQClass_bounds() ["chrome://browser/content/tabview.js":393]
    rect = undefined
    this = [object Object]
1 iQClass_width() ["chrome://browser/content/tabview.js":367]
    bounds = undefined
    this = [object Object]
2 TabItems__update(tab = [object XULElement @ 0x12128c4b0 (native @ 0x120d637e0)]) ["chrome://browser/content/tabview.js":5531]
    this = [object Object]
3 TabItems_update(tab = [object XULElement @ 0x12128c4b0 (native @ 0x120d637e0)]) ["chrome://browser/content/tabview.js":5471]
    this = [object Object]
4 TabItem_setBounds(options = [object Object], immediately = true, inRect = [object Object]) ["chrome://browser/content/tabview.js":5048]
    alphaRange = undefined
    pad = undefined
    css = [object Object]
    rect = [object Object]
    validSize = [object Object]
    this = [object Object]
5 GroupItem_arrange_children_each(i = 0, child = [object Object], [object Object]) ["chrome://browser/content/tabview.js":3405]
    this = [object ChromeWindow @ 0x125d29be0 (native @ 0x125d28838)]
6 GroupItem__gridArrange(options = [object Object], box = [object Object], childrenToArrange = [object Object]) ["chrome://browser/content/tabview.js":3399]
    self = [object Object]
    index = 0
    columns = 1
    rects = [object Object]
    dropIndex = false
    result = [object Object]
    arrangeOptions = [object Object]
    this = [object Object]
7 GroupItem_arrange(options = [object Object]) ["chrome://browser/content/tabview.js":3266]
    box = [object Object]
    shouldStack = false
    childrenToArrange = [object Object]
    this = [object Object]
8 GroupItem_setBounds(options = [object Object], immediately = true, inRect = [object Object]) ["chrome://browser/content/tabview.js":2573]
    offset = [object Object]
    contentCSS = [object Object]
    titlebarCSS = [object Object]
    css = [object Object]
    titleHeight = 18
    rect = [object Object]
    validSize = [object Object]
    this = [object Object]
9 anonymous(e = [object MouseEvent @ 0x1250b95d0 (native @ 0x1250b3ae0)]) ["chrome://browser/content/tabview.js":1800]
    oldWidth = undefined
    minWidth = undefined
    box = [object Object]
    mouse = [object Object]
    this = [object ChromeWindow @ 0x105eaad40 (native @ 0x105ea93f8)]
10 anonymous(event = [object MouseEvent @ 0x1250b95d0 (native @ 0x1250b3ae0)]) ["chrome://browser/content/tabview.js":692]
    this = [object ChromeWindow @ 0x105eaad40 (native @ 0x105ea93f8)]

The undefined being used all around is suspicious to me, but could just be a red herring.

Joe: Not sure if there's anything you'd be able to say about this, but since this is hitting a warning in gfx, I figured I'd CC you.
Created attachment 509887 [details]
backtrace

backtrace from gdb in case it ends up being relevant.
Blocks: 585689
Priority: -- → P3
(Assignee)

Comment 2

7 years ago
I get the same message (and there seem to be some more similar bug reports out there), and it got quite annoying (got this every couple seconds with Panorama and playing a Flash movie).

So that's what I think happens:
- in nsBlockFrame.cpp:2833 we have
  nsRuleNode::ComputeCoordPercentCalc(aCoord, nscoord_MAX)

aCoord is:
(gdb) p aCoord
$13 = (const nsStyleCoord &) @0x7f66fb459c78: {mUnit = eStyleUnit_Percent, mValue = {mInt = 1065353216, mFloat = 1, mPointer = 0x7fffffff3f800000}}
(gdb) p aCoord.GetPercentValue()
$14 = 1

- in nsNodeRule:497 we have
  return NSToCoordFloorClamped(aPercentageBasis * aCoord.GetPercentValue());

So that's effectively calling NSToCoordFloorClamped(nscoord_MAX) and there the warning comes from.

I attached a patch that should change no functionality and get rid of the warning, but I'm not sure why the check was made with >= instead of > in the first place, so I'm not completely sure that really nothing regresses with this.
(Assignee)

Comment 3

7 years ago
Created attachment 513163 [details] [diff] [review]
patch v1
(Assignee)

Updated

7 years ago
Attachment #513163 - Flags: review?(dbaron)
I suspect it might be better to make the NS_WARNING into NS_WARN_IF_FALSE instead, so you make the change for whether we warn but not whether we return.

That said, I think roc should probably review, since he wrote this (https://hg.mozilla.org/mozilla-central/rev/54946665b7c0).

Also, you should make the same change to NSToCoordFloorClamped, NSToCoordCeilClamped, and NSToCoordRoundWithClamp.
(Assignee)

Comment 5

7 years ago
Created attachment 513260 [details] [diff] [review]
patch v2

Now using NS_WARN_IF_FALSE and I also changed all relevant functions. The new patch changes no functionality at all, only the warning messages.
Attachment #513163 - Attachment is obsolete: true
Attachment #513260 - Flags: review?
Attachment #513163 - Flags: review?(dbaron)
(Assignee)

Updated

7 years ago
Attachment #513260 - Flags: review? → review?(roc)
(Assignee)

Comment 6

7 years ago
Can/should this go to mozilla-central before Firefox 4? Or should it wait until after the final?
Comment on attachment 513260 [details] [diff] [review]
patch v2

It's DEBUG-only, so a2.0=dbaron.
Attachment #513260 - Flags: approval2.0+
(Assignee)

Comment 8

7 years ago
Created attachment 513866 [details] [diff] [review]
patch for checkin
Assignee: nobody → mail
Status: NEW → ASSIGNED
(Assignee)

Updated

7 years ago
Keywords: checkin-needed

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/cfa2a5ef17c3
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.