Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(508)

Issue 583833007: Revert of Fix update of validity cache value, so that it reflects the correct state of any ... (Closed)

Created:
6 years, 3 months ago by spartha
Modified:
6 years, 3 months ago
Reviewers:
keishi, tkent
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Revert of Fix update of validity cache value, so that it reflects the correct state of any FormControlElement. (patchset #14 id:340001 of https://codereview.chromium.org/460343004/) Reason for revert: Caused regression in blink_perf. crbug.com/412054 Original issue's description: > Fix update of validity cache value, so that it reflects the correct > state of FormControlElements. > > This patch does a lazy update of validity, avoiding caching stale value. > setNeedsValidityCheck() now does not update validity, but flags it for update. > > BUG=400618, 403313 > TEST=LayoutTests/fast/forms/number/number-validity-badinput.html > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181422 TBR=keishi@chromium.org,tkent@chromium.org NOTREECHECKS=true NOTRY=true BUG=400618, 403313 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182379

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -38 lines) Patch
M LayoutTests/fast/forms/number/number-validity-badinput.html View 3 chunks +2 lines, -13 lines 0 comments Download
M LayoutTests/fast/forms/number/number-validity-badinput-expected.txt View 1 chunk +2 lines, -9 lines 0 comments Download
M Source/core/html/HTMLFormControlElement.h View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/html/HTMLFormControlElement.cpp View 2 chunks +7 lines, -12 lines 0 comments Download
M Source/core/html/HTMLInputElement.cpp View 2 chunks +1 line, -1 line 0 comments Download
M Source/core/html/HTMLTextAreaElement.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (2 generated)
spartha
Created Revert of Fix update of validity cache value, so that it reflects the correct ...
6 years, 3 months ago (2014-09-20 06:30:31 UTC) #1
tkent
lgtm. I was surprised that it caused regression, instead of improvement.
6 years, 3 months ago (2014-09-22 01:04:23 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/583833007/1
6 years, 3 months ago (2014-09-22 01:04:52 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1) as 182379
6 years, 3 months ago (2014-09-22 01:05:27 UTC) #6
spartha
6 years, 3 months ago (2014-09-24 15:07:51 UTC) #7
Message was sent while issue was closed.
On 2014/09/22 01:04:23, tkent (high review load) wrote:
> lgtm.
> I was surprised that it caused regression, instead of improvement.

The cause for the perf regression was the frequent style recalculation. I have
uploaded another patch https://codereview.chromium.org/601633004/ 
I check for change in validity to limit style update calls. This is similar to
older code, only that the validity value is cached in isValidFormControlElement
Please take a look and let me know if the approach is ok

Powered by Google App Engine
This is Rietveld 408576698