From: ager@chromium.org Date: Tue, 5 May 2009 10:15:05 +0000 (+0000) Subject: Revert workaround for http://crbug.com/9746. X-Git-Tag: upstream/4.7.83~24191 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5a4e24fe0f63726b53f711cb8ea72f867c049409;p=platform%2Fupstream%2Fv8.git Revert workaround for http://crbug.com/9746. Review URL: http://codereview.chromium.org/109015 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1860 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/objects.cc b/src/objects.cc index 80977c1..5f09833 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3305,13 +3305,6 @@ Vector String::ToUC16Vector() { } ASSERT(string_tag == kExternalStringTag); ExternalTwoByteString* ext = ExternalTwoByteString::cast(string); - // This is a workaround for Chromium bug 9746: http://crbug.com/9746 - // For external strings with a deleted resource we return a special - // Vector which will not compare to any string when doing SymbolTable - // lookups. - if (ext->resource() == NULL) { - return Vector(NULL, length); - } const uc16* start = reinterpret_cast(ext->resource()->data()); return Vector(start + offset, length); @@ -4128,18 +4121,6 @@ static inline bool CompareRawStringContents(Vector a, Vector b) { } -// This is a workaround for Chromium bug 9746: http://crbug.com/9746 -// Returns true if this Vector matches the problem exposed in the bug. -template -static bool CheckVectorForBug9746(Vector vec) { - // The problem is that somehow external string entries in the symbol - // table can have their resources collected while they are still in the - // table. This should not happen according to the test in the function - // DisposeExternalString in api.cc, but we have evidence that it does. - return (vec.start() == NULL) ? true : false; -} - - static StringInputBuffer string_compare_buffer_b; @@ -4150,9 +4131,7 @@ static inline bool CompareStringContentsPartial(IteratorA* ia, String* b) { VectorIterator ib(b->ToAsciiVector()); return CompareStringContents(ia, &ib); } else { - Vector vb = b->ToUC16Vector(); - if (CheckVectorForBug9746(vb)) return false; - VectorIterator ib(vb); + VectorIterator ib(b->ToUC16Vector()); return CompareStringContents(ia, &ib); } } else { @@ -4194,9 +4173,7 @@ bool String::SlowEquals(String* other) { return CompareRawStringContents(vec1, vec2); } else { VectorIterator buf1(vec1); - Vector vec2 = other->ToUC16Vector(); - if (CheckVectorForBug9746(vec2)) return false; - VectorIterator ib(vec2); + VectorIterator ib(other->ToUC16Vector()); return CompareStringContents(&buf1, &ib); } } else { @@ -4206,15 +4183,13 @@ bool String::SlowEquals(String* other) { } } else { Vector vec1 = this->ToUC16Vector(); - if (CheckVectorForBug9746(vec1)) return false; if (other->IsFlat()) { if (other->IsAsciiRepresentation()) { VectorIterator buf1(vec1); VectorIterator ib(other->ToAsciiVector()); return CompareStringContents(&buf1, &ib); } else { - Vector vec2 = other->ToUC16Vector(); - if (CheckVectorForBug9746(vec2)) return false; + Vector vec2(other->ToUC16Vector()); return CompareRawStringContents(vec1, vec2); } } else { @@ -4259,18 +4234,6 @@ bool String::MarkAsUndetectable() { bool String::IsEqualTo(Vector str) { - // This is a workaround for Chromium bug 9746: http://crbug.com/9746 - // The problem is that somehow external string entries in the symbol - // table can have their resources deleted while they are still in the - // table. This should not happen according to the test in the function - // DisposeExternalString in api.cc but we have evidence that it does. - // Thus we add this bailout here. - StringShape shape(this); - if (shape.IsExternalTwoByte()) { - ExternalTwoByteString* ext = ExternalTwoByteString::cast(this); - if (ext->resource() == NULL) return false; - } - int slen = length(); Access decoder(Scanner::utf8_decoder()); decoder->Reset(str.start(), str.length()); diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 0b498ed..288efba 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -4656,7 +4656,7 @@ TEST(ContextData) { CHECK(context_2->GetData()->StrictEquals(data_2)); // Simple test function which causes a break. - char* source = "function f() { debugger; }"; + const char* source = "function f() { debugger; }"; // Enter and run function in the first context. { diff --git a/test/cctest/test-strings.cc b/test/cctest/test-strings.cc index 6f42639..3dae363 100644 --- a/test/cctest/test-strings.cc +++ b/test/cctest/test-strings.cc @@ -394,16 +394,14 @@ class TwoByteResource: public v8::String::ExternalStringResource { public: TwoByteResource(const uint16_t* data, size_t length, bool* destructed) : data_(data), length_(length), destructed_(destructed) { - if (destructed_ != NULL) { - *destructed_ = false; - } + CHECK_NE(destructed, NULL); + *destructed_ = false; } virtual ~TwoByteResource() { - if (destructed_ != NULL) { - CHECK(!*destructed_); - *destructed_ = true; - } + CHECK_NE(destructed_, NULL); + CHECK(!*destructed_); + *destructed_ = true; } const uint16_t* data() const { return data_; } @@ -416,49 +414,6 @@ class TwoByteResource: public v8::String::ExternalStringResource { }; -TEST(ExternalCrBug9746) { - InitializeVM(); - v8::HandleScope handle_scope; - - // This set of tests verifies that the workaround for Chromium bug 9746 - // works correctly. In certain situations the external resource of a symbol - // is collected while the symbol is still part of the symbol table. - static uint16_t two_byte_data[] = { - 't', 'w', 'o', '-', 'b', 'y', 't', 'e', ' ', 'd', 'a', 't', 'a' - }; - static size_t two_byte_length = - sizeof(two_byte_data) / sizeof(two_byte_data[0]); - static const char* one_byte_data = "two-byte data"; - - // Allocate an external string resource and external string. - TwoByteResource* resource = new TwoByteResource(two_byte_data, - two_byte_length, - NULL); - Handle string = Factory::NewExternalStringFromTwoByte(resource); - Vector one_byte_vec = CStrVector(one_byte_data); - Handle compare = Factory::NewStringFromAscii(one_byte_vec); - - // Verify the correct behaviour before "collecting" the external resource. - CHECK(string->IsEqualTo(one_byte_vec)); - CHECK(string->Equals(*compare)); - - // "Collect" the external resource manually by setting the external resource - // pointer to NULL. Then redo the comparisons, they should not match AND - // not crash. - Handle external(ExternalTwoByteString::cast(*string)); - external->set_resource(NULL); - CHECK_EQ(false, string->IsEqualTo(one_byte_vec)); -#if !defined(DEBUG) - // These tests only work in non-debug as there are ASSERTs in the code that - // do prevent the ability to even get into the broken code when running the - // debug version of V8. - CHECK_EQ(false, string->Equals(*compare)); - CHECK_EQ(false, compare->Equals(*string)); - CHECK_EQ(false, string->Equals(Heap::empty_string())); -#endif // !defined(DEBUG) -} - - // Regression test case for http://crbug.com/9746. The problem was // that when we marked objects reachable only through weak pointers, // we ended up keeping a sliced symbol alive, even though we already @@ -514,7 +469,7 @@ TEST(Regress9746) { CHECK(buffer->IsTwoByteRepresentation()); // Finally, base a script on the slice of the external string and - // get its wrapper. This allocated yet another weak handle that + // get its wrapper. This allocates yet another weak handle that // indirectly refers to the external string. Handle