Fix cache disabling should stick when toggling request interception (#1110)

* Fix cache disabling should stick when toggling request interception

* Remove call to Security.setOverrideCertificateError

* Fix race

Author: kblok

lib/PuppeteerSharp.Tests/PageTests/SetCacheEnabledTests.cs

@@ -34,5 +34,22 @@ await Task.WhenAll(
 
             Assert.True(string.IsNullOrEmpty(waitForRequestTask.Result));
         }
+
+        [Fact]
+        public async Task ShouldStayDisabledWhenTogglingRequestInterceptionOnOff()
+        {
+            await Page.SetCacheEnabledAsync(false);
+            await Page.SetRequestInterceptionAsync(true);
+            await Page.SetRequestInterceptionAsync(false);
+
+            await Page.GoToAsync(TestConstants.ServerUrl + "/cached/one-style.html");
+            var waitForRequestTask = Server.WaitForRequest<string>("/cached/one-style.html", (request) => request.Headers["if-modified-since"]);
+
+            await Task.WhenAll(
+              waitForRequestTask,
+              Page.ReloadAsync());
+
+            Assert.True(string.IsNullOrEmpty(waitForRequestTask.Result));
+        }
     }
 }

lib/PuppeteerSharp/FrameManager.cs

@@ -22,14 +22,15 @@ internal class FrameManager
         private readonly List<string> _isolatedWorlds = new List<string>();
         private const string UtilityWorldName = "__puppeteer_utility_world__";
 
-        private FrameManager(CDPSession client, Page page, NetworkManager networkManager, TimeoutSettings timeoutSettings)
+        private FrameManager(CDPSession client, Page page, bool ignoreHTTPSErrors, TimeoutSettings timeoutSettings)
         {
             Client = client;
             Page = page;
             _frames = new ConcurrentDictionary<string, Frame>();
             _contextIdToContext = new Dictionary<int, ExecutionContext>();
             _logger = Client.Connection.LoggerFactory.CreateLogger<FrameManager>();
-            NetworkManager = networkManager;
+            NetworkManager = new NetworkManager(client, ignoreHTTPSErrors);
+            NetworkManager.FrameManager = this;
             TimeoutSettings = timeoutSettings;
             _asyncFrames = new AsyncDictionaryHelper<string, Frame>(_frames, "Frame {0} not found");
 
@@ -55,12 +56,25 @@ private FrameManager(CDPSession client, Page page, NetworkManager networkManager
         internal static async Task<FrameManager> CreateFrameManagerAsync(
             CDPSession client,
             Page page,
-            NetworkManager networkManager,
-            FrameTree frameTree,
+            bool ignoreHTTPSErrors,
             TimeoutSettings timeoutSettings)
         {
-            var frameManager = new FrameManager(client, page, networkManager, timeoutSettings);
-            await frameManager.HandleFrameTreeAsync(frameTree).ConfigureAwait(false);
+            var frameManager = new FrameManager(client, page, ignoreHTTPSErrors, timeoutSettings);
+            var getFrameTreeTask = client.SendAsync<PageGetFrameTreeResponse>("Page.getFrameTree");
+
+            await Task.WhenAll(
+                client.SendAsync("Page.enable"),
+                getFrameTreeTask).ConfigureAwait(false);
+
+            await frameManager.HandleFrameTreeAsync(new FrameTree(getFrameTreeTask.Result.FrameTree)).ConfigureAwait(false);
+
+            await Task.WhenAll(
+                client.SendAsync("Page.setLifecycleEventsEnabled", new PageSetLifecycleEventsEnabledRequest { Enabled = true }),
+                client.SendAsync("Runtime.enable"),
+                frameManager.NetworkManager.InitializeAsync()).ConfigureAwait(false);
+
+            await frameManager.EnsureSecondaryDOMWorldAsync().ConfigureAwait(false);
+
             return frameManager;
         }
 
@@ -386,9 +400,9 @@ private async Task HandleFrameTreeAsync(FrameTree frameTree)
             }
         }
 
