Title: [153407] trunk/Source/WebCore
- Revision
- 153407
- Author
- [email protected]
- Date
- 2013-07-27 17:05:11 -0700 (Sat, 27 Jul 2013)
Log Message
HTMLParserScheduler gets into an inconsistent state when suspended for reasons
other than WillDeferLoading
https://bugs.webkit.org/show_bug.cgi?id=119172
Reviewed by Sam Weinig.
When loading is not deferred, even a suspended parser will be processing new data
from network, potentially starting its next chunk timer.
Limit suspending to when we can actually enforce it.
Here is what happens for each ReasonForSuspension:
- _javascript_DebuggerPaused: continuing to parse is probably wrong, but in practice,
this is unlikely to happen while debugging, and wasn't properly prevented before
this patch anyway.
- WillDeferLoading: No change in behavior.
- DocumentWillBecomeInactive: This is about page cache, and documents are only allowed
to be cached when fully loaded.
- PageWillBeSuspended: This appears to be part of Frame::suspendActiveDOMObjectsAndAnimations()
implementation, I'm guessing that it is appropriate to continue loading.
* dom/Document.cpp:
(WebCore::Document::suspendScheduledTasks):
(WebCore::Document::resumeScheduledTasks):
Only suspend/resume parsing when loading is deferred. This is not expressed directly,
but it's important to do this to avoid executing JS behind alerts and other modal dialogs.
* html/parser/HTMLParserScheduler.h: Added m_suspended member variable for assertions.
* html/parser/HTMLParserScheduler.cpp:
(WebCore::HTMLParserScheduler::HTMLParserScheduler):
(WebCore::HTMLParserScheduler::continueNextChunkTimerFired):
(WebCore::HTMLParserScheduler::scheduleForResume):
(WebCore::HTMLParserScheduler::suspend):
(WebCore::HTMLParserScheduler::resume):
Update m_suspended and assert as appropriate. No behavior changes for release mode.
* page/Frame.cpp: (WebCore::Frame::suspendActiveDOMObjectsAndAnimations):
Added a FIXME.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (153406 => 153407)
--- trunk/Source/WebCore/ChangeLog 2013-07-28 00:04:14 UTC (rev 153406)
+++ trunk/Source/WebCore/ChangeLog 2013-07-28 00:05:11 UTC (rev 153407)
@@ -1,5 +1,47 @@
2013-07-27 Alexey Proskuryakov <[email protected]>
+ HTMLParserScheduler gets into an inconsistent state when suspended for reasons
+ other than WillDeferLoading
+ https://bugs.webkit.org/show_bug.cgi?id=119172
+
+ Reviewed by Sam Weinig.
+
+ When loading is not deferred, even a suspended parser will be processing new data
+ from network, potentially starting its next chunk timer.
+
+ Limit suspending to when we can actually enforce it.
+
+ Here is what happens for each ReasonForSuspension:
+ - _javascript_DebuggerPaused: continuing to parse is probably wrong, but in practice,
+ this is unlikely to happen while debugging, and wasn't properly prevented before
+ this patch anyway.
+ - WillDeferLoading: No change in behavior.
+ - DocumentWillBecomeInactive: This is about page cache, and documents are only allowed
+ to be cached when fully loaded.
+ - PageWillBeSuspended: This appears to be part of Frame::suspendActiveDOMObjectsAndAnimations()
+ implementation, I'm guessing that it is appropriate to continue loading.
+
+ * dom/Document.cpp:
+ (WebCore::Document::suspendScheduledTasks):
+ (WebCore::Document::resumeScheduledTasks):
+ Only suspend/resume parsing when loading is deferred. This is not expressed directly,
+ but it's important to do this to avoid executing JS behind alerts and other modal dialogs.
+
+ * html/parser/HTMLParserScheduler.h: Added m_suspended member variable for assertions.
+
+ * html/parser/HTMLParserScheduler.cpp:
+ (WebCore::HTMLParserScheduler::HTMLParserScheduler):
+ (WebCore::HTMLParserScheduler::continueNextChunkTimerFired):
+ (WebCore::HTMLParserScheduler::scheduleForResume):
+ (WebCore::HTMLParserScheduler::suspend):
+ (WebCore::HTMLParserScheduler::resume):
+ Update m_suspended and assert as appropriate. No behavior changes for release mode.
+
+ * page/Frame.cpp: (WebCore::Frame::suspendActiveDOMObjectsAndAnimations):
+ Added a FIXME.
+
+2013-07-27 Alexey Proskuryakov <[email protected]>
+
Make SuspendableTimer safer
https://bugs.webkit.org/show_bug.cgi?id=119127
Modified: trunk/Source/WebCore/dom/Document.cpp (153406 => 153407)
--- trunk/Source/WebCore/dom/Document.cpp 2013-07-28 00:04:14 UTC (rev 153406)
+++ trunk/Source/WebCore/dom/Document.cpp 2013-07-28 00:05:11 UTC (rev 153407)
@@ -4816,7 +4816,12 @@
suspendActiveDOMObjects(reason);
scriptRunner()->suspend();
m_pendingTasksTimer.stop();
- if (m_parser)
+
+ // Deferring loading and suspending parser is necessary when we need to prevent re-entrant _javascript_ execution
+ // (e.g. while displaying an alert).
+ // It is not currently possible to suspend parser unless loading is deferred, because new data arriving from network
+ // will trigger parsing, and leave the scheduler in an inconsistent state where it doesn't know whether it's suspended or not.
+ if (reason == ActiveDOMObject::WillDeferLoading && m_parser)
m_parser->suspendScheduledTasks();
m_scheduledTasksAreSuspended = true;
@@ -4829,7 +4834,7 @@
ASSERT(m_scheduledTasksAreSuspended);
- if (m_parser)
+ if (reason == ActiveDOMObject::WillDeferLoading && m_parser)
m_parser->resumeScheduledTasks();
if (!m_pendingTasks.isEmpty())
m_pendingTasksTimer.startOneShot(0);
Modified: trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp (153406 => 153407)
--- trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp 2013-07-28 00:04:14 UTC (rev 153406)
+++ trunk/Source/WebCore/html/parser/HTMLParserScheduler.cpp 2013-07-28 00:05:11 UTC (rev 153407)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2010 Google, Inc. All Rights Reserved.
+ * Copyright (C) 2013 Apple, Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -99,6 +100,9 @@
, m_parserChunkSize(parserChunkSize(m_parser->document()->page()))
, m_continueNextChunkTimer(this, &HTMLParserScheduler::continueNextChunkTimerFired)
, m_isSuspendedWithActiveTimer(false)
+#if !ASSERT_DISABLED
+ , m_suspended(false)
+#endif
{
}
@@ -109,6 +113,7 @@
void HTMLParserScheduler::continueNextChunkTimerFired(Timer<HTMLParserScheduler>* timer)
{
+ ASSERT(!m_suspended);
ASSERT_UNUSED(timer, timer == &m_continueNextChunkTimer);
// FIXME: The timer class should handle timer priorities instead of this code.
// If a layout is scheduled, wait again to let the layout timer run first.
@@ -132,13 +137,18 @@
void HTMLParserScheduler::scheduleForResume()
{
+ ASSERT(!m_suspended);
m_continueNextChunkTimer.startOneShot(0);
}
-
void HTMLParserScheduler::suspend()
{
+ ASSERT(!m_suspended);
ASSERT(!m_isSuspendedWithActiveTimer);
+#if !ASSERT_DISABLED
+ m_suspended = true;
+#endif
+
if (!m_continueNextChunkTimer.isActive())
return;
m_isSuspendedWithActiveTimer = true;
@@ -147,7 +157,12 @@
void HTMLParserScheduler::resume()
{
+ ASSERT(m_suspended);
ASSERT(!m_continueNextChunkTimer.isActive());
+#if !ASSERT_DISABLED
+ m_suspended = false;
+#endif
+
if (!m_isSuspendedWithActiveTimer)
return;
m_isSuspendedWithActiveTimer = false;
Modified: trunk/Source/WebCore/html/parser/HTMLParserScheduler.h (153406 => 153407)
--- trunk/Source/WebCore/html/parser/HTMLParserScheduler.h 2013-07-28 00:04:14 UTC (rev 153406)
+++ trunk/Source/WebCore/html/parser/HTMLParserScheduler.h 2013-07-28 00:05:11 UTC (rev 153407)
@@ -103,6 +103,9 @@
int m_parserChunkSize;
Timer<HTMLParserScheduler> m_continueNextChunkTimer;
bool m_isSuspendedWithActiveTimer;
+#if !ASSERT_DISABLED
+ bool m_suspended;
+#endif
};
}
Modified: trunk/Source/WebCore/page/Frame.cpp (153406 => 153407)
--- trunk/Source/WebCore/page/Frame.cpp 2013-07-28 00:04:14 UTC (rev 153406)
+++ trunk/Source/WebCore/page/Frame.cpp 2013-07-28 00:05:11 UTC (rev 153407)
@@ -963,6 +963,7 @@
if (wasSuspended)
return;
+ // FIXME: Suspend/resume calls will not match if the frame is navigated, and gets a new document.
if (document()) {
document()->suspendScriptedAnimationControllerCallbacks();
animation()->suspendAnimationsForDocument(document());
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
