Refactoring creation of RemoteWebElement to use a factory

Previously, the RemoteWebDriver class had a protected method called
CreateElement that took the element ID as a parameter and returned a
RemoteWebElement object. That approach is not flexible enough for some
downstream projects that implement their own subclasses of RemoteWebDriver
and RemoteWebElement. To rectify that, this commit replaces the calling of
CreateElement with the use of a RemoteWebElementFactory class that handles
the creation of RemoteWebElements. Note carefully that the CreateElement
method still exists, so as to prevent compile-time errors for downstream
consumers, that method is no longer called. If you are a user who has
overridden that method as part of your project, you'll need to create an
implementation of RemoteWebElementFactory that creates your custom
subclass of RemoteWebElement.

Author: jimevans

dotnet/src/webdriver/Firefox/FirefoxDriver.cs

@@ -252,16 +252,6 @@ protected virtual void PrepareEnvironment()
             // Does nothing, but provides a hook for subclasses to do "stuff"
         }
 
-        /// <summary>
-        /// Creates a <see cref="RemoteWebElement"/> with the specified ID.
-        /// </summary>
-        /// <param name="elementId">The ID of this element.</param>
-        /// <returns>A <see cref="RemoteWebElement"/> with the specified ID. For the FirefoxDriver this will be a <see cref="FirefoxWebElement"/>.</returns>
-        protected override RemoteWebElement CreateElement(string elementId)
-        {
-            return new FirefoxWebElement(this, elementId);
-        }
-
         private static ICommandExecutor CreateExecutor(FirefoxDriverService service, FirefoxOptions options, TimeSpan commandTimeout)
         {
             ICommandExecutor executor = null;

dotnet/src/webdriver/Remote/RemoteWebDriver.cs

@@ -1,4 +1,4 @@
-// <copyright file="RemoteWebDriver.cs" company="WebDriver Committers">
+// <copyright file="RemoteWebDriver.cs" company="WebDriver Committers">
 // Licensed to the Software Freedom Conservancy (SFC) under one
 // or more contributor license agreements. See the NOTICE file
 // distributed with this work for additional information
@@ -79,6 +79,7 @@ public class RemoteWebDriver : IWebDriver, ISearchContext, IJavaScriptExecutor,
         private IApplicationCache appCache;
         private ILocationContext locationContext;
         private IFileDetector fileDetector = new DefaultFileDetector();
+        private RemoteWebElementFactory elementFactory;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="RemoteWebDriver"/> class. This constructor defaults proxy to http://127.0.0.1:4444/wd/hub
@@ -141,6 +142,7 @@ public RemoteWebDriver(ICommandExecutor commandExecutor, ICapabilities desiredCa
             this.StartSession(desiredCapabilities);
             this.mouse = new RemoteMouse(this);
             this.keyboard = new RemoteKeyboard(this);
+            this.elementFactory = new RemoteWebElementFactory(this);
 
             if (this.capabilities.HasCapability(CapabilityType.SupportsApplicationCache))
             {
@@ -436,6 +438,16 @@ protected ICommandExecutor CommandExecutor
             get { return this.executor; }
         }
 
+        /// <summary>
+        /// Gets or sets the factory object used to create instances of <see cref="RemoteWebElement"/>
+        /// or its subclasses.
+        /// </summary>
+        protected RemoteWebElementFactory ElementFactory
+        {
+            get { return this.elementFactory; }
+            set { this.elementFactory = value; }
+        }
+
         /// <summary>
         /// Finds the first element in the page that matches the <see cref="By"/> object
         /// </summary>
@@ -1015,19 +1027,7 @@ internal IWebElement GetElementFromResponse(Response response)
             Dictionary<string, object> elementDictionary = response.Value as Dictionary<string, object>;
             if (elementDictionary != null)
             {
-                // TODO: Remove this "if" logic once the spec is properly updated
-                // and remote-end implementations comply.
-                string id = string.Empty;
-                if (elementDictionary.ContainsKey(RemoteWebElement.ElementReferencePropertyName))
-                {
-                    id = (string)elementDictionary[RemoteWebElement.ElementReferencePropertyName];
-                }
-                else if (elementDictionary.ContainsKey(RemoteWebElement.LegacyElementReferencePropertyName))
-                {
-                    id = (string)elementDictionary[RemoteWebElement.LegacyElementReferencePropertyName];
-                }
-
-                element = this.CreateElement(id);
+                element = this.elementFactory.CreateElement(elementDictionary);
             }
 
             return element;
@@ -1049,19 +1049,7 @@ internal ReadOnlyCollection<IWebElement> GetElementsFromResponse(Response respon
                     Dictionary<string, object> elementDictionary = elementObject as Dictionary<string, object>;
                     if (elementDictionary != null)
                     {
-                        // TODO: Remove this "if" logic once the spec is properly updated
-                        // and remote-end implementations comply.
-                        string id = string.Empty;
-                        if (elementDictionary.ContainsKey(RemoteWebElement.ElementReferencePropertyName))
-                        {
-                            id = (string)elementDictionary[RemoteWebElement.ElementReferencePropertyName];
-                        }
-                        else if (elementDictionary.ContainsKey(RemoteWebElement.LegacyElementReferencePropertyName))
-                        {
-                            id = (string)elementDictionary[RemoteWebElement.LegacyElementReferencePropertyName];
-                        }
-
-                        RemoteWebElement element = this.CreateElement(id);
+                        RemoteWebElement element = this.elementFactory.CreateElement(elementDictionary);
                         toReturn.Add(element);
                     }
                 }
@@ -1242,6 +1230,7 @@ protected ReadOnlyCollection<IWebElement> FindElements(string mechanism, string
         /// </summary>
         /// <param name="elementId">The ID of this element.</param>
         /// <returns>A <see cref="RemoteWebElement"/> with the specified ID.</returns>
+        [Obsolete("This method is no longer called to create RemoteWebElement instances. Implement a subclass of RemoteWebElementFactory and set the ElementFactory property to create instances of custom RemoteWebElement subclasses.")]
         protected virtual RemoteWebElement CreateElement(string elementId)
         {
             RemoteWebElement toReturn = new RemoteWebElement(this, elementId);

dotnet/src/webdriver/Remote/RemoteWebElementFactory.cs

@@ -0,0 +1,102 @@
+// <copyright file="RemoteWebElementFactory.cs" company="WebDriver Committers">
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// </copyright>
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace OpenQA.Selenium.Remote
+{
+    public class RemoteWebElementFactory
+    {
+        private RemoteWebDriver driver;
+
+        public RemoteWebElementFactory(RemoteWebDriver parentDriver)
+        {
+            this.driver = parentDriver;
+        }
+
+        protected RemoteWebDriver ParentDriver
+        {
+            get { return this.driver; }
+        }
+
+        /// <summary>
+        /// Creates a <see cref="RemoteWebElement"/> from a dictionary containing a reference to an element.
+        /// </summary>
+        /// <param name="elementDictionary">The dictionary containing the element reference.</param>
+        /// <returns>A <see cref="RemoteWebElement"/> containing the information from the specified dictionary.</returns>
+        public virtual RemoteWebElement CreateElement(Dictionary<string, object> elementDictionary)
+        {
+            string elementId = this.GetElementId(elementDictionary);
+            return new RemoteWebElement(this.ParentDriver, elementId);
+        }
+
+        /// <summary>
+        /// Gets a value indicating wether the specified dictionary represents a reference to a web element.
+        /// </summary>
+        /// <param name="elementDictionary">The dictionary to check.</param>
+        /// <returns><see langword="true"/> if the dictionary contains an element reference; otherwise, <see langword="false"/>.</returns>
+        public bool ContainsElementReference(Dictionary<string, object> elementDictionary)
+        {
+            string elementPropertyName = string.Empty;
+            return this.TryGetElementPropertyName(elementDictionary, out elementPropertyName);
+        }
+
+        public string GetElementId(Dictionary<string, object> elementDictionary)
+        {
+            string elementPropertyName = string.Empty;
+            if (!this.TryGetElementPropertyName(elementDictionary, out elementPropertyName))
+            {
+                throw new ArgumentException("elementDictionary", "The specified dictionary does not contain an element reference");
+            }
+
+            string elementId = elementDictionary[elementPropertyName].ToString();
+            if (string.IsNullOrEmpty(elementId))
+            {
+                throw new InvalidOperationException("The specified element ID is either null or the empty string.");
+            }
+
+            return elementId;
+        }
+
+        private bool TryGetElementPropertyName(Dictionary<string, object> elementDictionary, out string elementPropertyName)
+        {
+            if (elementDictionary == null)
+            {
+                throw new ArgumentNullException("elementDictionary", "The dictionary containing the element reference cannot be null");
+            }
+
+            if (elementDictionary.ContainsKey(RemoteWebElement.ElementReferencePropertyName))
+            {
+                elementPropertyName = RemoteWebElement.ElementReferencePropertyName;
+                return true;
+            }
+
+            if (elementDictionary.ContainsKey(RemoteWebElement.LegacyElementReferencePropertyName))
+            {
+                elementPropertyName = RemoteWebElement.LegacyElementReferencePropertyName;
+                return true;
+            }
+
+            elementPropertyName = string.Empty;
+            return false;
+        }
+    }
+}