Looking at RedirectScheduler last night with othermaciej, it occurred to us that the psuedo-union ScheduleRedirection is really ugly. Othermaciej asked me to clean up the class, so here it is in a less ugly form.
Created attachment 52138 [details] Patch
Comment on attachment 52138 [details] Patch * Suggested renames: - AbstractRedirect --> ScheduledNavigation - SimpleRedirect --> ScheduledURLNavigation * It may or may not be helpful to also add "Scheduled" back to the concrete classes too. * Odd that locationChangePending is true for classes other than LocationChange. Also: even if the API on the scheduler is locationChangePending(), shouldn't the boolean method on an individual AbstractRedirect (or ScheduledNavigation) object be isLocationChange or countsAsLocationChange or something? It's only telling you about what it is, not what is pending. That's my style comments. r- to consider those. I will have to do another pass to check behavior-preservingness of the refactoring (since the lines got moved around a lot, was hard to check that with a linear scan).
Created attachment 52142 [details] Patch
Attachment 52142 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/1621116
Attachment 52142 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1572099
Created attachment 52144 [details] Patch
Comment on attachment 52144 [details] Patch > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > + Cleanup RedirectScheduler > + https://bugs.webkit.org/show_bug.cgi?id=36874 > + > + Removed the nutty ScheduledRedirection struct in favor of a hierarchy ^^^ bad indentation otherwise, this all looks good to me. given the naming of the file, it seems like it would be nice if the base interface were named ScheduledRedirect, but I realize why you chose not to do that.
Yeah, the better solution is probably to rename the class. :)
Created attachment 52188 [details] Patch for landing
Comment on attachment 52188 [details] Patch for landing Clearing flags on attachment: 52188 Committed r56875: <http://trac.webkit.org/changeset/56875>
All reviewed patches have been landed. Closing bug.