[TextAutosizing] Refactoring to eliminate boolean parameter
Created attachment 189556 [details] Patch
Comment on attachment 189556 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189556&action=review > Source/WebCore/rendering/TextAutosizer.cpp:178 > + float multiplier = clusterShouldBeAutosized(clusterInfo, textWidth) ? clusterMultiplier(clusterInfo, windowInfo, textWidth) : 1.0f; Nit: Might be clearer to first compute a "bool shouldBeAutosized" on a separate line.
This a nice refactoring - definitely makes things clearer (though I had to resort to "git diff --word-diff" to understand the patch!). Julien (as I think it was your suggestion to remove the boolean parameter), what do you think?
Created attachment 189587 [details] Patch
Comment on attachment 189587 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189587&action=review > Source/WebCore/rendering/TextAutosizer.cpp:157 > + multiplier = std::max(1.0f, multiplier); > > + return multiplier; Nit: These 2 lines could be squashed in one: return std::max(1.0f, multiplier); > Source/WebCore/rendering/TextAutosizer.h:32 > +#include <WebCore/platform/text/WritingMode.h> We don't use absolute path when #including in WebCore. This should be enough: #include "WritingMode.h"
Created attachment 189760 [details] Patch
Comment on attachment 189760 [details] Patch You don't need to ask for review again, only commit-queue so that you are not stuck waiting on a reviewer.
Comment on attachment 189760 [details] Patch Clearing flags on attachment: 189760 Committed r143749: <http://trac.webkit.org/changeset/143749>
All reviewed patches have been landed. Closing bug.