[dotnet] Fix JavaScriptEngine.ScriptCallbackBindings not containing new bindings (#15221)

* [dotnet] Fix `JavaScriptEngine.ScriptCallbackBindings` not containing new bindings

* Add tests

* Implement IEquatable on `InitializationScript`

* Remove implementation of IEquatable, override normal `Equals`

* Override InitializationScript.GetHashCode

Author: RenderMichael

dotnet/src/webdriver/InitializationScript.cs

@@ -41,6 +41,28 @@ public class InitializationScript
         /// </summary>
         public string ScriptSource { get; internal set; }
 
+        /// <summary>
+        /// Indicates whether the current <see cref="InitializationScript"/> is equal to another <see cref="InitializationScript"/> of the same type.
+        /// </summary>
+        /// <param name="other">An <see cref="InitializationScript"/> to compare with this <see cref="InitializationScript"/>.</param>
+        /// <returns><see langword="true"/> if the current <see cref="InitializationScript"/> is equal to the other parameter; otherwise, <see langword="false"/>.</returns>
+        public override bool Equals(object obj)
+        {
+            return obj is InitializationScript other && this.ScriptId == other.ScriptId && this.ScriptName == other.ScriptName && this.ScriptSource == other.ScriptSource;
+        }
+
+        /// <summary>
+        /// Serves as a hash function for a particular <see cref="InitializationScript"/>.
+        /// </summary>
+        /// <returns>A hash code for the current <see cref="InitializationScript"/>.</returns>
+        public override int GetHashCode()
+        {
+            int result = this.ScriptId.GetHashCode();
+            result = (31 * result) + this.ScriptName.GetHashCode();
+            result = (31 * result) + this.ScriptSource.GetHashCode();
+            return result;
+        }
+
         /// <summary>
         /// Returns a string that represents the current object.
         /// </summary>

dotnet/src/webdriver/JavaScriptEngine.cs

@@ -40,7 +40,7 @@ public class JavaScriptEngine : IJavaScriptEngine
         private Lazy<DevToolsSession> session;
         private Dictionary<string, InitializationScript> initializationScripts = new Dictionary<string, InitializationScript>();
         private Dictionary<string, PinnedScript> pinnedScripts = new Dictionary<string, PinnedScript>();
-        private List<string> bindings = new List<string>();
+        private HashSet<string> bindings = new HashSet<string>();
         private bool isEnabled = false;
         private bool isDisposed = false;
 
@@ -271,7 +271,7 @@ public async Task UnpinScript(PinnedScript script)
         /// <returns>A task that represents the asynchronous operation.</returns>
         public async Task AddScriptCallbackBinding(string bindingName)
         {
-            if (this.bindings.Contains(bindingName))
+            if (!this.bindings.Add(bindingName))
             {
                 throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "A binding named {0} has already been added", bindingName));
             }
@@ -288,7 +288,7 @@ public async Task AddScriptCallbackBinding(string bindingName)
         public async Task RemoveScriptCallbackBinding(string bindingName)
         {
             await this.session.Value.Domains.JavaScript.RemoveBinding(bindingName).ConfigureAwait(false);
-            this.bindings.Remove(bindingName);
+            _ = this.bindings.Remove(bindingName);
         }
 
         /// <summary>

dotnet/test/common/ExecutingJavascriptTest.cs

@@ -480,7 +480,7 @@ public void ShouldBeAbleToExecuteABigChunkOfJavascriptCode()
         [IgnoreBrowser(Selenium.Browser.Safari, "Safari does not support Chrome DevTools Protocol")]
         public async Task ShouldBeAbleToPinJavascriptCodeAndExecuteRepeatedly()
         {
-            IJavaScriptEngine jsEngine = new JavaScriptEngine(driver);
+            using IJavaScriptEngine jsEngine = new JavaScriptEngine(driver);
 
             driver.Url = xhtmlTestPage;
 
@@ -500,6 +500,107 @@ public async Task ShouldBeAbleToPinJavascriptCodeAndExecuteRepeatedly()
                 Throws.TypeOf<JavaScriptException>());
         }
 
