[webkit-reviews] review denied: [Bug 36874] Cleanup RedirectScheduler : [Attachment 52138] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 31 01:29:38 PDT 2010


Maciej Stachowiak <mjs at apple.com> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 36874: Cleanup RedirectScheduler
https://bugs.webkit.org/show_bug.cgi?id=36874

Attachment 52138: Patch
https://bugs.webkit.org/attachment.cgi?id=52138&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
* 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).


More information about the webkit-reviews mailing list