15055 2007-08-22 17:08:40 -0700 Divide by 0 in ProgressTracker::incrementProgress 2007-12-10 15:54:58 -0800 1 1 1 Unclassified WebKit Page Loading 523.x (Safari 3) All All RESOLVED FIXED P2 Normal --- 1 brettw brettw oldest_to_newest 1675 0 brettw 2007-08-22 17:08:40 -0700 int numPendingOrLoadingRequests = m_originatingProgressFrame->loader()->numPendingOrLoadingRequests(true); estimatedBytesForPendingRequests = progressItemDefaultEstimatedLength * numPendingOrLoadingRequests; remainingBytes = ((m_totalPageAndResourceBytesToLoad + estimatedBytesForPendingRequests) - m_totalBytesReceived); percentOfRemainingBytes = (double)bytesReceived / (double)remainingBytes; The problem here is that remainingBytes can be 0 depending on the (not very reliable) estimatedBytesForPendingRequests. This should be an easy fix. 907 1 16204 brettw 2007-09-05 09:01:29 -0700 Created attachment 16204 Patch 337 2 16204 oliver 2007-09-18 13:55:43 -0700 Comment on attachment 16204 Patch There should be an ASSERTION in the else block to check that remainingBytes is not <0 57000 3 brettw 2007-09-25 10:09:26 -0700 Why should there be an assertion? Won't that just look like a crash in release mode as well? From what I understand, the whole point is that the "remaining bytes" is an estimate and not necessarily reality. As far as the math here goes, you might as well be using rand(). It doesn't make sense to assert that rand() is any particular value. Unless you are going to assert that the remaining bytes is guaranteed accurate, it doesn't make sense to add an assertion here. 57403 4 16204 mjs 2007-09-29 20:22:07 -0700 Comment on attachment 16204 Patch The fix looks good to me (I can understand why the remainingBytes estimate could be wrong, since resources might not specify their length). But do you have a test case that can trigger this? I believe it could be done as an http test. r- to consider providing a test case, but if that is not practical just reflag for review and I'll approve it. 63537 5 16204 brettw 2007-12-06 14:52:45 -0800 Comment on attachment 16204 Patch I don't think it's possible to write a test very easily. I think this would depend on the reported length from the server being wrong (maybe possible to test) and also that the packets arrive and WebKit does processing such that the size of the amount received is exactly the same as the predicted amount, without being complete. Feel free to suggest a way if you know. I don't know any PHP and I've never written an HTTP test, so there could be something I don't know. 63539 6 16204 darin 2007-12-06 14:55:45 -0800 Comment on attachment 16204 Patch We already have regression tests for things like this: The regression test engine runs a local copy of Apache, and we write server-side scripts to deliver data and delay as appropriate. For example: LayoutTests/http/tests/incremental/slow-utf8-html.pl LayoutTests/http/tests/incremental/split-hex-entities.pl Is this situation enough different from those that making a test is impractical? 63574 7 16204 darin 2007-12-07 01:16:20 -0800 Comment on attachment 16204 Patch OK, you wore me down. Usually we do require tests, but since this is obviously correct and you're not inspired to make a test, r=me 63872 8 andrew 2007-12-10 15:54:58 -0800 Committed in r28593 16204 2007-09-05 09:01:29 -0700 2007-12-07 01:16:20 -0800 Patch zero.diff text/plain 1583 brettw SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNTM2OSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTIgQEAKKzIwMDctMDktMDUgIE1hcnZpbiBEZWNrZXIgIDxtYXJ2LmRlY2tlckBn bWFpbC5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgRml4IGEgZGl2aWRlIGJ5IDAgaW4gdGhlIHByb2dyZXNzIHRyYWNrZXIuCisKKyAgICAgICAg KiBsb2FkZXIvUHJvZ3Jlc3NUcmFja2VyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlByb2dyZXNz VHJhY2tlcjo6aW5jcmVtZW50UHJvZ3Jlc3MpOgorCiAyMDA3LTA5LTA0ICBNYXJ2aW4gRGVja2Vy ICA8bWFydi5kZWNrZXJAZ21haWwuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IE1hY2llaiBT dGFjaG93aWFrLgpJbmRleDogV2ViQ29yZS9sb2FkZXIvUHJvZ3Jlc3NUcmFja2VyLmNwcAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBXZWJDb3JlL2xvYWRlci9Qcm9ncmVzc1RyYWNrZXIuY3BwCShyZXZpc2lvbiAy NTM2OSkKKysrIFdlYkNvcmUvbG9hZGVyL1Byb2dyZXNzVHJhY2tlci5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTE5MSw3ICsxOTEsMTAgQEAgdm9pZCBQcm9ncmVzc1RyYWNrZXI6OmluY3JlbWVudFBy b2dyZXNzKAogICAgIGludCBudW1QZW5kaW5nT3JMb2FkaW5nUmVxdWVzdHMgPSBtX29yaWdpbmF0 aW5nUHJvZ3Jlc3NGcmFtZS0+bG9hZGVyKCktPm51bVBlbmRpbmdPckxvYWRpbmdSZXF1ZXN0cyh0 cnVlKTsKICAgICBlc3RpbWF0ZWRCeXRlc0ZvclBlbmRpbmdSZXF1ZXN0cyA9IHByb2dyZXNzSXRl bURlZmF1bHRFc3RpbWF0ZWRMZW5ndGggKiBudW1QZW5kaW5nT3JMb2FkaW5nUmVxdWVzdHM7CiAg ICAgcmVtYWluaW5nQnl0ZXMgPSAoKG1fdG90YWxQYWdlQW5kUmVzb3VyY2VCeXRlc1RvTG9hZCAr IGVzdGltYXRlZEJ5dGVzRm9yUGVuZGluZ1JlcXVlc3RzKSAtIG1fdG90YWxCeXRlc1JlY2VpdmVk KTsKLSAgICBwZXJjZW50T2ZSZW1haW5pbmdCeXRlcyA9IChkb3VibGUpYnl0ZXNSZWNlaXZlZCAv IChkb3VibGUpcmVtYWluaW5nQnl0ZXM7CisgICAgaWYgKHJlbWFpbmluZ0J5dGVzID4gMCkgIC8v IFByZXZlbnQgZGl2aWRlIGJ5IDAuCisgICAgICAgIHBlcmNlbnRPZlJlbWFpbmluZ0J5dGVzID0g KGRvdWJsZSlieXRlc1JlY2VpdmVkIC8gKGRvdWJsZSlyZW1haW5pbmdCeXRlczsKKyAgICBlbHNl CisgICAgICAgIHBlcmNlbnRPZlJlbWFpbmluZ0J5dGVzID0gMS4wOwogICAgIAogICAgIC8vIFRy ZWF0IHRoZSBmaXJzdCBsYXlvdXQgYXMgdGhlIGhhbGYtd2F5IHBvaW50LgogICAgIGRvdWJsZSBt YXhQcm9ncmVzc1ZhbHVlID0gbV9vcmlnaW5hdGluZ1Byb2dyZXNzRnJhbWUtPmxvYWRlcigpLT5m aXJzdExheW91dERvbmUoKSA/IGZpbmFsUHJvZ3Jlc3NWYWx1ZSA6IC41Owo=