Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2154853002:
[SkXMLParser] Initial text node support (Closed)
Description[SkXMLParser] Initial text node support
Also disable entity processing.
R=bungeman@google.com,robertphillips@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2154853002
Committed: https://skia.googlesource.com/skia/+/125845553bfa25ef86b059d01254148cae708b59
Patch Set 1 #Patch Set 2 : buffered text #
Total comments: 6
Patch Set 3 : review #Patch Set 4 : rebase #Messages
Total messages: 28 (22 generated)
Description was changed from ========== [SkXMLParser] Initial text node support Also disable entity processing. R=bungeman@google.com,robertphillips@google.com ========== to ========== [SkXMLParser] Initial text node support Also disable entity processing. R=bungeman@google.com,robertphillips@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2154853002 ==========
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on master.client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp File src/xml/SkXMLParser.cpp (right): https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp... src/xml/SkXMLParser.cpp:77: SkAutoTCallVProc<skstd::remove_pointer_t<XML_Parser>, XML_ParserFree> fXMLParser; private: ? https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp... src/xml/SkXMLParser.cpp:98: void XMLCALL text_handler(void *data, const char* txt, int len) { Should be have a macro like "HANDLER_PREAMBLE(ctx)" that does this hard cast and hides the type? https://codereview.chromium.org/2154853002/diff/20001/tests/SkDOMTest.cpp File tests/SkDOMTest.cpp (right): https://codereview.chromium.org/2154853002/diff/20001/tests/SkDOMTest.cpp#new... tests/SkDOMTest.cpp:13: I think we go with file static over anonymous namespace.
https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp File src/xml/SkXMLParser.cpp (right): https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp... src/xml/SkXMLParser.cpp:77: SkAutoTCallVProc<skstd::remove_pointer_t<XML_Parser>, XML_ParserFree> fXMLParser; On 2016/07/18 15:29:18, robertphillips wrote: > private: ? Done. https://codereview.chromium.org/2154853002/diff/20001/src/xml/SkXMLParser.cpp... src/xml/SkXMLParser.cpp:98: void XMLCALL text_handler(void *data, const char* txt, int len) { On 2016/07/18 15:29:18, robertphillips wrote: > Should be have a macro like "HANDLER_PREAMBLE(ctx)" that does this hard cast and > hides the type? Done. https://codereview.chromium.org/2154853002/diff/20001/tests/SkDOMTest.cpp File tests/SkDOMTest.cpp (right): https://codereview.chromium.org/2154853002/diff/20001/tests/SkDOMTest.cpp#new... tests/SkDOMTest.cpp:13: On 2016/07/18 15:29:18, robertphillips wrote: > I think we go with file static over anonymous namespace. Done.
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-Trybot on master.client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)
The CQ bit was checked by fmalita@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robertphillips@google.com Link to the patchset: https://codereview.chromium.org/2154853002/#ps60001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by fmalita@chromium.org
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by fmalita@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [SkXMLParser] Initial text node support Also disable entity processing. R=bungeman@google.com,robertphillips@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2154853002 ========== to ========== [SkXMLParser] Initial text node support Also disable entity processing. R=bungeman@google.com,robertphillips@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2154853002 Committed: https://skia.googlesource.com/skia/+/125845553bfa25ef86b059d01254148cae708b59 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/125845553bfa25ef86b059d01254148cae708b59 |