-        internal Task EnsureSecondaryDOMWorldAsync() => EnsureSecondaryDOMWorldAsync(UtilityWorldName);
+        private Task EnsureSecondaryDOMWorldAsync() => EnsureSecondaryDOMWorldAsync(UtilityWorldName);
 
-        internal async Task EnsureSecondaryDOMWorldAsync(string name)
+        private async Task EnsureSecondaryDOMWorldAsync(string name)
         {
             if (_isolatedWorlds.Contains(name))
             {

lib/PuppeteerSharp/Messaging/SecuritySetIgnoreCertificateErrorsRequest.cs

@@ -0,0 +1,7 @@
+namespace PuppeteerSharp.Messaging
+{
+    internal class SecuritySetIgnoreCertificateErrorsRequest
+    {
+        public bool Ignore { get; set; }
+    }
+}

lib/PuppeteerSharp/NetworkManager.cs

@@ -25,13 +25,15 @@ internal class NetworkManager
         private List<string> _attemptedAuthentications = new List<string>();
         private bool _userRequestInterceptionEnabled;
         private bool _protocolRequestInterceptionEnabled;
-
+        private bool _ignoreHTTPSErrors;
+        private bool _userCacheDisabled;
         #endregion
 
-        internal NetworkManager(CDPSession client)
+        internal NetworkManager(CDPSession client, bool ignoreHTTPSErrors)
         {
             FrameManager = null;
             _client = client;
+            _ignoreHTTPSErrors = ignoreHTTPSErrors;
             _client.MessageReceived += Client_MessageReceived;
             _logger = _client.Connection.LoggerFactory.CreateLogger<NetworkManager>();
         }
@@ -47,6 +49,18 @@ internal NetworkManager(CDPSession client)
 
         #region Public Methods
 
+        internal async Task InitializeAsync()
+        {
+            await _client.SendAsync("Network.enable").ConfigureAwait(false);
+            if (_ignoreHTTPSErrors)
+            {
+                await _client.SendAsync("Security.setIgnoreCertificateErrors", new SecuritySetIgnoreCertificateErrorsRequest
+                {
+                    Ignore = true
+                }).ConfigureAwait(false);
+            }
+        }
+
         internal Task AuthenticateAsync(Credentials credentials)
         {
             _credentials = credentials;
@@ -89,6 +103,12 @@ internal Task SetUserAgentAsync(string userAgent)
                 UserAgent = userAgent
             });
 
+        internal Task SetCacheEnabledAsync(bool enabled)
+        {
+            _userCacheDisabled = !enabled;
+            return UpdateProtocolCacheDisabledAsync();
+        }
+
         internal Task SetRequestInterceptionAsync(bool value)
         {
             _userRequestInterceptionEnabled = value;
@@ -99,6 +119,12 @@ internal Task SetRequestInterceptionAsync(bool value)
 
         #region Private Methods
 
+        private Task UpdateProtocolCacheDisabledAsync()
+            => _client.SendAsync("Network.setCacheDisabled", new NetworkSetCacheDisabledRequest
+            {
+                CacheDisabled = _userCacheDisabled || _protocolRequestInterceptionEnabled
+            });
+
         private async void Client_MessageReceived(object sender, MessageEventArgs e)
         {
             try
@@ -365,10 +391,7 @@ private async Task UpdateProtocolRequestInterceptionAsync()
                 Array.Empty<object>();
 
             await Task.WhenAll(
-                _client.SendAsync("Network.setCacheDisabled", new NetworkSetCacheDisabledRequest
-                {
-                    CacheDisabled = enabled
-                }),
+                UpdateProtocolCacheDisabledAsync(),
                 _client.SendAsync("Network.setRequestInterception", new NetworkSetRequestInterceptionRequest
                 {
                     Patterns = patterns

lib/PuppeteerSharp/Page.cs

@@ -36,8 +36,6 @@ namespace PuppeteerSharp
     [DebuggerDisplay("Page {Url}")]
     public class Page : IDisposable
     {
-        private readonly bool _ignoreHTTPSErrors;
-        private NetworkManager _networkManager;
         private readonly TaskQueue _screenshotTaskQueue;
         private readonly EmulationManager _emulationManager;
         private readonly Dictionary<string, Delegate> _pageBindings;
@@ -59,7 +57,6 @@ public class Page : IDisposable
         private Page(
             CDPSession client,
             Target target,
-            bool ignoreHTTPSErrors,
             TaskQueue screenshotTaskQueue)
         {
             Client = client;
@@ -76,7 +73,6 @@ private Page(
             _workers = new Dictionary<string, Worker>();
             _logger = Client.Connection.LoggerFactory.CreateLogger<Page>();
             Accessibility = new Accessibility(client);
-            _ignoreHTTPSErrors = ignoreHTTPSErrors;
 
             _screenshotTaskQueue = screenshotTaskQueue;
 
@@ -92,8 +88,6 @@ private Page(
                     _closeCompletedTcs.TrySetResult(true);
                 }
             });
-
-            Client.MessageReceived += Client_MessageReceived;
         }
 
         internal CDPSession Client { get; }
@@ -521,14 +515,14 @@ public async Task<JSHandle> QueryObjectsAsync(JSHandle prototypeHandle)
         /// <returns>The request interception task.</returns>
         /// <param name="value">Whether to enable request interception..</param>
         public Task SetRequestInterceptionAsync(bool value)
-            => _networkManager.SetRequestInterceptionAsync(value);
+            => FrameManager.NetworkManager.SetRequestInterceptionAsync(value);
 
         /// <summary>
         /// Set offline mode for the page.
         /// </summary>
         /// <returns>Result task</returns>
         /// <param name="value">When <c>true</c> enables offline mode for the page.</param>
-        public Task SetOfflineModeAsync(bool value) => _networkManager.SetOfflineModeAsync(value);
+        public Task SetOfflineModeAsync(bool value) => FrameManager.NetworkManager.SetOfflineModeAsync(value);
 
         /// <summary>
         /// Returns the page's cookies
@@ -1120,7 +1114,7 @@ public Task CloseAsync(PageCloseOptions options = null)
         /// <param name="enabled">sets the <c>enabled</c> state of the cache</param>
         /// <returns>Task</returns>
         public Task SetCacheEnabledAsync(bool enabled = true)
-            => Client.SendAsync("Network.setCacheDisabled", new NetworkSetCacheDisabledRequest { CacheDisabled = !enabled });
+            => FrameManager.NetworkManager.SetCacheEnabledAsync(enabled);
 
         /// <summary>
         /// Fetches an element with <paramref name="selector"/>, scrolls it into view if needed, and then uses <see cref="Page.Mouse"/> to click in the center of the element.
@@ -1227,15 +1221,15 @@ public Task<T> EvaluateFunctionAsync<T>(string script, params object[] args)
         /// <param name="userAgent">Specific user agent to use in this page</param>
         /// <returns>Task</returns>
         public Task SetUserAgentAsync(string userAgent)
-            => _networkManager.SetUserAgentAsync(userAgent);
+            => FrameManager.NetworkManager.SetUserAgentAsync(userAgent);
 
         /// <summary>
         /// Sets extra HTTP headers that will be sent with every request the page initiates
         /// </summary>
         /// <param name="headers">Additional http headers to be sent with every request</param>
         /// <returns>Task</returns>
         public Task SetExtraHttpHeadersAsync(Dictionary<string, string> headers)
-            => _networkManager.SetExtraHTTPHeadersAsync(headers);
+            => FrameManager.NetworkManager.SetExtraHTTPHeadersAsync(headers);
 
         /// <summary>
         /// Provide credentials for http authentication <see href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication"/>
@@ -1245,7 +1239,7 @@ public Task SetExtraHttpHeadersAsync(Dictionary<string, string> headers)
         /// <remarks>
         /// To disable authentication, pass <c>null</c>
         /// </remarks>
-        public Task AuthenticateAsync(Credentials credentials) => _networkManager.AuthenticateAsync(credentials);
+        public Task AuthenticateAsync(Credentials credentials) => FrameManager.NetworkManager.AuthenticateAsync(credentials);
 
         /// <summary>
         /// Reloads the page
@@ -1432,15 +1426,15 @@ void requestEventListener(object sender, RequestEventArgs e)
                 if (predicate(e.Request))
                 {
                     requestTcs.TrySetResult(e.Request);
-                    _networkManager.Request -= requestEventListener;
+                    FrameManager.NetworkManager.Request -= requestEventListener;
                 }
             }
 
-            _networkManager.Request += requestEventListener;
+            FrameManager.NetworkManager.Request += requestEventListener;
 
             return await requestTcs.Task.WithTimeout(timeout, t =>
             {
-                _networkManager.Request -= requestEventListener;
+                FrameManager.NetworkManager.Request -= requestEventListener;
                 return new TimeoutException($"Timeout Exceeded: {t.TotalMilliseconds}ms exceeded");
             }).ConfigureAwait(false);
         }
@@ -1486,11 +1480,11 @@ void responseEventListener(object sender, ResponseCreatedEventArgs e)
                 if (predicate(e.Response))
                 {
                     responseTcs.TrySetResult(e.Response);
-                    _networkManager.Response -= responseEventListener;
+                    FrameManager.NetworkManager.Response -= responseEventListener;
                 }
             }
 
-            _networkManager.Response += responseEventListener;
+            FrameManager.NetworkManager.Response += responseEventListener;
 
             return await responseTcs.Task.WithTimeout(timeout).ConfigureAwait(false);
         }
@@ -1538,10 +1532,8 @@ internal static async Task<Page> CreateAsync(
             ViewPortOptions defaultViewPort,
             TaskQueue screenshotTaskQueue)
         {
-            await client.SendAsync("Page.enable", null).ConfigureAwait(false);
-            var result = await client.SendAsync<PageGetFrameTreeResponse>("Page.getFrameTree").ConfigureAwait(false);
-            var page = new Page(client, target, ignoreHTTPSErrors, screenshotTaskQueue);
-            await page.InitializeAsync(new FrameTree(result.FrameTree)).ConfigureAwait(false);
+            var page = new Page(client, target, screenshotTaskQueue);
+            await page.InitializeAsync(ignoreHTTPSErrors).ConfigureAwait(false);
 
             await Task.WhenAll(
                 client.SendAsync("Target.setAutoAttach", new TargetSetAutoAttachRequest
@@ -1561,16 +1553,6 @@ await Task.WhenAll(
                 client.SendAsync("Log.enable", null)
             ).ConfigureAwait(false);
 
-            await page.FrameManager.EnsureSecondaryDOMWorldAsync().ConfigureAwait(false);
-
-            if (ignoreHTTPSErrors)
-            {
-                await client.SendAsync("Security.setOverrideCertificateErrors", new SecuritySetOverrideCertificateErrorsRequest
-                {
-                    Override = true
-                }).ConfigureAwait(false);
-            }
-
             if (defaultViewPort != null)
             {
                 await page.SetViewportAsync(defaultViewPort).ConfigureAwait(false);
@@ -1579,20 +1561,20 @@ await Task.WhenAll(
             return page;
         }
 
-        private async Task InitializeAsync(FrameTree frameTree)
+        private async Task InitializeAsync(bool ignoreHTTPSErrors)
         {
-            _networkManager = new NetworkManager(Client);
-            FrameManager = await FrameManager.CreateFrameManagerAsync(Client, this, _networkManager, frameTree, _timeoutSettings).ConfigureAwait(false);
-            _networkManager.FrameManager = FrameManager;
+            FrameManager = await FrameManager.CreateFrameManagerAsync(Client, this, ignoreHTTPSErrors, _timeoutSettings).ConfigureAwait(false);
+            var networkManager = FrameManager.NetworkManager;
 
+            Client.MessageReceived += Client_MessageReceived;
             FrameManager.FrameAttached += (sender, e) => FrameAttached?.Invoke(this, e);
             FrameManager.FrameDetached += (sender, e) => FrameDetached?.Invoke(this, e);
             FrameManager.FrameNavigated += (sender, e) => FrameNavigated?.Invoke(this, e);
 
-            _networkManager.Request += (sender, e) => Request?.Invoke(this, e);
-            _networkManager.RequestFailed += (sender, e) => RequestFailed?.Invoke(this, e);
-            _networkManager.Response += (sender, e) => Response?.Invoke(this, e);
-            _networkManager.RequestFinished += (sender, e) => RequestFinished?.Invoke(this, e);
+            networkManager.Request += (sender, e) => Request?.Invoke(this, e);
+            networkManager.RequestFailed += (sender, e) => RequestFailed?.Invoke(this, e);
+            networkManager.Response += (sender, e) => Response?.Invoke(this, e);
+            networkManager.RequestFinished += (sender, e) => RequestFinished?.Invoke(this, e);
         }
 
         private async Task<Response> GoAsync(int delta, NavigationOptions options)
@@ -1822,17 +1804,14 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)
                         Load?.Invoke(this, EventArgs.Empty);
                         break;
                     case "Runtime.consoleAPICalled":
-                        await OnConsoleAPI(e.MessageData.ToObject<PageConsoleResponse>(true)).ConfigureAwait(false);
+                        await OnConsoleAPIAsync(e.MessageData.ToObject<PageConsoleResponse>(true)).ConfigureAwait(false);
                         break;
                     case "Page.javascriptDialogOpening":
                         OnDialog(e.MessageData.ToObject<PageJavascriptDialogOpeningResponse>(true));
                         break;
                     case "Runtime.exceptionThrown":
                         HandleException(e.MessageData.ToObject<RuntimeExceptionThrownResponse>(true).ExceptionDetails);
                         break;
-                    case "Security.certificateError":
-                        await OnCertificateError(e.MessageData.ToObject<CertificateErrorResponse>(true)).ConfigureAwait(false);
-                        break;
                     case "Inspector.targetCrashed":
                         OnTargetCrashed();
                         break;
@@ -1994,25 +1973,6 @@ private void OnTargetCrashed()
         private void EmitMetrics(PerformanceMetricsResponse metrics)
             => Metrics?.Invoke(this, new MetricEventArgs(metrics.Title, BuildMetricsObject(metrics.Metrics)));
 
-        private async Task OnCertificateError(CertificateErrorResponse e)
-        {
-            if (_ignoreHTTPSErrors)
-            {
-                try
-                {
-                    await Client.SendAsync("Security.handleCertificateError", new SecurityHandleCertificateErrorResponse
-                    {
-                        EventId = e.EventId,
-                        Action = "continue"
-                    }).ConfigureAwait(false);
-                }
-                catch (PuppeteerException ex)
-                {
-                    _logger.LogError(ex.ToString());
-                }
-            }
-        }
-
         private void HandleException(EvaluateExceptionResponseDetails exceptionDetails)
             => PageError?.Invoke(this, new PageErrorEventArgs(GetExceptionMessage(exceptionDetails)));
 
@@ -2041,7 +2001,7 @@ private void OnDialog(PageJavascriptDialogOpeningResponse message)
             Dialog?.Invoke(this, new DialogEventArgs(dialog));
         }
 
-        private Task OnConsoleAPI(PageConsoleResponse message)
+        private Task OnConsoleAPIAsync(PageConsoleResponse message)
         {
             if (message.ExecutionContextId == 0)
             {