Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(634)

Unified Diff: third_party/WebKit/Source/core/loader/NavigationScheduler.cpp

Issue 2487403002: Allow navigations to frames that aren't being unloaded in the unload handler. (Closed)
Patch Set: Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
diff --git a/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp b/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
index a52bf28c0827f5310a0e239a0d70378389b47b14..95080bcbd1c51f678c4b45b101ec881f8e09e90d 100644
--- a/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
+++ b/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
@@ -106,8 +106,6 @@ void maybeLogScheduledNavigationClobber(ScheduledNavigationType type,
} // namespace
-unsigned NavigationDisablerForUnload::s_navigationDisableCount = 0;
-
class ScheduledNavigation
: public GarbageCollectedFinalized<ScheduledNavigation> {
WTF_MAKE_NONCOPYABLE(ScheduledNavigation);
@@ -365,36 +363,13 @@ bool NavigationScheduler::isNavigationScheduledWithin(double interval) const {
return m_redirect && m_redirect->delay() <= interval;
}
-// TODO(dcheng): There are really two different load blocking concepts at work
-// here and they have been incorrectly tangled together.
-//
-// 1. NavigationDisablerForUnload is for blocking navigation scheduling during
-// a beforeunload or unload events. Scheduled navigations during
-// beforeunload would make it possible to get trapped in an endless loop of
-// beforeunload dialogs. Scheduled navigations during the unload handler
-// makes is possible to cancel a navigation that was initiated right before
-// it commits.
-//
-// Checking Frame::isNavigationAllowed() doesn't make sense in this context:
-// NavigationScheduler is always cleared when a new load commits, so it's
-// impossible for a scheduled navigation to clobber a navigation that just
-// committed.
-//
-// 2. FrameNavigationDisabler / LocalFrame::isNavigationAllowed() are intended
-// to prevent Documents from being reattached during destruction, since it
-// can cause bugs with security origin confusion. This is primarily intended
-// to block /synchronous/ navigations during things lke
-// Document::detachLayoutTree().
inline bool NavigationScheduler::shouldScheduleReload() const {
- return m_frame->page() && m_frame->isNavigationAllowed() &&
- NavigationDisablerForUnload::isNavigationAllowed();
+ return m_frame->page() && m_frame->isNavigationAllowed();
}
inline bool NavigationScheduler::shouldScheduleNavigation(
const String& url) const {
- return m_frame->page() && m_frame->isNavigationAllowed() &&
- (protocolIsJavaScript(url) ||
- NavigationDisablerForUnload::isNavigationAllowed());
+ return m_frame->page() && m_frame->isNavigationAllowed();
}
void NavigationScheduler::scheduleRedirect(double delay, const String& url) {

Powered by Google App Engine
This is Rietveld 408576698