Remove dynamic evaluate functions (#167)

Meir017 · kblok · commit 99f62f9bcda7 · 2018-04-25T08:53:17.000-03:00
diff --git a/lib/PuppeteerSharp.Tests/Frame/WaitForSelectorTests.cs b/lib/PuppeteerSharp.Tests/Frame/WaitForSelectorTests.cs
@@ -214,4 +214,4 @@ public async Task ShouldReturnTheElementHandle()
             Assert.Equal("anything", await Page.EvaluateFunctionAsync<string>("x => x.textContent", await waitForSelector));
         }
     }
-}
+}
diff --git a/lib/PuppeteerSharp.Tests/Page/CloseTests.cs b/lib/PuppeteerSharp.Tests/Page/CloseTests.cs
@@ -11,12 +11,12 @@ public async Task ShouldRejectAllPromisesWhenPageIsClosed()
         {
             var newPage = await Browser.NewPageAsync();
             var neverResolves = newPage.EvaluateFunctionAsync("() => new Promise(r => {})");
-            
+
             // Put into a var to avoid warning
             var t = newPage.CloseAsync();
 
             var exception = await Assert.ThrowsAsync<TargetClosedException>(async () => await neverResolves);
-            
+
             Assert.Contains("Protocol error", exception.Message);
         }
     }
diff --git a/lib/PuppeteerSharp.Tests/Page/CookiesTests.cs b/lib/PuppeteerSharp.Tests/Page/CookiesTests.cs
@@ -14,10 +14,7 @@ public async Task ShouldGetAndSetCookies()
             await Page.GoToAsync(TestConstants.ServerUrl + "/grid.html");
             Assert.Empty(await Page.GetCookiesAsync());
 
-            await Page.EvaluateFunctionAsync(@"() =>
-                {
-                    document.cookie = 'username=John Doe';
-                }");
+            await Page.EvaluateExpressionAsync("document.cookie = 'username=John Doe'");
             var cookie = Assert.Single(await Page.GetCookiesAsync());
             Assert.Equal("username", cookie.Name);
             Assert.Equal("John Doe", cookie.Value);
diff --git a/lib/PuppeteerSharp.Tests/Page/EvaluateTests.cs b/lib/PuppeteerSharp.Tests/Page/EvaluateTests.cs
@@ -94,5 +94,19 @@ public async Task ShouldProperlySerializeNullFields()
             Assert.True(result.ContainsKey("a"));
             Assert.Null(result["a"]);
         }
+
+        [Fact]
+        public async Task EvaluateExpressionBigObject()
+        {
+            var window = await Page.EvaluateExpressionAsync("window");
+            Assert.Null(window);
+        }
+
+        [Fact]
+        public async Task EvaluateFunctionBigObject()
+        {
+            var window = await Page.EvaluateFunctionAsync("() => window");
+            Assert.Null(window);
+        }
     }
 }
