Fixed a pair of issues affecting GlobalMembers on V8.

ClearScriptLib · ClearScriptLib · commit 07207dd38731 · 2014-11-25T16:53:09.000-05:00
diff --git a/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp b/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp
@@ -697,7 +697,7 @@ void V8ContextImpl::GetGlobalProperty(Local<String> hName, const PropertyCallbac
         {
             for (auto it = stack.rbegin(); it != stack.rend(); it++)
             {
-                if ((*it)->Has(hName))
+                if ((*it)->HasOwnProperty(hName))
                 {
                     CALLBACK_RETURN((*it)->Get(hName));
                 }
@@ -721,8 +721,9 @@ void V8ContextImpl::SetGlobalProperty(Local<String> hName, Local<Value> hValue,
         {
             for (auto it = stack.rbegin(); it != stack.rend(); it++)
             {
-                if ((*it)->HasOwnProperty(hName) && (*it)->Set(hName, hValue))
+                if ((*it)->HasOwnProperty(hName))
                 {
+                    (*it)->Set(hName, hValue);
                     CALLBACK_RETURN(hValue);
                 }
             }
@@ -745,7 +746,7 @@ void V8ContextImpl::QueryGlobalProperty(Local<String> hName, const PropertyCallb
         {
             for (auto it = stack.rbegin(); it != stack.rend(); it++)
             {
-                if ((*it)->Has(hName))
+                if ((*it)->HasOwnProperty(hName))
                 {
                     CALLBACK_RETURN((*it)->GetPropertyAttributes(hName));
                 }
diff --git a/ClearScriptTest/BugFixTest.cs b/ClearScriptTest/BugFixTest.cs
@@ -790,6 +790,23 @@ public void BugFix_Resurrection_V8ScriptItem()
             }
         }
 
+        [TestMethod, TestCategory("BugFix")]
+        public void BugFix_V8_GlobalMembers_ReadOnlyPropertyCrash()
+        {
+            // this test is for a crash that occurred only on debug V8 builds
+            engine.AddHostObject("bag", HostItemFlags.GlobalMembers, new PropertyBag());
+            engine.AddHostObject("test", HostItemFlags.GlobalMembers, new { foo = 123 });
+            TestUtil.AssertException<ScriptEngineException>(() => engine.Execute("foo = 456"));
+        }
+
+        [TestMethod, TestCategory("BugFix")]
+        public void BugFix_V8_GlobalMembers_NativeFunctionHiding()
+        {
+            engine.Execute("function toString() { return 'ABC'; }");
+            engine.AddHostObject("bag", HostItemFlags.GlobalMembers, new PropertyBag());
+            Assert.AreEqual("ABC", engine.Evaluate("toString()"));
+        }
+
         // ReSharper restore InconsistentNaming
 
         #endregion

Original file line number	Diff line number	Diff line change
`@@ -697,7 +697,7 @@ void V8ContextImpl::GetGlobalProperty(Local<String> hName, const PropertyCallbac`
`697`	`697`	`{`
`698`	`698`	`for (auto it = stack.rbegin(); it != stack.rend(); it++)`
`699`	`699`	`{`
`700`		`- if ((*it)->Has(hName))`
	`700`	`+ if ((*it)->HasOwnProperty(hName))`
`701`	`701`	`{`
`702`	`702`	`CALLBACK_RETURN((*it)->Get(hName));`
`703`	`703`	`}`
`@@ -721,8 +721,9 @@ void V8ContextImpl::SetGlobalProperty(Local<String> hName, Local<Value> hValue,`
`721`	`721`	`{`
`722`	`722`	`for (auto it = stack.rbegin(); it != stack.rend(); it++)`
`723`	`723`	`{`
`724`		`- if ((it)->HasOwnProperty(hName) && (it)->Set(hName, hValue))`
	`724`	`+ if ((*it)->HasOwnProperty(hName))`
`725`	`725`	`{`
	`726`	`+ (*it)->Set(hName, hValue);`
`726`	`727`	`CALLBACK_RETURN(hValue);`
`727`	`728`	`}`
`728`	`729`	`}`
`@@ -745,7 +746,7 @@ void V8ContextImpl::QueryGlobalProperty(Local<String> hName, const PropertyCallb`
`745`	`746`	`{`
`746`	`747`	`for (auto it = stack.rbegin(); it != stack.rend(); it++)`
`747`	`748`	`{`
`748`		`- if ((*it)->Has(hName))`
	`749`	`+ if ((*it)->HasOwnProperty(hName))`
`749`	`750`	`{`
`750`	`751`	`CALLBACK_RETURN((*it)->GetPropertyAttributes(hName));`
`751`	`752`	`}`