First, there is a long story which come from: https://bugs.webkit.org/show_bug.cgi?id=42861 Current situation is the timer could forward the gesture state only if the nesting level is zero. The problem is the nesting level is managed in the Document (ScriptExecutionContext), that means all timers in the same document share one nesting level. If one repeat timer was launched, all other timers after that could not forward the gesture state, due to all of them has a non-zero nesting level. Below is my testing page, three images at the end of page could delay the onload event. Then follow below steps to reproduce what I met: 1. Click "Start repeating timer" first 2. After the text change to "Running...", click "Goto next page" 3. When WebKit page come out, click back button. The expected page is the testing page, but it moves to the page before it, because the NavigationScheduler::mustLockBackForwardList refused the creation of new history item. <html> <head> </head> <body> <input type="button" value="Goto next page" onclick="gotoNextPage();"> <input type="button" value="Start repeating timer" onclick="startRepeatingTimer();"> <p> <div id="Timer">Paused</div> </p> <script type="text/javascript"> function gotoNextPage(){ setTimeout(function(){location.href="http://www.webkit.org/";},300); } function startRepeatingTimer(){ setInterval(function(){document.getElementById("Timer").innerHTML="Running...";},500); } </script> <img src="http://therecoveringpolitician.com/wp-content/uploads/2013/01/moderate.jpg" width="640" height="480"> <img src="http://cullogo.com/full/wallpapers-high-resolution-widescreen-hd-pk.jpg" width="640" height="480"> <img src="http://www.highresolutionwallpapers.net/wallpapers/autumn-1680x1050.jpg" width="640" height="480"> </body> </html> According to Web App APIs defintion(http://www.w3.org/html/wg/drafts/html/CR/webappapis.html#timer-nesting-level), the nesting level of repeating timer will be increased in the loop. I doubt that the nesting level should be owned by the timer itself, do not share with others. Just as described in the definition, that is task's nesting level.
I do not prefer to clear the nesting level before the early return in DOMTimer::fired, some algorithms rely on it. Maybe moving nesting level from Document to DOMTimer is a optional solution.
(In reply to comment #1) > I do not prefer to clear the nesting level before the early return in DOMTimer::fired, some algorithms rely on it. Maybe moving nesting level from Document to DOMTimer is a optional solution. I miss-understood that, the DOMTimer already has its own nesting level. So that is reasonable to clear the nesting level of script execution context.
Created attachment 240093 [details] Fixing patch for review I'd like to submit one patch to reset the nesting level in the script execution context, would you please help to review it?
The correct way to ask for review is to set the review flag to ?, not to +.
Attachment 240093 [details] did not pass style-queue: ERROR: Source/WebCore/page/DOMTimer.cpp:210: Tab found; better to use spaces [whitespace/tab] [1] ERROR: Source/WebCore/page/DOMTimer.cpp:211: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 240098 [details] Fixing patch for review Corrected the review flag and coding style. Thanks.
Comment on attachment 240098 [details] Fixing patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=240098&action=review Looks good. A few small issues. A larger issues is that we require a regression test with every bug fix. Can we construct one that demonstrates this bug? If we cannot, then the change log should include information about why we were not able to construct a regression test. > Source/WebCore/ChangeLog:6 > + Unreviewed. This needs to include the Reviewed by NOBODY (OOPS!) line so the patch management system can add the name of the reviewer here. > Source/WebCore/ChangeLog:9 > + (WebCore::DOMTimer::fired): This needs to describe the change and explain why it’s correct. > Source/WebCore/page/DOMTimer.cpp:211 > + I suggest omitting this blank line.
Created attachment 240176 [details] Fixing patch for review Thanks for Darin's Review!
(In reply to comment #7) > Comment on attachment 240098 [details] > Fixing patch for review > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240098&action=review > > Looks good. A few small issues. > > A larger issues is that we require a regression test with every bug fix. Can > we construct one that demonstrates this bug? If we cannot, then the change > log should include information about why we were not able to construct a > regression test. > > > Source/WebCore/ChangeLog:6 > > + Unreviewed. > > This needs to include the Reviewed by NOBODY (OOPS!) line so the patch > management system can add the name of the reviewer here. > > > Source/WebCore/ChangeLog:9 > > + (WebCore::DOMTimer::fired): > > This needs to describe the change and explain why it’s correct. > > > Source/WebCore/page/DOMTimer.cpp:211 > > + > > I suggest omitting this blank line. Thanks! I updated it again.
Comment on attachment 240093 [details] Fixing patch for review Cleared review+ from obsolete attachment 240093 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Mass move bugs into the DOM component.
I updated test case from Comment 0 with HTTPS link and new images: Link - https://jsfiddle.net/c10wr9y3/show In Safari 15.6, clicking on "Goto Next Page" does not work at all while it works fro all other browsers and I get following in Console: Refused to display 'https://webkit.org/' in a frame because it set 'X-Frame-Options' to 'SAMEORIGIN'. I am not sure this is the behavior about this bug or not but this is my testing results, all other browsers allow to clicking "Goto Next Page" and then navigate by giving errors and upon return, they show "Timer" as "PAUSED".
Similar patch in Blink: https://chromium.googlesource.com/chromium/src.git/+/81f013144ccfc759101193455919b4c1d04c1715