Bug 23677 – Reduce use of memcpy in TextResourceDecoder

• Status: RESOLVED
• Resolution: INVALID
• Component: Text

[Darin Adler]

The actual decoding in TextResourceDecoder optimizes the case where the buffer is empty. But checkForCSSCharset and checkForHeadCharset should do that too.

[Darin Adler]

Created attachment 27241 [details]
patch

[Dave Hyatt]

Comment on attachment 27241 [details]
patch

r=me

[Stephanie Lewis]

When I tried to test this no content would get displayed on local pages.  I can load pages over the network.  

[Darin Adler]

(In reply to comment #3)
> When I tried to test this no content would get displayed on local pages.  I can
> load pages over the network.  

I'll do more testing and make sure this works in both cases before checking in.

[David Levin]

It looks like there is a case in which pEnd does not get set and is used.

const char* pEnd;

if (bytesEqual(ptr, '<', '?', 'x', 'm', 'l')) {
    pEnd = ptr + dataSize;
    ...
} else if (bytesEqual(ptr, '<', 0, '?', 0, 'x', 0)) {
   ...
  return true;
}
...

If none of the "if" conditions is true, then pEnd is left unset and then is used in the while loop right after this.

[Darin Adler]

Comment on attachment 27241 [details]
patch

Besides the problem David Levin spotted, there are others too. I'll put up a new patch at some point. I started working on this a few days ago but haven't had any time to finish it off.

[Darin Adler]

Created attachment 237748 [details]
Patch

[Build Bot]

Comment on attachment 237748 [details]
Patch

Attachment 237748 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6688987141898240

New failing tests:
http/tests/appcache/document-write-html-element-2.html
accessibility/anchor-linked-anonymous-block-crash.html
http/tests/appcache/auth.html
http/tests/appcache/document-write-html-element.html
accessibility/aria-presentational-role.html
accessibility/aria-labelledby-on-input.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
accessibility/aria-labeled-with-hidden-node.html
http/tests/appcache/credential-url.html
accessibility/aria-activedescendant-crash.html
canvas/philip/tests/2d.canvas.readonly.html
http/tests/appcache/different-scheme.html
accessibility/aria-hidden.html
accessibility/aria-none-role.html
accessibility/aria-link-supports-press.html
compositing/bounds-in-flipped-writing-mode.html
http/tests/appcache/deferred-events.html
canvas/philip/tests/2d.canvas.reference.html
accessibility/aria-describedby-on-input.html
canvas/philip/tests/2d.clearRect.basic.html
animations/animation-css-rule-types.html
http/tests/appcache/empty-manifest.html
accessibility/aria-fallback-roles.html
http/tests/appcache/different-https-origin-resource-main.html
accessibility/adjacent-continuations-cause-assertion-failure.html
compositing/backface-visibility/backface-visibility-simple.html
http/tests/appcache/detached-iframe.html
accessibility/aria-help.html

[Build Bot]

Created attachment 237752 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Alexey Proskuryakov]

EWS was unhappy when trying the patch, and then just stopped. I'm trying to re-submit it to EWS now.

[Alexey Proskuryakov]

Comment on attachment 237748 [details]
Patch

Looks like the patch actually regresses the tests that EWS complained about above.

[Darin Adler]

Wonder what’s wrong. This is the same problem I had back 5 years ago!

[Darin Adler]

This code has been rewritten and this issue no longer exists.