+        [Test]
+        [NeedsFreshDriver(IsCreatedAfterTest = true)]
+        [IgnoreBrowser(Selenium.Browser.IE, "IE does not support Chrome DevTools Protocol")]
+        [IgnoreBrowser(Selenium.Browser.Firefox, "Firefox does not support Chrome DevTools Protocol")]
+        [IgnoreBrowser(Selenium.Browser.Safari, "Safari does not support Chrome DevTools Protocol")]
+        public async Task ShouldBeAbleToAddInitializationScriptAndExecuteOnNewDocument()
+        {
+            const string ScriptValue = "alert('notice')";
+            const string ScriptName = "AlertScript";
+
+            using IJavaScriptEngine jsEngine = new JavaScriptEngine(driver);
+
+            var initScript = await jsEngine.AddInitializationScript(ScriptName, ScriptValue);
+
+            Assert.That(initScript, Is.Not.Null);
+            Assert.That(initScript.ScriptSource, Is.EqualTo(ScriptValue));
+            Assert.That(initScript.ScriptName, Is.EqualTo(ScriptName));
+            Assert.That(initScript.ScriptId, Is.Not.Null);
+
+            await jsEngine.StartEventMonitoring();
+
+            driver.Navigate().Refresh();
+            driver.SwitchTo().Alert().Accept();
+
+            Assert.That(jsEngine.InitializationScripts, Does.Contain(initScript));
+            await jsEngine.RemoveInitializationScript(ScriptName);
+
+            driver.Navigate().Refresh();
+            Assert.That(() => driver.SwitchTo().Alert().Accept(), Throws.TypeOf<NoAlertPresentException>());
+
+            Assert.That(jsEngine.InitializationScripts, Does.Not.Contain(initScript));
+
+            await jsEngine.AddInitializationScript(ScriptName, ScriptValue);
+
+            driver.Navigate().Refresh();
+            driver.SwitchTo().Alert().Accept();
+            Assert.That(jsEngine.InitializationScripts, Does.Contain(initScript));
+
+            await jsEngine.ClearInitializationScripts();
+
+            driver.Navigate().Refresh();
+            Assert.That(() => driver.SwitchTo().Alert().Accept(), Throws.TypeOf<NoAlertPresentException>());
+            Assert.That(jsEngine.InitializationScripts, Is.Empty);
+
+            await jsEngine.AddInitializationScript(ScriptName, ScriptValue);
+            driver.Navigate().Refresh();
+            driver.SwitchTo().Alert().Accept();
+
+            await jsEngine.ClearAll();
+            driver.Navigate().Refresh();
+            Assert.That(() => driver.SwitchTo().Alert().Accept(), Throws.TypeOf<NoAlertPresentException>());
+            Assert.That(jsEngine.InitializationScripts, Is.Empty);
+        }
+
+        [Test]
+        [NeedsFreshDriver(IsCreatedAfterTest = true)]
+        [IgnoreBrowser(Selenium.Browser.IE, "IE does not support Chrome DevTools Protocol")]
+        [IgnoreBrowser(Selenium.Browser.Firefox, "Firefox does not support Chrome DevTools Protocol")]
+        [IgnoreBrowser(Selenium.Browser.Safari, "Safari does not support Chrome DevTools Protocol")]
+        public async Task ShouldBeAbleToAddAndRemoveScriptCallbackBinding()
+        {
+            const string ScriptValue = "alert('Hello world')";
+            const string ScriptName = "alert";
+
+            using IJavaScriptEngine jsEngine = new JavaScriptEngine(driver);
+
+            var executedBindings = new List<string>();
+            jsEngine.JavaScriptCallbackExecuted += AddToList;
+            await jsEngine.AddInitializationScript(ScriptName, ScriptValue);
+            await jsEngine.StartEventMonitoring();
+
+            driver.Navigate().Refresh();
+            driver.SwitchTo().Alert().Accept();
+
+            await jsEngine.AddScriptCallbackBinding(ScriptName);
+
+            driver.Navigate().Refresh();
+            Assert.That(() => driver.SwitchTo().Alert().Accept(), Throws.TypeOf<NoAlertPresentException>());
+
+            Assert.That(executedBindings, Does.Contain(ScriptName));
+            int oldCount = executedBindings.Count;
+            driver.Navigate().Refresh();
+
+            Assert.That(executedBindings, Has.Count.GreaterThan(oldCount));
+            Assert.That(jsEngine.ScriptCallbackBindings, Does.Contain(ScriptName));
+            oldCount = executedBindings.Count;
+
+            await jsEngine.RemoveScriptCallbackBinding(ScriptName);
+            Assert.That(jsEngine.ScriptCallbackBindings, Is.Empty);
+            await jsEngine.AddScriptCallbackBinding(ScriptName);
+            Assert.That(jsEngine.ScriptCallbackBindings, Does.Contain(ScriptName));
+            await jsEngine.ClearScriptCallbackBindings();
+            Assert.That(jsEngine.ScriptCallbackBindings, Is.Empty);
+
+            jsEngine.JavaScriptCallbackExecuted -= AddToList;
+            driver.Navigate().Refresh();
+            Assert.That(executedBindings, Has.Count.EqualTo(oldCount));
+
+            void AddToList(object sender, JavaScriptCallbackExecutedEventArgs e) => executedBindings.Add(e.BindingName);
+        }
+
         [Test]
         public void ShouldBeAbleToExecuteScriptAndReturnElementsList()
         {