Bug 5877 – Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Remove remaining APPLE_CHANGES and KHTML_NO_CPP_DOM blocks from WebCore

The previous bug:
http://bugzilla.opendarwin.org/show_bug.cgi?id=5711
only removed APPLE_CHANGES from WebCore/khtml

This removes all the rest from WebCore (kwq mostly), with the exception of SVG, and additionally removes 
all KHTML_NO_CPP_DOM blocks.

[Eric Seidel (no email)]

Created attachment 4847 [details]
The patch

[Eric Seidel (no email)]

Created attachment 4848 [details]
cleanup.pl script

[Eric Seidel (no email)]

Created attachment 4849 [details]
A hacked version of the partial preprocessor (partpp.pl)

[Eric Seidel (no email)]

Comment on attachment 4847 [details]
The patch

This also removes all #if 0 blocks (which may or may not be a good thing).

[Darin Adler]

Comment on attachment 4847 [details]
The patch

This patch shows lots of cases where it leaves stray multiple blank lines. So
it's going to leave formatting a little messy. I'd like to see a version that
has a heuristic for nicely eating the excess blank lines.

I also see cases where there's a comment just before an #if 0 section. This
patch is going to lave those comments stranded. An example is in DOMHTML.mm.

Some of this already probably took place with the last "remove ifdefs" patch,
which I did not review in detail.

[Eric Seidel (no email)]

Created attachment 4855 [details]
Responding to Darin's comments.

[Eric Seidel (no email)]

Comment on attachment 4855 [details]
Responding to Darin's comments.

To address Darin's concerns, I did a couple things:

1.  I did a search throughout the entire project looking for "excessive"
newlines and removed them.
2.  I did a manual review of each of the previously changed files and removed
any extra comments, newlines.
3.  I removed dom_misc.* entirely, and moved the remaining DOM::EventListener
and DOM::NodeFilterCondition onto khtml::Shared<T>
4.  As part of my manual walk through khtml_part.cpp I removed
DIRECT_LINKAGE_TO_ECMA as well.

[Eric Seidel (no email)]

Comment on attachment 4847 [details]
The patch

Retracting review on this one for now.	See the new larger patch instead.

[Darin Adler]

Comment on attachment 4855 [details]
Responding to Darin's comments.

Here's an example of the kind of blank line thing I'm not so happy about:

---------

 #include "khtmlpart_p.h"

-#if !KHTML_NO_CPLUSPLUS_DOM
-#include "dom/html_document.h"
-#endif

 #include <CoreServices/CoreServices.h>

---------

and here's another:

---------

   cancelRedirection();

-#if !KHTML_NO_CPLUSPLUS_DOM
-  // null node activated.
-  emit nodeActivated(Node());
-#endif
 }

---------

Otherwise, this patch looks fine.

[Eric Seidel (no email)]

Comment on attachment 4855 [details]
Responding to Darin's comments.

Retracting review, posting a simpler patch instead.

[Eric Seidel (no email)]

Created attachment 4926 [details]
A newer hacked version of partpp.pl which has whitespace removal heuristics

[Eric Seidel (no email)]

Created attachment 4927 [details]
Diff from the original (http://www.phost.de/~stefan/Files/partpp.pl) partpp.pl (for the curious)

[Eric Seidel (no email)]

Created attachment 4928 [details]
SpacingHeuristics.pm perl module

[Eric Seidel (no email)]

Created attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)

[Eric Seidel (no email)]

Comment on attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)

This is a simpler patch which is entirely script generated and addresses
darin's whitespace concerns.  Script is also attached to the bug.  This should
be a trivial review which any reviewer should feel comfortable with.

[Eric Seidel (no email)]

Created attachment 4930 [details]
newer cleanup.pl (trivial partpp.pl wrapper)

[Maciej Stachowiak]

Comment on attachment 4929 [details]
completely script generated patch to remove APPLE_CHANGES and KHTML_NO_CPP_DOM (and whitespace)

rs=me