Remove Node::aboutToUnload and be more explicit about what it was for
Created attachment 174376 [details] Patch
Comment on attachment 174376 [details] Patch Attachment 174376 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14845391
Comment on attachment 174376 [details] Patch Attachment 174376 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14843514
Created attachment 174382 [details] Patch Fix silly copy and paste fail
Comment on attachment 174382 [details] Patch Attachment 174382 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14831723
Comment on attachment 174382 [details] Patch Attachment 174382 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14848301
Comment on attachment 174382 [details] Patch Attachment 174382 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/14837715
Comment on attachment 174382 [details] Patch Attachment 174382 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14833672
Created attachment 174390 [details] Patch
Comment on attachment 174390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174390&action=review > Source/WebCore/ChangeLog:14 > + No new tests, this is just a refactor. refactoring? > Source/WebCore/loader/FrameLoader.cpp:395 > + if (currentFocusedNode && currentFocusedNode->toInputElement()) It's probably better if we had a local variable to avoid calling toInputElement twice.
(In reply to comment #10) > (From update of attachment 174390 [details]) > ... > > Source/WebCore/loader/FrameLoader.cpp:395 > > + if (currentFocusedNode && currentFocusedNode->toInputElement()) > > It's probably better if we had a local variable to avoid calling toInputElement twice. There's nothing performance sensitive here that should require caching the value and that just adds more lines of code.
Created attachment 174469 [details] Patch for landing
(In reply to comment #11) > (In reply to comment #10) > > (From update of attachment 174390 [details] [details]) > > ... > > > Source/WebCore/loader/FrameLoader.cpp:395 > > > + if (currentFocusedNode && currentFocusedNode->toInputElement()) > > > > It's probably better if we had a local variable to avoid calling toInputElement twice. > > There's nothing performance sensitive here that should require caching the value and that just adds more lines of code. Death by thousand cuts...
Comment on attachment 174469 [details] Patch for landing Clearing flags on attachment: 174469 Committed r134806: <http://trac.webkit.org/changeset/134806>
All reviewed patches have been landed. Closing bug.
(In reply to comment #13) > Death by thousand cuts... Isn't that an argument for any premature optimization? There's one small cut here that occurs during an already very expensive operation. Death by a thousand cuts applies to small costs that are distributed throughout the codebase and so they wouldn't show up on a profile even though they do affect overall performance. That doesn't seem to be the case here.