From: robertphillips@google.com Date: Fri, 13 Dec 2013 19:36:25 +0000 (+0000) Subject: Improved SkPathRef interface security X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~9678 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0efb21bd1cd359b732a59753f3c1da096aab561a;p=platform%2Fupstream%2FlibSkiaSharp.git Improved SkPathRef interface security https://codereview.chromium.org/115323004/ git-svn-id: http://skia.googlecode.com/svn/trunk@12676 2bbb7eff-a529-9590-31e7-b0007b416f81 --- diff --git a/include/core/SkPath.h b/include/core/SkPath.h index 1526461..9b3f281 100644 --- a/include/core/SkPath.h +++ b/include/core/SkPath.h @@ -1005,7 +1005,9 @@ private: // 'rect' needs to be sorted void setBounds(const SkRect& rect) { - fPathRef->setBounds(rect); + SkPathRef::Editor ed(&fPathRef); + + ed.setBounds(rect); } #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index 99fd90a..c55f549 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -49,7 +49,8 @@ public: /** * Returns the array of points. */ - SkPoint* points() { return fPathRef->fPoints; } + SkPoint* points() { return fPathRef->getPoints(); } + const SkPoint* points() const { return fPathRef->points(); } /** * Gets the ith point. Shortcut for this->points() + i @@ -58,6 +59,10 @@ public: SkASSERT((unsigned) i < (unsigned) fPathRef->fPointCnt); return this->points() + i; }; + const SkPoint* atPoint(int i) const { + SkASSERT((unsigned) i < (unsigned) fPathRef->fPointCnt); + return this->points() + i; + }; /** * Adds the verb and allocates space for the number of points indicated by the verb. The @@ -89,6 +94,7 @@ public: void resetToSize(int newVerbCnt, int newPointCnt, int newConicCount) { fPathRef->resetToSize(newVerbCnt, newPointCnt, newConicCount); } + /** * Gets the path ref that is wrapped in the Editor. */ @@ -96,6 +102,8 @@ public: void setIsOval(bool isOval) { fPathRef->setIsOval(isOval); } + void setBounds(const SkRect& rect) { fPathRef->setBounds(rect); } + private: SkPathRef* fPathRef; }; @@ -158,13 +166,6 @@ public: return fBounds; } - void setBounds(const SkRect& rect) { - SkASSERT(rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom); - fBounds = rect; - fBoundsIsDirty = false; - fIsFinite = fBounds.isFinite(); - } - /** * Transforms a path ref by a matrix, allocating a new one only if necessary. */ @@ -299,6 +300,13 @@ private: fBoundsIsDirty = false; } + void setBounds(const SkRect& rect) { + SkASSERT(rect.fLeft <= rect.fRight && rect.fTop <= rect.fBottom); + fBounds = rect; + fBoundsIsDirty = false; + fIsFinite = fBounds.isFinite(); + } + /** Makes additional room but does not change the counts or change the genID */ void incReserve(int additionalVerbs, int additionalPoints) { SkDEBUGCODE(this->validate();) @@ -418,6 +426,12 @@ private: void setIsOval(bool isOval) { fIsOval = isOval; } + SkPoint* getPoints() { + SkDEBUGCODE(this->validate();) + fIsOval = false; + return fPoints; + } + enum { kMinSize = 256, }; @@ -441,6 +455,7 @@ private: mutable uint32_t fGenerationID; SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time. + friend class PathRefTest_Private; typedef SkRefCnt INHERITED; }; diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index a57e2f4..0711e3f 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -24,7 +24,6 @@ SkPathRef::Editor::Editor(SkAutoTUnref* pathRef, } fPathRef = *pathRef; fPathRef->fGenerationID = 0; - fPathRef->fIsOval = false; SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);) } diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 991b4fd..b33ef97 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -2679,8 +2679,8 @@ static void test_circle_with_add_paths(skiatest::Reporter* reporter) { SkMatrix translate; translate.setTranslate(SkIntToScalar(12), SkIntToScalar(12)); - // For simplicity, all the path concatenation related operations - // would mark it non-circle, though in theory it's still a circle. + // Although all the path concatenation related operations leave + // the path a circle, most mark it as a non-circle for simplicity // empty + circle (translate) path = empty; @@ -2690,7 +2690,7 @@ static void test_circle_with_add_paths(skiatest::Reporter* reporter) { // circle + empty (translate) path = circle; path.addPath(empty, translate); - check_for_circle(reporter, path, false, kCircleDir); + check_for_circle(reporter, path, true, kCircleDir); // test reverseAddPath path = circle; @@ -3138,77 +3138,74 @@ static void test_contains(skiatest::Reporter* reporter) { } } -static void test_pathref(skiatest::Reporter* reporter) { - static const int kRepeatCnt = 10; - - SkPathRef* pathRef = SkPathRef::CreateEmpty(); - SkAutoTUnref pathRef2(SkPathRef::CreateEmpty()); - SkMatrix mat; - - mat.setTranslate(10, 10); +class PathRefTest_Private { +public: + static void TestPathRef(skiatest::Reporter* reporter) { + static const int kRepeatCnt = 10; - SkPathRef::CreateTransformedCopy(&pathRef2, *pathRef, mat); + SkAutoTUnref pathRef(SkNEW(SkPathRef)); - SkPathRef::Editor ed(&pathRef2); + SkPathRef::Editor ed(&pathRef); - { - ed.growForRepeatedVerb(SkPath::kMove_Verb, kRepeatCnt); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countVerbs()); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countPoints()); - REPORTER_ASSERT(reporter, 0 == pathRef2->getSegmentMasks()); - for (int i = 0; i < kRepeatCnt; ++i) { - REPORTER_ASSERT(reporter, SkPath::kMove_Verb == pathRef2->atVerb(i)); + { + ed.growForRepeatedVerb(SkPath::kMove_Verb, kRepeatCnt); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countVerbs()); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countPoints()); + REPORTER_ASSERT(reporter, 0 == pathRef->getSegmentMasks()); + for (int i = 0; i < kRepeatCnt; ++i) { + REPORTER_ASSERT(reporter, SkPath::kMove_Verb == pathRef->atVerb(i)); + } + ed.resetToSize(0, 0, 0); } - ed.resetToSize(0, 0, 0); - } - { - ed.growForRepeatedVerb(SkPath::kLine_Verb, kRepeatCnt); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countVerbs()); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countPoints()); - REPORTER_ASSERT(reporter, SkPath::kLine_SegmentMask == pathRef2->getSegmentMasks()); - for (int i = 0; i < kRepeatCnt; ++i) { - REPORTER_ASSERT(reporter, SkPath::kLine_Verb == pathRef2->atVerb(i)); + { + ed.growForRepeatedVerb(SkPath::kLine_Verb, kRepeatCnt); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countVerbs()); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countPoints()); + REPORTER_ASSERT(reporter, SkPath::kLine_SegmentMask == pathRef->getSegmentMasks()); + for (int i = 0; i < kRepeatCnt; ++i) { + REPORTER_ASSERT(reporter, SkPath::kLine_Verb == pathRef->atVerb(i)); + } + ed.resetToSize(0, 0, 0); } - ed.resetToSize(0, 0, 0); - } - { - ed.growForRepeatedVerb(SkPath::kQuad_Verb, kRepeatCnt); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countVerbs()); - REPORTER_ASSERT(reporter, 2*kRepeatCnt == pathRef2->countPoints()); - REPORTER_ASSERT(reporter, SkPath::kQuad_SegmentMask == pathRef2->getSegmentMasks()); - for (int i = 0; i < kRepeatCnt; ++i) { - REPORTER_ASSERT(reporter, SkPath::kQuad_Verb == pathRef2->atVerb(i)); + { + ed.growForRepeatedVerb(SkPath::kQuad_Verb, kRepeatCnt); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countVerbs()); + REPORTER_ASSERT(reporter, 2*kRepeatCnt == pathRef->countPoints()); + REPORTER_ASSERT(reporter, SkPath::kQuad_SegmentMask == pathRef->getSegmentMasks()); + for (int i = 0; i < kRepeatCnt; ++i) { + REPORTER_ASSERT(reporter, SkPath::kQuad_Verb == pathRef->atVerb(i)); + } + ed.resetToSize(0, 0, 0); } - ed.resetToSize(0, 0, 0); - } - { - SkScalar* weights = NULL; - ed.growForRepeatedVerb(SkPath::kConic_Verb, kRepeatCnt, &weights); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countVerbs()); - REPORTER_ASSERT(reporter, 2*kRepeatCnt == pathRef2->countPoints()); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countWeights()); - REPORTER_ASSERT(reporter, SkPath::kConic_SegmentMask == pathRef2->getSegmentMasks()); - REPORTER_ASSERT(reporter, NULL != weights); - for (int i = 0; i < kRepeatCnt; ++i) { - REPORTER_ASSERT(reporter, SkPath::kConic_Verb == pathRef2->atVerb(i)); + { + SkScalar* weights = NULL; + ed.growForRepeatedVerb(SkPath::kConic_Verb, kRepeatCnt, &weights); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countVerbs()); + REPORTER_ASSERT(reporter, 2*kRepeatCnt == pathRef->countPoints()); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countWeights()); + REPORTER_ASSERT(reporter, SkPath::kConic_SegmentMask == pathRef->getSegmentMasks()); + REPORTER_ASSERT(reporter, NULL != weights); + for (int i = 0; i < kRepeatCnt; ++i) { + REPORTER_ASSERT(reporter, SkPath::kConic_Verb == pathRef->atVerb(i)); + } + ed.resetToSize(0, 0, 0); } - ed.resetToSize(0, 0, 0); - } - { - ed.growForRepeatedVerb(SkPath::kCubic_Verb, kRepeatCnt); - REPORTER_ASSERT(reporter, kRepeatCnt == pathRef2->countVerbs()); - REPORTER_ASSERT(reporter, 3*kRepeatCnt == pathRef2->countPoints()); - REPORTER_ASSERT(reporter, SkPath::kCubic_SegmentMask == pathRef2->getSegmentMasks()); - for (int i = 0; i < kRepeatCnt; ++i) { - REPORTER_ASSERT(reporter, SkPath::kCubic_Verb == pathRef2->atVerb(i)); + { + ed.growForRepeatedVerb(SkPath::kCubic_Verb, kRepeatCnt); + REPORTER_ASSERT(reporter, kRepeatCnt == pathRef->countVerbs()); + REPORTER_ASSERT(reporter, 3*kRepeatCnt == pathRef->countPoints()); + REPORTER_ASSERT(reporter, SkPath::kCubic_SegmentMask == pathRef->getSegmentMasks()); + for (int i = 0; i < kRepeatCnt; ++i) { + REPORTER_ASSERT(reporter, SkPath::kCubic_Verb == pathRef->atVerb(i)); + } + ed.resetToSize(0, 0, 0); } - ed.resetToSize(0, 0, 0); } -} +}; static void test_operatorEqual(skiatest::Reporter* reporter) { SkPath a; @@ -3369,6 +3366,6 @@ DEF_TEST(Path, reporter) { test_conicTo_special_case(reporter); test_get_point(reporter); test_contains(reporter); - test_pathref(reporter); PathTest_Private::TestPathTo(reporter); + PathRefTest_Private::TestPathRef(reporter); }