Eliminate some cases of double hashing, other related refactoring
Created attachment 327328 [details] Patch
Comment on attachment 327328 [details] Patch Attachment 327328 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5295660 Number of test failures exceeded the failure limit.
Created attachment 327330 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 327331 [details] Patch
Comment on attachment 327331 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327331&action=review > Source/WebCore/html/FormController.cpp:325 > + String signature = formSignature(*form); I would use auto here. > Source/WebCore/html/FormController.h:37 > +using FormControlState = Vector<String>; Nice. > Source/WebCore/html/HTMLTextAreaElement.cpp:117 > + return m_isDirty ? FormControlState { { value() } } : FormControlState { }; In cases like these, I wonder if if (!m_dirty) return { }; return { { value() } }; is nicer due to not needing to restate FormControlState. But looking at it, I think the ternary is nicer.
Committed r225037: <https://trac.webkit.org/changeset/225037>
<rdar://problem/35639247>
I'm seeing compile issues with the AppleWin debug build that seem to point to this patch when compiling WebKitLegacy. warning C4373: 'WebCore::InspectorDOMAgent::resolveNode': virtual function overrides 'Inspector::DOMBackendDispatcherHandler::resolveNode', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4373 Adding #pragma warning( disable : 4373) to InspectorDOMAgent.h seems to get things to build. Should the definitions in DOMBackendDispatchHandler be updated or should we just be disabling this warning on Windows?
(In reply to Don Olmstead from comment #8) > Should the definitions in DOMBackendDispatchHandler be updated or should we > just be disabling this warning on Windows? The warning doesn’t point to any problem we actually have. It points to problems we might have had with "previous versions of the compiler". It’s OK to disable the warning. But I think that an even better fix for this issues is to remove *all* the bogus const from DOMBackendDispatchHandler and wherever else we have it. There is not a good reason to mark function arguments themselves const.
(In reply to Darin Adler from comment #9) > (In reply to Don Olmstead from comment #8) > > Should the definitions in DOMBackendDispatchHandler be updated or should we > > just be disabling this warning on Windows? > > The warning doesn’t point to any problem we actually have. It points to > problems we might have had with "previous versions of the compiler". It’s OK > to disable the warning. > > But I think that an even better fix for this issues is to remove *all* the > bogus const from DOMBackendDispatchHandler and wherever else we have it. > There is not a good reason to mark function arguments themselves const. Thanks for the suggestion. I'll take a look at this when I get some free time to hack on the protocol generator some more. Filed: https://bugs.webkit.org/show_bug.cgi?id=180043