Making CString more efficient...
Created attachment 159592 [details] Patch
Comment on attachment 159592 [details] Patch Attachment 159592 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13549197
Comment on attachment 159592 [details] Patch Attachment 159592 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/13549200
Created attachment 159603 [details] Patch
Once again Chromium has issues with WebKitTestAPI. I skipped the test as usual. I email Julien Chaffraix about the issue.
Comment on attachment 159603 [details] Patch Attachment 159603 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13546271 New failing tests: accessibility/aria-checkbox-checked.html animations/3d/matrix-transform-type-animation.html http/tests/appcache/cyrillic-uri.html canvas/philip/tests/2d.canvas.readonly.html animations/animation-add-events-in-handler.html http/tests/appcache/deferred-events-delete-while-raising.html animations/additive-transform-animations.html animations/3d/replace-filling-transform.html accessibility/anchor-linked-anonymous-block-crash.html http/tests/appcache/deferred-events-delete-while-raising-timer.html accessibility/aria-disabled.html http/tests/appcache/crash-when-navigating-away-then-back.html animations/animation-border-overflow.html animations/animation-direction-alternate-reverse.html accessibility/accessibility-object-detached.html http/tests/appcache/abort-cache-ondownloading-manifest-404.html http/tests/appcache/credential-url.html http/tests/appcache/access-via-redirect.php animations/3d/change-transform-in-end-event.html animations/animation-controller-drt-api.html animations/3d/transform-perspective.html accessibility/accessibility-node-reparent.html http/tests/appcache/deferred-events.html animations/3d/state-at-end-event-transform.html animations/3d/transform-origin-vs-functions.html accessibility/accessibility-node-memory-management.html animations/animation-css-rule-types.html animations/animation-direction-reverse-fill-mode-hardware.html accessibility/adjacent-continuations-cause-assertion-failure.html accessibility/aria-help.html
Created attachment 159622 [details] Archive of layout-test-results from gce-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 159726 [details] Patch
(In reply to comment #6) > (From update of attachment 159603 [details]) > Attachment 159603 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/13546271 I could not reproduce any of the Chromium failure. I uploaded the same patch again, maybe the bot was sick?
Comment on attachment 159726 [details] Patch Ok, Chromium uses CStringBuffer in WebCString. I missed that. I'll update.
Created attachment 159738 [details] Patch
Comment on attachment 159738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159738&action=review > Source/WTF/wtf/text/CString.cpp:37 > + if (length > (numeric_limits<size_t>::max() - sizeof(CStringBuffer) / sizeof(char))) > + CRASH(); sizeof(char) is defined as 1 by the C and C++ language. It's usually considered better style to just omit it. > Source/WTF/wtf/text/CString.cpp:41 > + size_t size = sizeof(CStringBuffer) + length * sizeof(char); Same comment about sizeof(char). > Source/WTF/wtf/text/CString.cpp:57 > + if (!str) > + return; I probably would have put an ASSERT(!length) in there too. > Source/WTF/wtf/text/CString.h:35 > +// CStringBuffer is the ref-counted storage class for the characters of CStringBuffer. Should be "for the characters in a CString", not "for the characters of CStringBuffer". > Source/WTF/wtf/text/CString.h:36 > +// The data is implicitely allocated 1 character longer than length(), as it is zero-terminated. the word implicitly does not have the letter “e” > Source/WTF/wtf/text/CString.h:49 > + CStringBuffer(size_t length) > + : m_length(length) > + { } This is not quite the usual style. Normally we put everything on one line, or everything on different lines. It's unusual to have { } on one line with everything else on separate lines. I also suggest marking this constructor explicit. > Source/WTF/wtf/text/CString.h:52 > + size_t m_length; I think this should be defined const.
Committed r126191: <http://trac.webkit.org/changeset/126191>