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

src/objects.cc

@@ -3305,13 +3305,6 @@ Vector<const uc16> 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<const uc16>(NULL, length);
-  }
   const uc16* start =
       reinterpret_cast<const uc16*>(ext->resource()->data());
   return Vector<const uc16>(start + offset, length);
@@ -4128,18 +4121,6 @@ static inline bool CompareRawStringContents(Vector<Char> a, Vector<Char> 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 <typename T>
-static bool CheckVectorForBug9746(Vector<T> 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<char> ib(b->ToAsciiVector());
       return CompareStringContents(ia, &ib);
     } else {
-      Vector<const uc16> vb = b->ToUC16Vector();
-      if (CheckVectorForBug9746(vb)) return false;
-      VectorIterator<uc16> ib(vb);
+      VectorIterator<uc16> ib(b->ToUC16Vector());
       return CompareStringContents(ia, &ib);
     }
   } else {
@@ -4194,9 +4173,7 @@ bool String::SlowEquals(String* other) {
           return CompareRawStringContents(vec1, vec2);
         } else {
           VectorIterator<char> buf1(vec1);
-          Vector<const uc16> vec2 = other->ToUC16Vector();
-          if (CheckVectorForBug9746(vec2)) return false;
-          VectorIterator<uc16> ib(vec2);
+          VectorIterator<uc16> ib(other->ToUC16Vector());
           return CompareStringContents(&buf1, &ib);
         }
       } else {
@@ -4206,15 +4183,13 @@ bool String::SlowEquals(String* other) {
       }
     } else {
       Vector<const uc16> vec1 = this->ToUC16Vector();
-      if (CheckVectorForBug9746(vec1)) return false;
       if (other->IsFlat()) {
         if (other->IsAsciiRepresentation()) {
           VectorIterator<uc16> buf1(vec1);
           VectorIterator<char> ib(other->ToAsciiVector());
           return CompareStringContents(&buf1, &ib);
         } else {
-          Vector<const uc16> vec2 = other->ToUC16Vector();
-          if (CheckVectorForBug9746(vec2)) return false;
+          Vector<const uc16> vec2(other->ToUC16Vector());
           return CompareRawStringContents(vec1, vec2);
         }
       } else {
@@ -4259,18 +4234,6 @@ bool String::MarkAsUndetectable() {
 
 
 bool String::IsEqualTo(Vector<const char> 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<Scanner::Utf8Decoder> decoder(Scanner::utf8_decoder());
   decoder->Reset(str.start(), str.length());

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.
   {

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> string = Factory::NewExternalStringFromTwoByte(resource);
-  Vector<const char> one_byte_vec = CStrVector(one_byte_data);
-  Handle<String> 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<ExternalTwoByteString> 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<Script> script = Factory::NewScript(slice);
     Handle<JSObject> wrapper = GetScriptWrapper(script);