Changeset 198924 in webkit

Timestamp:

Mar 31, 2016 3:43:45 PM (8 years ago)

Author:

dbates@webkit.org

Message:

REGRESSION (r195605): ASSERTION FAILED: !NoEventDispatchAssertion::isEventDispatchForbidden()
when pressing the back button on a page with a focused subframe
​https://bugs.webkit.org/show_bug.cgi?id=156033
<rdar://problem/25446561>

Reviewed by Chris Dumez.

Source/WebCore:

Fixes an assertion failure when navigating back, by pressing the browser back button, to
the previous page from a page with a focused subframe.

Following r195605 (​https://bugs.webkit.org/show_bug.cgi?id=153449), the responsibility for
dispatching a DOM pagehide event moved from CachedFrame to PageCache and we now instantiate
a NoEventDispatchAssertion object to enforce the invariant that no additional DOM events are
dispatched as part of adding a page to the page cache. When adding a page with a focused
subframe to the page cache we focus its main frame, which implicitly defocuses the subframe
and dispatches a DOM blur event at it. Therefore an assertion failure occurs when dispatching
this DOM blur event (because a NoEventDispatchAssertion object was allocated on the stack).

Test: fast/history/back-from-page-with-focused-iframe.html

• history/CachedFrame.cpp:

(WebCore::CachedFrame::CachedFrame): Move logic to focus the main frame from here...

• history/PageCache.cpp:

(WebCore::PageCache::addIfCacheable): to here such that any DOM blur and focus events
are dispatched before instantiate the NoEventDispatchAssertion object and enter the page
cache.

LayoutTests:

Add a test to ensure that when navigating back from a page with a focused <iframe> f, a DOM
blur event is dispatched to f, a DOM focus event is dispatched at the main frame and that
an assertion failure does not occur (only applicable in a debug build).

• fast/history/back-from-page-with-focused-iframe-expected.txt: Added.
• fast/history/back-from-page-with-focused-iframe.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/history/back-from-page-with-focused-iframe-expected.txt (added)
• LayoutTests/fast/history/back-from-page-with-focused-iframe.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/history/CachedFrame.cpp (modified) (2 diffs)
• Source/WebCore/history/PageCache.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-03-31  Daniel Bates  <dabates@apple.com>
+
+        REGRESSION (r195605): ASSERTION FAILED: !NoEventDispatchAssertion::isEventDispatchForbidden()
+        when pressing the back button on a page with a focused subframe
+        https://bugs.webkit.org/show_bug.cgi?id=156033
+        <rdar://problem/25446561>
+
+        Reviewed by Chris Dumez.
+
+        Add a test to ensure that when navigating back from a page with a focused <iframe> f, a DOM
+        blur event is dispatched to f, a DOM focus event is dispatched at the main frame and that
+        an assertion failure does not occur (only applicable in a debug build).
+
+        * fast/history/back-from-page-with-focused-iframe-expected.txt: Added.
+        * fast/history/back-from-page-with-focused-iframe.html: Added.
+
 2016-03-31  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-03-31  Daniel Bates  <dabates@apple.com>
+
+        REGRESSION (r195605): ASSERTION FAILED: !NoEventDispatchAssertion::isEventDispatchForbidden()
+        when pressing the back button on a page with a focused subframe
+        https://bugs.webkit.org/show_bug.cgi?id=156033
+        <rdar://problem/25446561>
+
+        Reviewed by Chris Dumez.
+
+        Fixes an assertion failure when navigating back, by pressing the browser back button, to
+        the previous page from a page with a focused subframe.
+
+        Following r195605 (https://bugs.webkit.org/show_bug.cgi?id=153449), the responsibility for
+        dispatching a DOM pagehide event moved from CachedFrame to PageCache and we now instantiate
+        a NoEventDispatchAssertion object to enforce the invariant that no additional DOM events are
+        dispatched as part of adding a page to the page cache. When adding a page with a focused
+        subframe to the page cache we focus its main frame, which implicitly defocuses the subframe
+        and dispatches a DOM blur event at it. Therefore an assertion failure occurs when dispatching
+        this DOM blur event (because a NoEventDispatchAssertion object was allocated on the stack).
+
+        Test: fast/history/back-from-page-with-focused-iframe.html
+
+        * history/CachedFrame.cpp:
+        (WebCore::CachedFrame::CachedFrame): Move logic to focus the main frame from here...
+        * history/PageCache.cpp:
+        (WebCore::PageCache::addIfCacheable): to here such that any DOM blur and focus events
+        are dispatched before instantiate the NoEventDispatchAssertion object and enter the page
+        cache.
+
 2016-03-31  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/history/CachedFrame.cpp

@@ -34 +34 @@
 #include "EventNames.h"
 #include "ExceptionCode.h"
-#include "FocusController.h"
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
@@ -148 +147 @@
     ASSERT(m_view);
 
-    if (frame.page()->focusController().focusedFrame() == &frame)
-        frame.page()->focusController().setFocusedFrame(&frame.mainFrame());
-
     // Custom scrollbar renderers will get reattached when the document comes out of the page cache
     m_view->detachCustomScrollbars();

trunk/Source/WebCore/history/PageCache.cpp

@@ -38 +38 @@
 #include "Document.h"
 #include "DocumentLoader.h"
+#include "FocusController.h"
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
@@ -393 +394 @@
     setInPageCache(*page, true);
 
+    // Focus the main frame, defocusing a focused subframe (if we have one). We do this here,
+    // before the page enters the page cache, while we still can dispatch DOM blur/focus events.
+    if (page->focusController().focusedFrame())
+        page->focusController().setFocusedFrame(&page->mainFrame());
+
     // Fire the pagehide event in all frames.
     firePageHideEventRecursively(page->mainFrame());