Bug 39147

Summary: Frame has many trivial member functions that should be inlined
Product: WebKit Reporter: Darin Adler <darin>
Component: Page LoadingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, atwilson, eric, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch beidson: review+, beidson: commit-queue-

Darin Adler
Reported 2010-05-14 19:17:33 PDT
Frame has many trivial member functions that should be inlined
Attachments
Patch (18.93 KB, patch)
2010-05-14 19:20 PDT, Darin Adler 		beidson: review+
beidson: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2010-05-14 19:20:34 PDT
Created attachment 56132 [details] Patch
Mark Rowe (bdash)
Comment 2 2010-05-15 03:15:01 PDT
Hah, member variables named m_kjsStatusBarText and m_kjsDefaultStatusBarText.
Brady Eidson
Comment 3 2010-05-17 16:17:06 PDT
Comment on attachment 56132 [details] Patch > -void Frame::removeEditingStyleFromElement(Element*) const > -{ > -} > - May as well remove this one from the header, too? r+, after the EWS bots get a chance to churn.
Darin Adler
Comment 4 2010-05-17 16:49:04 PDT
Committed r59630: <http://trac.webkit.org/changeset/59630>
WebKit Review Bot
Comment 5 2010-05-17 16:58:33 PDT
http://trac.webkit.org/changeset/59630 might have broken Qt Linux Release minimal and Chromium Linux Release
Eric Seidel (no email)
Comment 6 2010-05-17 17:08:08 PDT
Looks like the compile failures are real.
Darin Adler
Comment 7 2010-05-17 17:12:16 PDT
But not seen by the EWS. Ugh.
Eric Seidel (no email)
Comment 8 2010-05-17 17:13:28 PDT
:( Working on making the EWS faster this week.
Darin Adler
Comment 9 2010-05-17 17:15:57 PDT
EWS built on Qt and was green. It was not too slow, it was wrong!
Darin Adler
Comment 10 2010-05-17 17:16:15 PDT
http://trac.webkit.org/changeset/59632 is an attempt to fix the Qt build.
Eric Seidel (no email)
Comment 11 2010-05-17 17:18:16 PDT
Hmmm. I wonder if there are further dependency issues which need work on Qt.
Andrew Wilson
Comment 12 2010-05-17 17:53:00 PDT
Breaks Chromium also. I'm about to land a patch which #includes CSSMutableStyleDeclaration.h in Frame.h - non-ideal, but the alternative is to un-inline more functions which I'm assuming is contrary to what Darin intended. Nobody is responding on IRC on this, so I'm going to make the minimum fix (add #include) to get things to compile for now.
Darin Adler
Comment 13 2010-05-17 17:59:04 PDT
(In reply to comment #12) > Breaks Chromium also. > > I'm about to land a patch which #includes CSSMutableStyleDeclaration.h in Frame.h - non-ideal, but the alternative is to un-inline more functions which I'm assuming is contrary to what Darin intended. Nobody is responding on IRC on this, so I'm going to make the minimum fix (add #include) to get things to compile for now. I think it's OK to un-inline some more functions.
Note You need to log in before you can comment on or make changes to this bug.