diff --git a/lib/PuppeteerSharp/ExecutionContext.cs b/lib/PuppeteerSharp/ExecutionContext.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using System.Linq;
-using Newtonsoft.Json.Linq;
 
 namespace PuppeteerSharp
 {
@@ -24,27 +23,63 @@ public ExecutionContext(Session client, ContextPayload contextPayload, Func<dyna
         public string FrameId { get; internal set; }
         public bool IsDefault { get; internal set; }
 
-        public async Task<object> EvaluateExpressionAsync(string script)
-            => await EvaluateExpressionAsync<object>(script);
-
-        public async Task<T> EvaluateExpressionAsync<T>(string script)
-        {
-            var handle = await EvaluateExpressionHandleAsync(script);
-            var result = await handle.JsonValue<T>();
-            await handle.Dispose();
-            return result;
-        }
-
-        public async Task<object> EvaluateFunctionAsync(string script, params object[] args)
-            => await EvaluateFunctionAsync<object>(script, args);
-
-        public async Task<T> EvaluateFunctionAsync<T>(string script, params object[] args)
-        {
-            var handle = await EvaluateFunctionHandleAsync(script, args);
-            var result = await handle.JsonValue<T>();
-            await handle.Dispose();
-            return result;
-        }
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// </remarks>
+        /// <seealso cref="EvaluateFunctionAsync(string, object[])"/>
+        /// <seealso cref="EvaluateExpressionHandleAsync(string)"/>
+        /// <returns>Task which resolves to script return value</returns>
+        public Task<object> EvaluateExpressionAsync(string script)
+            => EvaluateExpressionAsync<object>(script);
+
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <typeparam name="T">The type to deserialize the result to</typeparam>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// </remarks>
+        /// <seealso cref="EvaluateFunctionAsync{T}(string, object[])"/>
+        /// <seealso cref="EvaluateExpressionHandleAsync(string)"/>
+        /// <returns>Task which resolves to script return value</returns>
+        public Task<T> EvaluateExpressionAsync<T>(string script)
+            => EvaluateAsync<T>(EvaluateExpressionHandleAsync(script));
+
+        /// <summary>
+        /// Executes a function in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <param name="args">Arguments to pass to script</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// <see cref="JSHandle"/> instances can be passed as arguments
+        /// </remarks>
+        /// <seealso cref="EvaluateExpressionAsync(string)"/>
+        /// <seealso cref="EvaluateFunctionHandleAsync(string, object[])"/>
+        /// <returns>Task which resolves to script return value</returns>
+        public Task<object> EvaluateFunctionAsync(string script, params object[] args)
+            => EvaluateFunctionAsync<object>(script, args);
+
+        /// <summary>
+        /// Executes a function in browser context
+        /// </summary>
+        /// <typeparam name="T">The type to deserialize the result to</typeparam>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <param name="args">Arguments to pass to script</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// <see cref="JSHandle"/> instances can be passed as arguments
+        /// </remarks>
+        /// <seealso cref="EvaluateExpressionAsync{T}(string)"/>
+        /// <seealso cref="EvaluateFunctionHandleAsync(string, object[])"/>
+        /// <returns>Task which resolves to script return value</returns>
+        public Task<T> EvaluateFunctionAsync<T>(string script, params object[] args)
+            => EvaluateAsync<T>(EvaluateFunctionHandleAsync(script, args));
 
         internal async Task<JSHandle> EvaluateExpressionHandleAsync(string script)
         {
@@ -79,6 +114,16 @@ internal async Task<JSHandle> EvaluateFunctionHandleAsync(string script, params
             });
         }
 
+        private async Task<T> EvaluateAsync<T>(Task<JSHandle> handleEvaluator)
+        {
+            var handle = await handleEvaluator;
+            var result = await handle.JsonValue<T>()
+                .ContinueWith(jsonTask => jsonTask.Exception != null ? default(T) : jsonTask.Result);
+
+            await handle.Dispose();
+            return result;
+        }
+
         private async Task<JSHandle> EvaluateHandleAsync(string method, dynamic args)
         {
             dynamic response = await _client.SendAsync(method, args);
@@ -136,6 +181,5 @@ public async Task<dynamic> QueryObjects(JSHandle prototypeHandle)
 
             return ObjectHandleFactory(response.objects);
         }
-
     }
-}
+}
diff --git a/lib/PuppeteerSharp/Frame.cs b/lib/PuppeteerSharp/Frame.cs
@@ -1,7 +1,6 @@
 ﻿using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
