137631 2014-10-10 18:13:56 -0700 DOMTimer nesting level may not impact each other. 2023-06-23 20:06:01 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All NEW P2 Normal --- 1 chenhao webkit-unassigned ahmad.saleem792 ap barraclough bfulgham cdumez chenhao commit-queue darin rniwa oldest_to_newest 1041083 0 chenhao 2014-10-10 18:13:56 -0700 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. 1041084 1 chenhao 2014-10-10 18:22:03 -0700 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. 1042081 2 chenhao 2014-10-15 23:47:06 -0700 (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. 1042711 3 240093 chenhao 2014-10-19 19:46:54 -0700 Created attachment 240093 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? 1042725 4 darin 2014-10-19 20:41:51 -0700 The correct way to ask for review is to set the review flag to ?, not to +. 1042728 5 commit-queue 2014-10-19 20:43:14 -0700 Attachment 240093 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. 1042732 6 240098 chenhao 2014-10-19 21:35:40 -0700 Created attachment 240098 Fixing patch for review Corrected the review flag and coding style. Thanks. 1042844 7 240098 darin 2014-10-20 09:53:10 -0700 Comment on attachment 240098 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. 1043074 8 240176 chenhao 2014-10-20 19:24:10 -0700 Created attachment 240176 Fixing patch for review Thanks for Darin's Review! 1043075 9 chenhao 2014-10-20 19:27:18 -0700 (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. 1125671 10 240093 ossy 2015-09-14 11:13:36 -0700 Comment on attachment 240093 Fixing patch for review Cleared review+ from obsolete attachment 240093 so that this bug does not appear in http://webkit.org/pending-commit. 1503259 11 lforschler 2019-02-06 09:18:50 -0800 Mass move bugs into the DOM component. 1887856 12 ahmad.saleem792 2022-07-30 07:40:30 -0700 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". 1963458 13 ahmad.saleem792 2023-06-23 20:06:01 -0700 Similar patch in Blink: https://chromium.googlesource.com/chromium/src.git/+/81f013144ccfc759101193455919b4c1d04c1715 240093 2014-10-19 19:46:54 -0700 2015-09-14 11:13:36 -0700 Fixing patch for review review.patch text/plain 1027 chenhao SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NDg3MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE0LTEwLTE5ICBIb3JreSBD aGVuICA8Y2hlbmhhb0B1Y3dlYi5jb20+CisKKyAgICAgICAgRE9NVGltZXIgbmVzdGluZyBsZXZl bCBtYXkgbm90IGltcGFjdCBlYWNoIG90aGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xMzc2MzEKKworICAgICAgICBVbnJldmlld2VkLgorCisgICAg ICAgICogcGFnZS9ET01UaW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpET01UaW1lcjo6Zmly ZWQpOgorCiAyMDE0LTEwLTE5ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlh LmNvbT4KIAogICAgICAgICBbR1RLXSBNb3ZlIEd0a0RyYWdBbmREcm9wSGVscGVyIGZyb20gUGxh dGZvcm0gdG8gV2ViS2l0MgpJbmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9ET01UaW1lci5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9ET01UaW1lci5jcHAJKHJldmlzaW9u IDE3NDg3MykKKysrIFNvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NVGltZXIuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0yMDYsNyArMjA2LDkgQEAgdm9pZCBET01UaW1lcjo6ZmlyZWQoKQogICAgICAgICAg ICAgbV90aHJvdHRsZVN0YXRlID0gU2hvdWxkVGhyb3R0bGU7CiAgICAgICAgICAgICB1cGRhdGVU aW1lckludGVydmFsSWZOZWNlc3NhcnkoKTsKICAgICAgICAgfQotCisgICAgICAgIAorCWNvbnRl eHQtPnNldFRpbWVyTmVzdGluZ0xldmVsKDApOworCQogICAgICAgICByZXR1cm47CiAgICAgfQog Cg== 240098 2014-10-19 21:35:40 -0700 2014-10-20 19:24:10 -0700 Fixing patch for review review.patch text/plain 976 chenhao SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NDg3MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE0LTEwLTE5ICBIb3JreSBD aGVuICA8Y2hlbmhhb0B1Y3dlYi5jb20+CisKKyAgICAgICAgRE9NVGltZXIgbmVzdGluZyBsZXZl bCBtYXkgbm90IGltcGFjdCBlYWNoIG90aGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xMzc2MzEKKworICAgICAgICBVbnJldmlld2VkLgorCisgICAg ICAgICogcGFnZS9ET01UaW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpET01UaW1lcjo6Zmly ZWQpOgorCiAyMDE0LTEwLTE5ICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlh LmNvbT4KIAogICAgICAgICBbR1RLXSBNb3ZlIEd0a0RyYWdBbmREcm9wSGVscGVyIGZyb20gUGxh dGZvcm0gdG8gV2ViS2l0MgpJbmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9ET01UaW1lci5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9ET01UaW1lci5jcHAJKHJldmlzaW9u IDE3NDg3MykKKysrIFNvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NVGltZXIuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0yMDcsNiArMjA3LDggQEAgdm9pZCBET01UaW1lcjo6ZmlyZWQoKQogICAgICAgICAg ICAgdXBkYXRlVGltZXJJbnRlcnZhbElmTmVjZXNzYXJ5KCk7CiAgICAgICAgIH0KIAorICAgICAg ICBjb250ZXh0LT5zZXRUaW1lck5lc3RpbmdMZXZlbCgwKTsKKwogICAgICAgICByZXR1cm47CiAg ICAgfQogCg== 240176 2014-10-20 19:24:10 -0700 2014-10-22 20:09:52 -0700 Fixing patch for review review.patch text/plain 1464 chenhao SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NDkxOCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDE0LTEwLTIwICBIb3JreSBD aGVuICA8Y2hlbmhhb0B1Y3dlYi5jb20+CisKKyAgICAgICAgRE9NVGltZXIgbmVzdGluZyBsZXZl bCBtYXkgbm90IGltcGFjdCBlYWNoIG90aGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xMzc2MzEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBSZXNldCB0aGUgc2hhcmVkIG5lc3RpbmcgbGV2ZWwgd2hlbiBm aW5pc2hpbmcgb25lIHJlcGVhdCBvZiBET01UaW1lci4KKyAgICAgICAgVGhlIG5lc3RpbmcgbGV2 ZWwgb2YgU2NyaXB0IEV4ZWN1dGlvbiBDb250ZXh0IHNob3VsZCBwcmVzZW50IHRoZSBudW1iZXIg b2YgcnVubmluZyBET01UaW1lciBpbiBvbmUgZG9jdW1lbnQuIAorICAgICAgICBFdmVuIGlmIHNl dmVyYWwgcmVwZWF0IHRpbWVycyBhcmUgYWN0aXZlLCB0aGUgbmVzdGluZyBsZXZlbCBjb3VsZCBm aWd1cmUgb3V0IGlmIG9uZSB0aW1lciBpcyBhY3R1YWxseSBuZXN0ZWQuCisKKyAgICAgICAgTm90 IHRlc3RhYmxlLCB0aGF0IG5lZWRzIHVzZXIgb3BlcmF0aW9uIGR1cmluZyB0ZXN0aW5nLiBQbGVh c2UgcmVmZXIgdG8gYnVnIGRlc2NyaXB0aW9uIGZvciB0ZXN0aW5nLgorCisgICAgICAgICogcGFn ZS9ET01UaW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpET01UaW1lcjo6ZmlyZWQpOgorICAg ICAgICAgICAgLSByZXNldCB0aGUgbmVzdGluZyBsZXZlbCBiZWZvcmUgZWFybHkgcmV0dXJuLgor CiAyMDE0LTEwLTIwICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgogCiAgICAg ICAgIE1ha2UgcG9zdCBjaGVja2luIHN1Z2dlc3RlZCBjaGFuZ2VzIHRvIHIxNzQ4NDcKSW5kZXg6 IFNvdXJjZS9XZWJDb3JlL3BhZ2UvRE9NVGltZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJDb3JlL3BhZ2UvRE9NVGltZXIuY3BwCShyZXZpc2lvbiAxNzQ5MTgpCisrKyBTb3VyY2UvV2Vi Q29yZS9wYWdlL0RPTVRpbWVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMjA3LDYgKzIwNyw3IEBA IHZvaWQgRE9NVGltZXI6OmZpcmVkKCkKICAgICAgICAgICAgIHVwZGF0ZVRpbWVySW50ZXJ2YWxJ Zk5lY2Vzc2FyeSgpOwogICAgICAgICB9CiAKKyAgICAgICAgY29udGV4dC0+c2V0VGltZXJOZXN0 aW5nTGV2ZWwoMCk7CiAgICAgICAgIHJldHVybjsKICAgICB9CiAK