39211 2010-05-17 02:45:06 -0700 [Chromium] Windows: Speed of indeterminate progress bar should be constant 2010-09-02 01:31:44 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC Windows Vista RESOLVED FIXED P2 Normal --- 0 morrita webkit-unassigned tkent oldest_to_newest 226709 0 morrita 2010-05-17 02:45:06 -0700 On Chromium, animation speed for indeterminate progress bar changes varies by the size of the bar. On windows, indeterminate progress bar shows animation should be constant speed Regardless the size of the bar. 226711 1 56229 morrita 2010-05-17 03:07:28 -0700 Created attachment 56229 patch v0 226712 2 morrita 2010-05-17 03:09:40 -0700 Kent-san, could you review this one? 226716 3 56229 tkent 2010-05-17 03:27:52 -0700 Comment on attachment 56229 patch v0 WebCore/rendering/RenderThemeChromiumWin.cpp:664 + static const double progressAnimationNumFrames = 60; If this value is not used anymore, remove it. WebCore/rendering/RenderThemeChromiumWin.cpp:665 + static const double progressIndeterminateOverlayDelta = 175; Could you add a comment why this value is 175 please? The symbol name wasn't clear to me at first glance. I recommend to make it contain "DeltaPerFrame" or "PixelsPerFrame". WebCore/rendering/RenderThemeChromiumWin.cpp:667 + static const int progressIndeterminateOverlayWidth = 120; ditto 226717 4 tkent 2010-05-17 03:30:41 -0700 (In reply to comment #3) > WebCore/rendering/RenderThemeChromiumWin.cpp:667 > + static const int progressIndeterminateOverlayWidth = 120; > ditto I meant ditto for adding a comment. The symbol name is ok. 226723 5 56232 morrita 2010-05-17 03:58:38 -0700 Created attachment 56232 patch v1 226725 6 morrita 2010-05-17 04:00:41 -0700 Hi kent-san, thank you for reviewing! (In reply to comment #3) > (From update of attachment 56229 [details]) > WebCore/rendering/RenderThemeChromiumWin.cpp:664 > + static const double progressAnimationNumFrames = 60; > If this value is not used anymore, remove it. Fixed. > > > WebCore/rendering/RenderThemeChromiumWin.cpp:665 > + static const double progressIndeterminateOverlayDelta = 175; > Could you add a comment why this value is 175 please? > The symbol name wasn't clear to me at first glance. I recommend to make it contain "DeltaPerFrame" or "PixelsPerFrame". Fixed to rename it to progressIndeterminateOverlayPixelsPerSecond. > > > WebCore/rendering/RenderThemeChromiumWin.cpp:667 > + static const int progressIndeterminateOverlayWidth = 120; Fixed to remove underling blank line to indicate comment coverage. 226729 7 56232 tkent 2010-05-17 04:38:57 -0700 Comment on attachment 56232 patch v1 OK. 227035 8 56232 morrita 2010-05-17 18:29:07 -0700 Comment on attachment 56232 patch v1 Clearing flags on attachment: 56232 Committed r59635: <http://trac.webkit.org/changeset/59635> 227036 9 morrita 2010-05-17 18:29:16 -0700 All reviewed patches have been landed. Closing bug. 56229 2010-05-17 03:07:28 -0700 2010-05-17 03:58:33 -0700 patch v0 bug-39211-20100517190726.patch text/plain 2940 morrita ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NGUxNmI4YzI0ZmU1ODRiMGZlM2E5Zjk0MTU5N2U1YTZiYTM0N2YwNS4uMWVmOTAxNjRkY2ZlNWVi MjZlZTY4NzI2MjdlY2U3NjZlNzY4YTUzOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wNS0xNyAgTU9S SVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbQ2hyb21pdW1dIFdpbmRvd3M6IFNwZWVkIG9mIGlu ZGV0ZXJtaW5hdGUgcHJvZ3Jlc3MgYmFyIHNob3VsZCBiZSBjb25zdGFudAorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzkyMTEKKworICAgICAgICBObyBu ZXcgdGVzdHMsIGFuaW1hdGlvbiBlZmZlY3QgY2FuIGJlIGNvbmZpcm1lZCBvbmx5IG1hbnVhbGx5 LgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1XaW4uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVDaHJvbWl1bVdpbjo6YW5pbWF0aW9uRHVyYXRpb25G b3JQcm9ncmVzc0Jhcik6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUNocm9taXVtV2lu OjpwYWludFByb2dyZXNzQmFyKToKKwogMjAxMC0wNS0xNiAgRGFuIEJlcm5zdGVpbiAgPG1pdHpA YXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZmIC0tZ2l0 IGEvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVDaHJvbWl1bVdpbi5jcHAgYi9XZWJDb3Jl L3JlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVtV2luLmNwcAppbmRleCA0N2Y3MWEyMDk5ZjJl MTk2YTc1YTMzOTE4OWJmZTg2NDdkMmI2MDQxLi40YmJhMTA5ZGZmYTU5OWU2ODU2ZGRkMGYyYWNm MDM3ZGU4YTZmMTE1IDEwMDY0NAotLS0gYS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUNo cm9taXVtV2luLmNwcAorKysgYi9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVt V2luLmNwcApAQCAtNjU5LDkgKzY1OSwxMiBAQCBib29sIFJlbmRlclRoZW1lQ2hyb21pdW1XaW46 OnBhaW50VGV4dEZpZWxkSW50ZXJuYWwoUmVuZGVyT2JqZWN0KiBvLAogLy8gTVNETiBzYXlzIHRo YXQgdXBkYXRlIGludGVydmFscyBmb3IgdGhlIGJhciBpcyAzMG1zLgogLy8gaHR0cDovL21zZG4u bWljcm9zb2Z0LmNvbS9lbi11cy9saWJyYXJ5L2JiNzYwODQyKHY9VlMuODUpLmFzcHgKIHN0YXRp YyBjb25zdCBkb3VibGUgcHJvZ3Jlc3NBbmltYXRpb25GcmFtZVJhdGUgPSAwLjAzMzsKLS8vIFRo ZXJlIGlzIG5vIGRvY3VtZW50YXRpb24gYWJvdXQgdGhlIGFuaW1hdGlvbiBzcGVlZCBvZiAKKy8v IFRoZXJlIGlzIG5vIGRvY3VtZW50YXRpb24gYWJvdXQgdGhlIGFuaW1hdGlvbiBzcGVlZCBub3Ig ZnJhbWUtcmF0ZSBvZiAKIC8vIHRoZSBpbmRldGVybWluYXRlIHByb2dyZXNzIGJhci4gU28gd2Ug anVzdCBndWVzcyBpdC4KIHN0YXRpYyBjb25zdCBkb3VibGUgcHJvZ3Jlc3NBbmltYXRpb25OdW1G cmFtZXMgPSA2MDsKK3N0YXRpYyBjb25zdCBkb3VibGUgcHJvZ3Jlc3NJbmRldGVybWluYXRlT3Zl cmxheURlbHRhID0gIDE3NTsKKworc3RhdGljIGNvbnN0IGludCBwcm9ncmVzc0luZGV0ZXJtaW5h dGVPdmVybGF5V2lkdGggPSAxMjA7CiAKIGRvdWJsZSBSZW5kZXJUaGVtZUNocm9taXVtV2luOjph bmltYXRpb25SZXBlYXRJbnRlcnZhbEZvclByb2dyZXNzQmFyKFJlbmRlclByb2dyZXNzKikgY29u c3QKIHsKQEAgLTY3Miw3ICs2NzUsNyBAQCBkb3VibGUgUmVuZGVyVGhlbWVDaHJvbWl1bVdpbjo6 YW5pbWF0aW9uRHVyYXRpb25Gb3JQcm9ncmVzc0JhcihSZW5kZXJQcm9ncmVzcyogcgogewogICAg IGlmIChyZW5kZXJQcm9ncmVzcy0+aXNEZXRlcm1pbmF0ZSgpKQogICAgICAgICByZXR1cm4gMDsK LSAgICByZXR1cm4gcHJvZ3Jlc3NBbmltYXRpb25OdW1GcmFtZXMgKiBwcm9ncmVzc0FuaW1hdGlv bkZyYW1lUmF0ZTsKKyAgICByZXR1cm4gKHJlbmRlclByb2dyZXNzLT53aWR0aCgpICsgcHJvZ3Jl c3NJbmRldGVybWluYXRlT3ZlcmxheVdpZHRoKSAvIHByb2dyZXNzSW5kZXRlcm1pbmF0ZU92ZXJs YXlEZWx0YTsKIH0KIAogdm9pZCBSZW5kZXJUaGVtZUNocm9taXVtV2luOjphZGp1c3RQcm9ncmVz c0JhclN0eWxlKENTU1N0eWxlU2VsZWN0b3IqLCBSZW5kZXJTdHlsZSosIEVsZW1lbnQqKSBjb25z dApAQCAtNjk0LDggKzY5Nyw4IEBAIGJvb2wgUmVuZGVyVGhlbWVDaHJvbWl1bVdpbjo6cGFpbnRQ cm9ncmVzc0JhcihSZW5kZXJPYmplY3QqIG8sIGNvbnN0IFJlbmRlck9iamVjCiAgICAgICAgICAg ICB2YWx1ZVJlY3QgPSBJbnRSZWN0KHIueCgpLCByLnkoKSwgZHgsIHIuaGVpZ2h0KCkpOwogICAg IH0gZWxzZSB7CiAgICAgICAgIHZhbHVlUGFydCA9IFBQX01PVkVPVkVSTEFZOwotICAgICAgICBp bnQgZHggPSByLndpZHRoKCkgKiByZW5kZXJQcm9ncmVzcy0+YW5pbWF0aW9uUHJvZ3Jlc3MoKSAq IDIgLSByLndpZHRoKCk7Ci0gICAgICAgIHZhbHVlUmVjdCA9IEludFJlY3Qoci54KCkgKyBkeCwg ci55KCksIHIud2lkdGgoKSwgci5oZWlnaHQoKSk7CisgICAgICAgIGludCBkeCA9IChyLndpZHRo KCkgKyBwcm9ncmVzc0luZGV0ZXJtaW5hdGVPdmVybGF5V2lkdGgpICogcmVuZGVyUHJvZ3Jlc3Mt PmFuaW1hdGlvblByb2dyZXNzKCkgLSBwcm9ncmVzc0luZGV0ZXJtaW5hdGVPdmVybGF5V2lkdGg7 CisgICAgICAgIHZhbHVlUmVjdCA9IEludFJlY3Qoci54KCkgKyBkeCwgci55KCksIHByb2dyZXNz SW5kZXRlcm1pbmF0ZU92ZXJsYXlXaWR0aCwgci5oZWlnaHQoKSk7CiAgICAgfQogCiAgICAgVGhl bWVQYWludGVyIHBhaW50ZXIoaS5jb250ZXh0LCByKTsK 56232 2010-05-17 03:58:38 -0700 2010-05-17 18:29:07 -0700 patch v1 bug-39211-20100517195837.patch text/plain 3098 morrita ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NGUxNmI4YzI0ZmU1ODRiMGZlM2E5Zjk0MTU5N2U1YTZiYTM0N2YwNS4uMWVmOTAxNjRkY2ZlNWVi MjZlZTY4NzI2MjdlY2U3NjZlNzY4YTUzOSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMC0wNS0xNyAgTU9S SVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBbQ2hyb21pdW1dIFdpbmRvd3M6IFNwZWVkIG9mIGlu ZGV0ZXJtaW5hdGUgcHJvZ3Jlc3MgYmFyIHNob3VsZCBiZSBjb25zdGFudAorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzkyMTEKKworICAgICAgICBObyBu ZXcgdGVzdHMsIGFuaW1hdGlvbiBlZmZlY3QgY2FuIGJlIGNvbmZpcm1lZCBvbmx5IG1hbnVhbGx5 LgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlclRoZW1lQ2hyb21pdW1XaW4uY3BwOgorICAg ICAgICAoV2ViQ29yZTo6UmVuZGVyVGhlbWVDaHJvbWl1bVdpbjo6YW5pbWF0aW9uRHVyYXRpb25G b3JQcm9ncmVzc0Jhcik6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJUaGVtZUNocm9taXVtV2lu OjpwYWludFByb2dyZXNzQmFyKToKKwogMjAxMC0wNS0xNiAgRGFuIEJlcm5zdGVpbiAgPG1pdHpA YXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZmIC0tZ2l0 IGEvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVDaHJvbWl1bVdpbi5jcHAgYi9XZWJDb3Jl L3JlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVtV2luLmNwcAppbmRleCA0N2Y3MWEyMDk5ZjJl MTk2YTc1YTMzOTE4OWJmZTg2NDdkMmI2MDQxLi41ZDM5Njk4OTc4MmMxODZhMjYxODYyMTdiZGM2 YzNiYWJmZTJlZGI1IDEwMDY0NAotLS0gYS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUNo cm9taXVtV2luLmNwcAorKysgYi9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJUaGVtZUNocm9taXVt V2luLmNwcApAQCAtNjU5LDkgKzY1OSwxMSBAQCBib29sIFJlbmRlclRoZW1lQ2hyb21pdW1XaW46 OnBhaW50VGV4dEZpZWxkSW50ZXJuYWwoUmVuZGVyT2JqZWN0KiBvLAogLy8gTVNETiBzYXlzIHRo YXQgdXBkYXRlIGludGVydmFscyBmb3IgdGhlIGJhciBpcyAzMG1zLgogLy8gaHR0cDovL21zZG4u bWljcm9zb2Z0LmNvbS9lbi11cy9saWJyYXJ5L2JiNzYwODQyKHY9VlMuODUpLmFzcHgKIHN0YXRp YyBjb25zdCBkb3VibGUgcHJvZ3Jlc3NBbmltYXRpb25GcmFtZVJhdGUgPSAwLjAzMzsKLS8vIFRo ZXJlIGlzIG5vIGRvY3VtZW50YXRpb24gYWJvdXQgdGhlIGFuaW1hdGlvbiBzcGVlZCBvZiAKLS8v IHRoZSBpbmRldGVybWluYXRlIHByb2dyZXNzIGJhci4gU28gd2UganVzdCBndWVzcyBpdC4KLXN0 YXRpYyBjb25zdCBkb3VibGUgcHJvZ3Jlc3NBbmltYXRpb25OdW1GcmFtZXMgPSA2MDsKKy8vIFRo ZXJlIGlzIG5vIGRvY3VtZW50YXRpb24gYWJvdXQgdGhlIGFuaW1hdGlvbiBzcGVlZCwgZnJhbWUt cmF0ZSwgbm9yIAorLy8gc2l6ZSBvZiBtb3Zpbmcgb3ZlcmxheSBvZiB0aGUgaW5kZXRlcm1pbmF0 ZSBwcm9ncmVzcyBiYXIuIAorLy8gU28gd2UganVzdCBvYnNlcnZlZCByZWFsLXdvcmxkIHByb2dy YW1zIGFuZCBndWVzc2VkIGZvbGxvd2luZyBwYXJhbWV0ZXJzLgorc3RhdGljIGNvbnN0IGRvdWJs ZSBwcm9ncmVzc0luZGV0ZXJtaW5hdGVPdmVybGF5UGl4ZWxzUGVyU2Vjb25kID0gIDE3NTsKK3N0 YXRpYyBjb25zdCBpbnQgcHJvZ3Jlc3NJbmRldGVybWluYXRlT3ZlcmxheVdpZHRoID0gMTIwOwog CiBkb3VibGUgUmVuZGVyVGhlbWVDaHJvbWl1bVdpbjo6YW5pbWF0aW9uUmVwZWF0SW50ZXJ2YWxG b3JQcm9ncmVzc0JhcihSZW5kZXJQcm9ncmVzcyopIGNvbnN0CiB7CkBAIC02NzIsNyArNjc0LDcg QEAgZG91YmxlIFJlbmRlclRoZW1lQ2hyb21pdW1XaW46OmFuaW1hdGlvbkR1cmF0aW9uRm9yUHJv Z3Jlc3NCYXIoUmVuZGVyUHJvZ3Jlc3MqIHIKIHsKICAgICBpZiAocmVuZGVyUHJvZ3Jlc3MtPmlz RGV0ZXJtaW5hdGUoKSkKICAgICAgICAgcmV0dXJuIDA7Ci0gICAgcmV0dXJuIHByb2dyZXNzQW5p bWF0aW9uTnVtRnJhbWVzICogcHJvZ3Jlc3NBbmltYXRpb25GcmFtZVJhdGU7CisgICAgcmV0dXJu IChyZW5kZXJQcm9ncmVzcy0+d2lkdGgoKSArIHByb2dyZXNzSW5kZXRlcm1pbmF0ZU92ZXJsYXlX aWR0aCkgLyBwcm9ncmVzc0luZGV0ZXJtaW5hdGVPdmVybGF5UGl4ZWxzUGVyU2Vjb25kOwogfQog CiB2b2lkIFJlbmRlclRoZW1lQ2hyb21pdW1XaW46OmFkanVzdFByb2dyZXNzQmFyU3R5bGUoQ1NT U3R5bGVTZWxlY3RvciosIFJlbmRlclN0eWxlKiwgRWxlbWVudCopIGNvbnN0CkBAIC02OTQsOCAr Njk2LDggQEAgYm9vbCBSZW5kZXJUaGVtZUNocm9taXVtV2luOjpwYWludFByb2dyZXNzQmFyKFJl bmRlck9iamVjdCogbywgY29uc3QgUmVuZGVyT2JqZWMKICAgICAgICAgICAgIHZhbHVlUmVjdCA9 IEludFJlY3Qoci54KCksIHIueSgpLCBkeCwgci5oZWlnaHQoKSk7CiAgICAgfSBlbHNlIHsKICAg ICAgICAgdmFsdWVQYXJ0ID0gUFBfTU9WRU9WRVJMQVk7Ci0gICAgICAgIGludCBkeCA9IHIud2lk dGgoKSAqIHJlbmRlclByb2dyZXNzLT5hbmltYXRpb25Qcm9ncmVzcygpICogMiAtIHIud2lkdGgo KTsKLSAgICAgICAgdmFsdWVSZWN0ID0gSW50UmVjdChyLngoKSArIGR4LCByLnkoKSwgci53aWR0 aCgpLCByLmhlaWdodCgpKTsKKyAgICAgICAgaW50IGR4ID0gKHIud2lkdGgoKSArIHByb2dyZXNz SW5kZXRlcm1pbmF0ZU92ZXJsYXlXaWR0aCkgKiByZW5kZXJQcm9ncmVzcy0+YW5pbWF0aW9uUHJv Z3Jlc3MoKSAtIHByb2dyZXNzSW5kZXRlcm1pbmF0ZU92ZXJsYXlXaWR0aDsKKyAgICAgICAgdmFs dWVSZWN0ID0gSW50UmVjdChyLngoKSArIGR4LCByLnkoKSwgcHJvZ3Jlc3NJbmRldGVybWluYXRl T3ZlcmxheVdpZHRoLCByLmhlaWdodCgpKTsKICAgICB9CiAKICAgICBUaGVtZVBhaW50ZXIgcGFp bnRlcihpLmNvbnRleHQsIHIpOwo=