Last Comment Bug 379709 - During the initial reflow of SVG we should assume scrollbars won't be needed
: During the initial reflow of SVG we should assume scrollbars won't be needed
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Jonathan Watt [:jwatt]
:
: Jet Villegas (:jet)
Mentors:
Depends on: 736753
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-04 06:20 PDT by Jonathan Watt [:jwatt]
Modified: 2012-03-19 10:22 PDT (History)
7 users (show)
tor: blocking1.9-
tor: wanted1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (4.30 KB, patch)
2012-03-16 15:02 PDT, Jonathan Watt [:jwatt]
no flags Details | Diff | Splinter Review
patch (4.08 KB, patch)
2012-03-17 07:54 PDT, Jonathan Watt [:jwatt]
bzbarsky: review+
Details | Diff | Splinter Review

Description User image Jonathan Watt [:jwatt] 2007-05-04 06:20:17 PDT
In bug 378975 the initial reflow of documents was changed to assume that a scrollbar would be needed, and if not, documents are reflowed a second time. This change is intended to improve performance for long documents in HTML that require scrollbars (the common case in HTML) but in SVG documents don't get longer the more complex they are. Scrollbars are the rare case. It would be good to make this optimization conditional so SVG isn't penalized (maybe even check the SVG root element in advance to _know_ if we need scrollbars).
Comment 1 User image Boris Zbarsky [:bz] (still a bit busy) 2007-05-04 11:27:25 PDT
Sure.  Feel free to tweak nsHTMLScrollFrame::InInitialReflow().  I guess you're want to see whether the scrolled frame is an SVG outer frame when mInner.mIsRoot?
Comment 2 User image Jonathan Watt [:jwatt] 2012-03-16 15:02:06 PDT
Created attachment 606752 [details] [diff] [review]
patch

Was reminded again while debugging that this is still happening.
Comment 3 User image Boris Zbarsky [:bz] (still a bit busy) 2012-03-16 20:02:17 PDT
Comment on attachment 606752 [details] [diff] [review]
patch

Why is the nsLayoutUtils method needed?  Why not just put that code inline in the scrollframe code?
Comment 4 User image Jonathan Watt [:jwatt] 2012-03-17 03:09:14 PDT
Because then I need to export nsSVGOuterSVGFrame.h, and all headers that in depends on, and all the headers they depend on, etc.
Comment 5 User image Robert Longson 2012-03-17 04:37:36 PDT
We have a state bit for the outer SVG frame. That's better than a GetType call.
Comment 6 User image Jonathan Watt [:jwatt] 2012-03-17 04:44:12 PDT
To use that I need to check that the frame is an SVG frame first, which just makes the code more verbose for no real win.
Comment 7 User image Boris Zbarsky [:bz] (still a bit busy) 2012-03-17 07:23:49 PDT
> Because then I need to export nsSVGOuterSVGFrame.h

Export in what sense?  You just need to add the relevant directory to LOCAL_INCLUDES in layout/generic, no?  That's why it works in layout/base...
Comment 8 User image Jonathan Watt [:jwatt] 2012-03-17 07:54:44 PDT
Created attachment 606866 [details] [diff] [review]
patch

Yeah, good point; I should get more sleep.
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2012-03-18 22:35:58 PDT
Comment on attachment 606866 [details] [diff] [review]
patch

r=me
Comment 10 User image Matt Brubeck (:mbrubeck) 2012-03-19 10:22:52 PDT
https://hg.mozilla.org/mozilla-central/rev/b81ec953e4d6

Note You need to log in before you can comment on or make changes to this bug.