-using PuppeteerSharp.Input;
 
 namespace PuppeteerSharp
 {
@@ -50,24 +49,66 @@ public Frame(Session client, Page page, Frame parentFrame, string frameId)
 
         #region Public Methods
 
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// </remarks>
+        /// <seealso cref="EvaluateFunctionAsync(string, object[])"/>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<dynamic> EvaluateExpressionAsync(string script)
         {
             var context = await GetExecutionContextAsync();
             return await context.EvaluateExpressionAsync(script);
         }
 
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <typeparam name="T">The type to deserialize the result to</typeparam>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// </remarks>
+        /// <seealso cref="EvaluateFunctionAsync{T}(string, object[])"/>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<T> EvaluateExpressionAsync<T>(string script)
         {
             var context = await GetExecutionContextAsync();
             return await context.EvaluateExpressionAsync<T>(script);
         }
 
+        /// <summary>
+        /// Executes a function in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <param name="args">Arguments to pass to script</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// <see cref="JSHandle"/> instances can be passed as arguments
+        /// </remarks>
+        /// <seealso cref="EvaluateExpressionAsync(string)"/>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<dynamic> EvaluateFunctionAsync(string script, params object[] args)
         {
             var context = await GetExecutionContextAsync();
             return await context.EvaluateFunctionAsync(script, args);
         }
 
+        /// <summary>
+        /// Executes a function in browser context
+        /// </summary>
+        /// <typeparam name="T">The type to deserialize the result to</typeparam>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <param name="args">Arguments to pass to script</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// <see cref="JSHandle"/> instances can be passed as arguments
+        /// </remarks>
+        /// <seealso cref="EvaluateExpressionAsync{T}(string)"/>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<T> EvaluateFunctionAsync<T>(string script, params object[] args)
         {
             var context = await GetExecutionContextAsync();
@@ -122,28 +163,25 @@ internal Task<ElementHandle> AddScriptTag(dynamic options)
             throw new NotImplementedException();
         }
 
-        internal async Task<string> GetContentAsync()
-        {
-            return await EvaluateFunctionAsync<string>(@"() => {
+        internal Task<string> GetContentAsync()
+            => EvaluateFunctionAsync<string>(@"() => {
                 let retVal = '';
                 if (document.doctype)
                     retVal = new XMLSerializer().serializeToString(document.doctype);
                 if (document.documentElement)
                     retVal += document.documentElement.outerHTML;
                 return retVal;
             }");
-        }
 
-        internal async Task SetContentAsync(string html)
-        {
-            await EvaluateFunctionAsync(@"html => {
+        internal Task SetContentAsync(string html)
+            => EvaluateFunctionAsync(@"html => {
                 document.open();
                 document.write(html);
                 document.close();
             }", html);
-        }
 
-        internal async Task<string> GetTitleAsync() => await EvaluateExpressionAsync<string>("document.title");
+
+        internal Task<string> GetTitleAsync() => EvaluateExpressionAsync<string>("document.title");
 
         internal void OnLifecycleEvent(string loaderId, string name)
         {
diff --git a/lib/PuppeteerSharp/Page.cs b/lib/PuppeteerSharp/Page.cs
@@ -155,12 +155,29 @@ public async Task<ElementHandle> GetElementAsync(string selector)
         public async Task<IEnumerable<ElementHandle>> GetElementsAsync(string selector)
             => await MainFrame.GetElementsAsync(selector);
 
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// </remarks>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<JSHandle> EvaluateExpressionHandleAsync(string script)
         {
             var context = await MainFrame.GetExecutionContextAsync();
             return await context.EvaluateExpressionHandleAsync(script);
         }
 
+        /// <summary>
+        /// Executes a script in browser context
+        /// </summary>
+        /// <param name="script">Script to be evaluated in browser context</param>
+        /// <remarks>
+        /// If the script, returns a Promise, then the method would wait for the promise to resolve and return its value.
+        /// <see cref="JSHandle"/> instances can be passed as arguments
+        /// </remarks>
+        /// <returns>Task which resolves to script return value</returns>
         public async Task<JSHandle> EvaluateFunctionHandleAsync(string pageFunction, params object[] args)
         {
             var context = await MainFrame.GetExecutionContextAsync();
@@ -521,7 +538,7 @@ public Task CloseAsync()
 
             return Task.CompletedTask;
         }
-
+        
         public Task<dynamic> EvaluateExpressionAsync(string script)
             => _frameManager.MainFrame.EvaluateExpressionAsync(script);
 

Original file line number	Diff line number	Diff line change
`@@ -214,4 +214,4 @@ public async Task ShouldReturnTheElementHandle()`
`214`	`214`	`Assert.Equal("anything", await Page.EvaluateFunctionAsync<string>("x => x.textContent", await waitForSelector));`
`215`	`215`	`}`
`216`	`216`	`}`
`217`		`-}`
	`217`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -11,12 +11,12 @@ public async Task ShouldRejectAllPromisesWhenPageIsClosed()`
`11`	`11`	`{`
`12`	`12`	`var newPage = await Browser.NewPageAsync();`
`13`	`13`	`var neverResolves = newPage.EvaluateFunctionAsync("() => new Promise(r => {})");`
`14`		`-`
	`14`	`+`
`15`	`15`	`// Put into a var to avoid warning`
`16`	`16`	`var t = newPage.CloseAsync();`
`17`	`17`
`18`	`18`	`var exception = await Assert.ThrowsAsync<TargetClosedException>(async () => await neverResolves);`
`19`		`-`
	`19`	`+`
`20`	`20`	`Assert.Contains("Protocol error", exception.Message);`
`21`	`21`	`}`
`22`	`22`	`}`