Bug 39147 - Frame has many trivial member functions that should be inlined
Summary: Frame has many trivial member functions that should be inlined
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-14 19:17 PDT by Darin Adler
Modified: 2010-05-17 17:59 PDT (History)
5 users (show)

See Also:

Attachments
Patch (18.93 KB, patch)
2010-05-14 19:20 PDT, Darin Adler 		beidson: review+
beidson: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-05-14 19:17:33 PDT 
Frame has many trivial member functions that should be inlined
Comment 1 Darin Adler 2010-05-14 19:20:34 PDT 
Created attachment 56132 [details]
Patch
Comment 2 Mark Rowe (bdash) 2010-05-15 03:15:01 PDT 
Hah, member variables named m_kjsStatusBarText and m_kjsDefaultStatusBarText.
Comment 3 Brady Eidson 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.
Comment 4 Darin Adler 2010-05-17 16:49:04 PDT 
Committed r59630: <http://trac.webkit.org/changeset/59630>
Comment 5 WebKit Review Bot 2010-05-17 16:58:33 PDT 
http://trac.webkit.org/changeset/59630 might have broken Qt Linux Release minimal and Chromium Linux Release
Comment 6 Eric Seidel (no email) 2010-05-17 17:08:08 PDT 
Looks like the compile failures are real.
Comment 7 Darin Adler 2010-05-17 17:12:16 PDT 
But not seen by the EWS. Ugh.
Comment 8 Eric Seidel (no email) 2010-05-17 17:13:28 PDT 
:(  Working on making the EWS faster this week.
Comment 9 Darin Adler 2010-05-17 17:15:57 PDT 
EWS built on Qt and was green. It was not too slow, it was wrong!
Comment 10 Darin Adler 2010-05-17 17:16:15 PDT 
http://trac.webkit.org/changeset/59632 is an attempt to fix the Qt build.
Comment 11 Eric Seidel (no email) 2010-05-17 17:18:16 PDT 
Hmmm.  I wonder if there are further dependency issues which need work on Qt.
Comment 12 Andrew Wilson 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.
Comment 13 Darin Adler 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.