Bug 31079 - (r48167) "Document.h" includes "Page.h", makes WebCore painful to work on
Summary: (r48167) "Document.h" includes "Page.h", makes WebCore painful to work on
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-03 12:54 PST by Brady Eidson
Modified: 2009-11-03 13:06 PST (History)
0 users

See Also:


Attachments
Revert Document.h to a forward declaration of Page, move the inline function into Document.cpp, and add the Page.h include to the few other sites that now need it. (4.12 KB, patch)
2009-11-03 13:00 PST, Brady Eidson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2009-11-03 12:54:28 PST
In resolving https://bugs.webkit.org/show_bug.cgi?id=25503, http://trac.webkit.org/changeset/48167 changed a forward declaration of "Page" in "Document.h" to an include of "Page.h" and it seems like it was for the sole purpose of convenience (there's no indication that the inline status of the method in the header was important to this patch)

 I noticed this while working on BackForwardList.h and having to recompile over 1,000 files for every tweak I made.  Any time we add a header to "Document.h", we're increasing the change that work on the dark corners of WebCore will cause world rebuilds.

This is not good for the project.

I'll have a patch coming up shortly that removes #include "Page.h" from Document.h, and also removes #include "BackForwardList.h" from Page.h
Comment 1 Brady Eidson 2009-11-03 12:56:01 PST
In retrospect, the BackForwardList.h include is a lot older and would affect WebKit builds as well, so I'll hold off for now.

The Document.h include is still only a couple of months old and much much MUCH more severe, so I'll have a patch for that.
Comment 2 Brady Eidson 2009-11-03 13:00:58 PST
Created attachment 42409 [details]
Revert Document.h to a forward declaration of Page, move the inline function into Document.cpp, and add the Page.h include to the few other sites that now need it.
Comment 3 Darin Adler 2009-11-03 13:04:13 PST
Comment on attachment 42409 [details]
Revert Document.h to a forward declaration of Page, move the inline function into Document.cpp, and add the Page.h include to the few other sites that now need it.

> +InspectorTimelineAgent* Document::inspectorTimelineAgent() const {
> +    return page() ? page()->inspectorTimelineAgent() : 0;
> +}

Please move the brace to a separate line rather than having it on the same line as the function.

Otherwise, fine, r=me
Comment 4 Brady Eidson 2009-11-03 13:06:36 PST
http://trac.webkit.org/changeset/50476