Revert r7258 and r7260.

They apparently break Threading tests on at least Mac and Win64.

TBR=vitalyr@chromium.org

Review URL: http://codereview.chromium.org/6709028

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7262 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/api.cc

@@ -2391,9 +2391,6 @@ bool v8::Object::SetPrototype(Handle<Value> value) {
   ENTER_V8;
   i::Handle<i::JSObject> self = Utils::OpenHandle(this);
   i::Handle<i::Object> value_obj = Utils::OpenHandle(*value);
-  // We do not allow exceptions thrown while setting the prototype
-  // propagate outside.
-  TryCatch try_catch;
   EXCEPTION_PREAMBLE();
   i::Handle<i::Object> result = i::SetPrototype(self, value_obj);
   has_pending_exception = result.is_null();
@@ -2580,25 +2577,6 @@ bool v8::Object::HasIndexedLookupInterceptor() {
 }
 
 
-static Local<Value> GetPropertyByLookup(i::Handle<i::JSObject> receiver,
-                                        i::Handle<i::String> name,
-                                        i::LookupResult* lookup) {
-  if (!lookup->IsProperty()) {
-    // No real property was found.
-    return Local<Value>();
-  }
-
-  // If the property being looked up is a callback, it can throw
-  // an exception.
-  EXCEPTION_PREAMBLE();
-  i::Handle<i::Object> result = i::GetProperty(receiver, name, lookup);
-  has_pending_exception = result.is_null();
-  EXCEPTION_BAILOUT_CHECK(Local<Value>());
-
-  return Utils::ToLocal(result);
-}
-
-
 Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
       Handle<String> key) {
   ON_BAILOUT("v8::Object::GetRealNamedPropertyInPrototypeChain()",
@@ -2608,7 +2586,17 @@ Local<Value> v8::Object::GetRealNamedPropertyInPrototypeChain(
   i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
   i::LookupResult lookup;
   self_obj->LookupRealNamedPropertyInPrototypes(*key_obj, &lookup);
-  return GetPropertyByLookup(self_obj, key_obj, &lookup);
+  if (lookup.IsProperty()) {
+    PropertyAttributes attributes;
+    i::Object* property =
+        self_obj->GetProperty(*self_obj,
+                              &lookup,
+                              *key_obj,
+                              &attributes)->ToObjectUnchecked();
+    i::Handle<i::Object> result(property);
+    return Utils::ToLocal(result);
+  }
+  return Local<Value>();  // No real property was found in prototype chain.
 }
 
 
@@ -2619,7 +2607,17 @@ Local<Value> v8::Object::GetRealNamedProperty(Handle<String> key) {
   i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
   i::LookupResult lookup;
   self_obj->LookupRealNamedProperty(*key_obj, &lookup);
-  return GetPropertyByLookup(self_obj, key_obj, &lookup);
+  if (lookup.IsProperty()) {
+    PropertyAttributes attributes;
+    i::Object* property =
+        self_obj->GetProperty(*self_obj,
+                              &lookup,
+                              *key_obj,
+                              &attributes)->ToObjectUnchecked();
+    i::Handle<i::Object> result(property);
+    return Utils::ToLocal(result);
+  }
+  return Local<Value>();  // No real property was found in prototype chain.
 }
 
 

src/handles.cc

@@ -333,15 +333,6 @@ Handle<Object> GetProperty(Handle<Object> obj,
 }
 
 
-Handle<Object> GetProperty(Handle<JSObject> obj,
-                           Handle<String> name,
-                           LookupResult* result) {
-  PropertyAttributes attributes;
-  CALL_HEAP_FUNCTION(obj->GetProperty(*obj, result, *name, &attributes),
-                     Object);
-}
-
-
 Handle<Object> GetElement(Handle<Object> obj,
                           uint32_t index) {
   CALL_HEAP_FUNCTION(Runtime::GetElement(obj, index), Object);

src/handles.h

@@ -280,11 +280,6 @@ Handle<Object> GetProperty(Handle<JSObject> obj,
 Handle<Object> GetProperty(Handle<Object> obj,
                            Handle<Object> key);
 
-Handle<Object> GetProperty(Handle<JSObject> obj,
-                           Handle<String> name,
-                           LookupResult* result);
-
-
 Handle<Object> GetElement(Handle<Object> obj,
                           uint32_t index);
 

src/messages.cc

@@ -109,23 +109,12 @@ Handle<JSMessageObject> MessageHandler::MakeMessageObject(
 
 void MessageHandler::ReportMessage(MessageLocation* loc,
                                    Handle<Object> message) {
-  // We are calling into embedder's code which can throw exceptions.
-  // Thus we need to save current exception state, reset it to the clean one
-  // and ignore scheduled exceptions callbacks can throw.
-  Top::ExceptionScope exception_scope;
-  Top::clear_pending_exception();
-  Top::set_external_caught_exception(false);
-
   v8::Local<v8::Message> api_message_obj = v8::Utils::MessageToLocal(message);
 
   v8::NeanderArray global_listeners(Factory::message_listeners());
   int global_length = global_listeners.length();
   if (global_length == 0) {
     DefaultMessageReport(loc, message);
-    if (Top::has_scheduled_exception()) {
-      // Consider logging it somehow.
-      Top::clear_scheduled_exception();
-    }
   } else {
     for (int i = 0; i < global_length; i++) {
       HandleScope scope;
@@ -136,10 +125,6 @@ void MessageHandler::ReportMessage(MessageLocation* loc,
           FUNCTION_CAST<v8::MessageCallback>(callback_obj->proxy());
       Handle<Object> callback_data(listener.get(1));
       callback(api_message_obj, v8::Utils::ToLocal(callback_data));
-      if (Top::has_scheduled_exception()) {
-        // Consider logging it somehow.
-        Top::clear_scheduled_exception();
-      }
     }
   }
 }

src/top.cc

@@ -971,55 +971,41 @@ bool Top::IsExternallyCaught() {
 }
 
 
-void Top::PropagatePendingExceptionToExternalTryCatch() {
-  ASSERT(has_pending_exception());
-
-  bool external_caught = IsExternallyCaught();
-  thread_local_.external_caught_exception_ = external_caught;
-
-  if (!external_caught) return;
-
-  if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
-    // Do not propagate OOM exception: we should kill VM asap.
-  } else if (thread_local_.pending_exception_ ==
-             Heap::termination_exception()) {
-    try_catch_handler()->can_continue_ = false;
-    try_catch_handler()->exception_ = Heap::null_value();
-  } else {
-    // At this point all non-object (failure) exceptions have
-    // been dealt with so this shouldn't fail.
-    ASSERT(!pending_exception()->IsFailure());
-    try_catch_handler()->can_continue_ = true;
-    try_catch_handler()->exception_ = pending_exception();
-    if (!thread_local_.pending_message_obj_->IsTheHole()) {
-      try_catch_handler()->message_ = thread_local_.pending_message_obj_;
-    }
-  }
-}
-
-
 void Top::ReportPendingMessages() {
   ASSERT(has_pending_exception());
-  PropagatePendingExceptionToExternalTryCatch();
-
   // If the pending exception is OutOfMemoryException set out_of_memory in
   // the global context.  Note: We have to mark the global context here
   // since the GenerateThrowOutOfMemory stub cannot make a RuntimeCall to
   // set it.
+  bool external_caught = IsExternallyCaught();
+  thread_local_.external_caught_exception_ = external_caught;
   HandleScope scope;
   if (thread_local_.pending_exception_ == Failure::OutOfMemoryException()) {
     context()->mark_out_of_memory();
   } else if (thread_local_.pending_exception_ ==
              Heap::termination_exception()) {
-    // Do nothing: if needed, the exception has been already propagated to
-    // v8::TryCatch.
+    if (external_caught) {
+      try_catch_handler()->can_continue_ = false;
+      try_catch_handler()->exception_ = Heap::null_value();
+    }
   } else {
+    // At this point all non-object (failure) exceptions have
+    // been dealt with so this shouldn't fail.
+    Object* pending_exception_object = pending_exception()->ToObjectUnchecked();
+    Handle<Object> exception(pending_exception_object);
+    thread_local_.external_caught_exception_ = false;
+    if (external_caught) {
+      try_catch_handler()->can_continue_ = true;
+      try_catch_handler()->exception_ = thread_local_.pending_exception_;
+      if (!thread_local_.pending_message_obj_->IsTheHole()) {
+        try_catch_handler()->message_ = thread_local_.pending_message_obj_;
+      }
+    }
     if (thread_local_.has_pending_message_) {
       thread_local_.has_pending_message_ = false;
       if (thread_local_.pending_message_ != NULL) {
         MessageHandler::ReportMessage(thread_local_.pending_message_);
       } else if (!thread_local_.pending_message_obj_->IsTheHole()) {
-        HandleScope scope;
         Handle<Object> message_obj(thread_local_.pending_message_obj_);
         if (thread_local_.pending_message_script_ != NULL) {
           Handle<Script> script(thread_local_.pending_message_script_);
@@ -1032,6 +1018,8 @@ void Top::ReportPendingMessages() {
         }
       }
     }
+    thread_local_.external_caught_exception_ = external_caught;
+    set_pending_exception(*exception);
   }
   clear_pending_message();
 }
@@ -1043,9 +1031,6 @@ void Top::TraceException(bool flag) {
 
 
 bool Top::OptionalRescheduleException(bool is_bottom_call) {
-  ASSERT(has_pending_exception());
-  PropagatePendingExceptionToExternalTryCatch();
-
   // Allways reschedule out of memory exceptions.
   if (!is_out_of_memory()) {
     bool is_termination_exception =

src/top.h

@@ -195,26 +195,22 @@ class Top {
     ASSERT(has_pending_exception());
     return thread_local_.pending_exception_;
   }
+  static bool external_caught_exception() {
+    return thread_local_.external_caught_exception_;
+  }
   static void set_pending_exception(MaybeObject* exception) {
     thread_local_.pending_exception_ = exception;
   }
   static void clear_pending_exception() {
     thread_local_.pending_exception_ = Heap::the_hole_value();
   }
+
   static MaybeObject** pending_exception_address() {
     return &thread_local_.pending_exception_;
   }
   static bool has_pending_exception() {
     return !thread_local_.pending_exception_->IsTheHole();
   }
-
-  static bool external_caught_exception() {
-    return thread_local_.external_caught_exception_;
-  }
-  static void set_external_caught_exception(bool value) {
-    thread_local_.external_caught_exception_ = value;
-  }
-
   static void clear_pending_message() {
     thread_local_.has_pending_message_ = false;
     thread_local_.pending_message_ = NULL;
@@ -465,25 +461,6 @@ class Top {
   static const char* kStackOverflowMessage;
 
  private:
-
-  static v8::TryCatch* catcher() {
-    return thread_local_.catcher_;
-  }
-
-  static void set_catcher(v8::TryCatch* catcher) {
-    thread_local_.catcher_ = catcher;
-  }
-
-  static void setup_external_caught() {
-    thread_local_.external_caught_exception_ =
-        has_pending_exception() &&
-        (thread_local_.catcher_ != NULL) &&
-        (try_catch_handler() == thread_local_.catcher_);
-  }
-
-  // Attempts to propagate the pending exception to the proper v8::TryCatch.
-  static void PropagatePendingExceptionToExternalTryCatch();
-
 #ifdef ENABLE_VMSTATE_TRACKING
   // Set of states used when communicating with the runtime profiler.
   //
@@ -548,29 +525,6 @@ class Top {
   friend class ThreadLocalTop;
 
   static void FillCache();
-
- public:
-  class ExceptionScope {
-   public:
-    ExceptionScope() :
-      // Scope currently can only be used for regular exceptions, not
-      // failures like OOM or termination exception.
-      pending_exception_(Top::pending_exception()->ToObjectUnchecked()),
-      external_caught_exception_(Top::external_caught_exception()),
-      catcher_(Top::catcher())
-    { }
-
-    ~ExceptionScope() {
-      Top::set_catcher(catcher_);
-      Top::set_external_caught_exception(external_caught_exception_);
-      Top::set_pending_exception(*pending_exception_);
-    }
-
-   private:
-    Handle<Object> pending_exception_;
-    bool external_caught_exception_;
-    v8::TryCatch* catcher_;
-  };
 };
 
 

test/cctest/test-api.cc

@@ -51,26 +51,18 @@ static bool IsNaN(double x) {
 }
 
 using ::v8::AccessorInfo;
-using ::v8::Arguments;
 using ::v8::Context;
 using ::v8::Extension;
 using ::v8::Function;
-using ::v8::FunctionTemplate;
-using ::v8::Handle;
 using ::v8::HandleScope;
 using ::v8::Local;
-using ::v8::Message;
-using ::v8::MessageCallback;
 using ::v8::Object;
 using ::v8::ObjectTemplate;
 using ::v8::Persistent;
 using ::v8::Script;
-using ::v8::StackTrace;
 using ::v8::String;
-using ::v8::TryCatch;
-using ::v8::Undefined;
-using ::v8::V8;
 using ::v8::Value;
+using ::v8::V8;
 
 namespace i = ::i;
 
@@ -8584,128 +8576,6 @@ THREADED_TEST(NamedPropertyHandlerGetterAttributes) {
 }
 
 
-static Handle<Value> ThrowingGetter(Local<String> name,
-                                    const AccessorInfo& info) {
-  ApiTestFuzzer::Fuzz();
-  ThrowException(Handle<Value>());
-  return Undefined();
-}
-
-
-THREADED_TEST(VariousGetPropertiesAndThrowingCallbacks) {
-  HandleScope scope;
-  LocalContext context;
-
-  Local<FunctionTemplate> templ = FunctionTemplate::New();
-  Local<ObjectTemplate> instance_templ = templ->InstanceTemplate();
-  instance_templ->SetAccessor(v8_str("f"), ThrowingGetter);
-
-  Local<Object> instance = templ->GetFunction()->NewInstance();
-
-  Local<Object> another = Object::New();
-  another->SetPrototype(instance);
-
-  Local<Object> with_js_getter = CompileRun(
-      "o = {};\n"
-      "o.__defineGetter__('f', function() { throw undefined; });\n"
-      "o\n").As<Object>();
-  CHECK(!with_js_getter.IsEmpty());
-
-  TryCatch try_catch;
-
-  Local<Value> result = instance->GetRealNamedProperty(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-
-  result = another->GetRealNamedProperty(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-
-  result = another->GetRealNamedPropertyInPrototypeChain(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-
-  result = another->Get(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-
-  result = with_js_getter->GetRealNamedProperty(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-
-  result = with_js_getter->Get(v8_str("f"));
-  CHECK(try_catch.HasCaught());
-  try_catch.Reset();
-  CHECK(result.IsEmpty());
-}
-
-
-static Handle<Value> ThrowingCallbackWithTryCatch(const Arguments& args) {
-  TryCatch try_catch;
-  // Verboseness is important: it triggers message delivery which can call into
-  // external code.
-  try_catch.SetVerbose(true);
-  CompileRun("throw 'from JS';");
-  CHECK(try_catch.HasCaught());
-  CHECK(!i::Top::has_pending_exception());
-  CHECK(!i::Top::has_scheduled_exception());
-  return Undefined();
-}
-
-
-static void WithTryCatch(Handle<Message> message, Handle<Value> data) {
-  TryCatch try_catch;
-}
-
-
-static void ThrowFromJS(Handle<Message> message, Handle<Value> data) {
-  CompileRun("throw 'ThrowInJS';");
-}
-
-
-static void ThrowViaApi(Handle<Message> message, Handle<Value> data) {
-  ThrowException(v8_str("ThrowViaApi"));
-}
-
-
-static void WebKitLike(Handle<Message> message, Handle<Value> data) {
-  Handle<String> errorMessageString = message->Get();
-  CHECK(!errorMessageString.IsEmpty());
-  message->GetStackTrace();
-  message->GetScriptResourceName();
-}
-
-THREADED_TEST(ExceptionsDoNotPropagatePastTryCatch) {
-  HandleScope scope;
-  LocalContext context;
-
-  Local<Function> func =
-      FunctionTemplate::New(ThrowingCallbackWithTryCatch)->GetFunction();
-  context->Global()->Set(v8_str("func"), func);
-
-  MessageCallback callbacks[] =
-      { NULL, WebKitLike, ThrowViaApi, ThrowFromJS, WithTryCatch };
-  for (unsigned i = 0; i < sizeof(callbacks)/sizeof(callbacks[0]); i++) {
-    MessageCallback callback = callbacks[i];
-    if (callback != NULL) {
-      V8::AddMessageListener(callback);
-    }
-    ExpectFalse(
-        "var thrown = false;\n"
-        "try { func(); } catch(e) { thrown = true; }\n"
-        "thrown\n");
-    if (callback != NULL) {
-      V8::RemoveMessageListeners(callback);
-    }
-  }
-}
-
-
 static v8::Handle<Value> ParentGetter(Local<String> name,
                                       const AccessorInfo& info) {
   ApiTestFuzzer::Fuzz();