EmailInputType stores a ScriptRegexp in a static local keeping the regex context alive forever |
||||
Issue descriptionhttps://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/html/forms/EmailInputType.cpp&q=EmailInputType.cpp&sq=package:chromium&l=142 loyso@ in https://codereview.chromium.org/1360723003 made us forcibly clear the V8PerContextData in the regex scope when doing leak detection. This is to compensate for the fact that the static local makes the V8PerContextData live forever. We should probably just not create the static local ScriptRegexp at all and instead create a new one each time. If we think creating the regex object is too expensive we can cache it as a property of the v8::Context itself, though v8 is supposed to cache regexes for us anyway, so in theory creating a new ScriptRegexp for each EmailInputType should be "cheap". Doing this allows us to clear the regex context when under memory pressure, or periodically, since it's not used very often (only form validation and some random inspector things). Doing a single form validation should probably not cause a permanent memory increase the size of a v8::Context.
,
Mar 24 2016
tkent-san: Would you mind taking a look at this?
,
Jun 3 2016
,
Jun 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc3381f2aa9ccbb200f09d58ce34c22f7027491a commit fc3381f2aa9ccbb200f09d58ce34c22f7027491a Author: tkent <tkent@chromium.org> Date: Mon Jun 06 04:34:10 2016 Make ScriptRegexp for email address a member of EmailInputType. Make it a data member of EmailInputType instead of a function-local static variable. - Move EmailInputType constructor implementation to the .cpp file to void to include ScriptRegexp.h in EmailInputType.h. - Add ScriptRegexp arguments to convertEmailAddressToASCII() and isValidEmailAddress() and add createEmailRegexp() for ease of unit testing. BUG= 597530 Review-Url: https://codereview.chromium.org/2037963002 Cr-Commit-Position: refs/heads/master@{#397970} [modify] https://crrev.com/fc3381f2aa9ccbb200f09d58ce34c22f7027491a/third_party/WebKit/Source/core/html/forms/EmailInputType.cpp [modify] https://crrev.com/fc3381f2aa9ccbb200f09d58ce34c22f7027491a/third_party/WebKit/Source/core/html/forms/EmailInputType.h [modify] https://crrev.com/fc3381f2aa9ccbb200f09d58ce34c22f7027491a/third_party/WebKit/Source/core/html/forms/EmailInputTypeTest.cpp
,
Jun 6 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by esprehn@chromium.org
, Mar 24 2016