cefsharp / CefSharp

.NET (WPF and Windows Forms) bindings for the Chromium Embedded Framework
http://cefsharp.github.io/
Other
9.88k stars 2.92k forks source link

WinForms Example - File > Close Tab does not allow a fetch to clear cookies on page termination but CTRL + W hotkey does #4846

Open madaster97 opened 5 months ago

madaster97 commented 5 months ago

Is there an existing issue for this?

CefSharp Version

v124.3.80

Operating System

Windows 10

Architecture

x64

.Net Version

net472

Implementation

WinForms

Reproduction Steps

In a CefSharp WinForms application hosting one of my company's web pages, we see an issue where closing a browser tab from the WinForms application will trigger page termination events but does not update the browser's cookie cache based on any fetch responses triggered during those events. For example, we register an unload handler like this (and the approach works in Chrome/Edge):

function serverHandlerUnload() {
            fetch(new Request("/ClearCookie", { method: "POST", keepalive: !0, mode: "same-origin", credentials: "same-origin" }));
}

Where the fetch response has a SetCookie header that clears the existing one.

I was able to reproduce this in the CefSharp.WinForms.Example and was hoping to find a workaround/fix to this that we could use. What's strange is that if you instead close the tab with the CTRL+W hotkey, the fetch responses are able to update the cookie cache!

With the CefSharp.WinForms.Example project and a razor page that demonstrates the cookie handling I can do the following workflow:

  1. Download the demo website and dotnet run it
  2. Open http://localhost:5022 in 2 browser tabs
  3. Click the "Set cookie by client" button in one tab, and refresh both tabs by hitting "Go" in each
  4. In one tab, select the "SERVER" handler radio button and click File > Close Tab
  5. In the second tab, either refresh or click "Re-check cookie" and see the cookie has not been updated in the browser (it should have been cleared by step 4, but wasn't)

If I repeat the above, but in step 4 use the CTRL+W hotkey instead of File > CloseTab to close the tab, the cookies do get cleared! In this case there is a noticeable lag before the tab control/page is completely closed (probably something that is waiting on the fetch call).

The cookies are still not cleared if, in step 4, I swap the unload event to the pagehide (when not persisted) event. This swaps what event we fire the fetch on, and for pagehide swaps to a handler function that respects the event.persisted value:

function serverHandlePageHide(event) {
            if (event.persisted) {
                /* the page isn't being discarded, so it can be reused later */
            } else {
                fetch(new Request("/ClearCookie", { method: "POST", keepalive: !0, mode: "same-origin", credentials: "same-origin" }));
            }
        }

I bring this up because I realize unload is flagged as deprecated, but its replacement of pagehide doesn't work either. For reference:

If you're specifically trying to detect page unload events, the pagehide event is the best option.

The cookies are cleared if, in step 4, I select the "CLIENT" handler instead of "SERVER". In this case, this swaps the handler to just set document.cookie instead of using a fetch response to do so. It seems that if the handlers can run immediately, then they have time to clear the cookies:

function clientHandlerUnload() {
            document.cookie = 'myCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT';
        }

Note this isn't a workaround in our case since we want the server to also invalidate the session it has for the cookie before we clear it from the browser.

Expected behavior

Closing a browser tab from WinForms should support the same termination handling as closing from the browser process (what I assume CTRL+W is doing that File > Close Tab is not in the example).

Actual behavior

Closing a browser tab from WinForms does not support the fetch > clear cookies workflow on termination.

Regression?

I've been having build issues (so many VS errors) and have only been able to test things in v124.3.80.

Known Workarounds

No response

Does this problem also occur in the CEF Sample Application

No

Other information

For cefclient I did not see the issue when I tried Tests > New Window (or Popup Window) and treat that like the tab to close.

amaitland commented 5 months ago

Does https://github.com/cefsharp/CefSharp.MinimalExample/pull/147#discussion_r1624081814 work in the context of the minimal example?

madaster97 commented 5 months ago

Hey @amaitland , when I wrote that other issue I was only looking at network traffic and wasn't testing the full use case (the write up above).

When I tested the demo site with the minimal example, I noticed (1) the example doesn't support multiple tabs and (2) it doesn't support persisting cookies across sessions, at least when running from source in VS. I do think that whatever conclusion we come to here could apply to the minimal example.

madaster97 commented 5 months ago

@amaitland , I did manage to test this in the minimal example. It does not show the same issue, at least in my test of closing/re-opening the minimal example. When I have no handler set the cookie persists, but I can successful clear it with both the client and server handlers, regardless of whether I put in my update from the other issue (removing the browser from the parent control before disposing the browser).

PS - I had to update my site to set a cookie expiration. Without that I wasn't seeing my cookies persist between runs in the first place. Just realized that today.

amaitland commented 4 months ago

I did manage to test this in the minimal example. It does not show the same issue, at least in my test of closing/re-opening the minimal example.

Great, thanks for confirming. When WM_Close is called, then the window will Dispose all it's child controls.

ChromiumWebBrowser.Dispose calls IBrowserHost.CloseBrowser(true) under the hood. The control is disposing, so there's no way for the browser to handle with user input.

https://cef-builds.spotifycdn.com/docs/126.2/classCefBrowserHost.html#a011b691f5e03fb84ffff2d856aab67a1

You can try deferring Dispose of the ChromiumWebBrowser control and instead call IBrowserHost.CloseBrowser(false). You'll likely need to hook LifeSpanHandler and then Dispose of the browser when it's completed the close.

madaster97 commented 2 months ago

Hi @amaitland , I'm not sure whether to open another issue for this, but I've made some progress on diagnosing our issue.

I think there is some sort of race condition happening between the main CefSharp process and the subprocesses. Specifically, if I run my website within the CefSharp.WinForms.Example project, I can inconsistently reproduce an issue where my SetCookie response (triggered from a fetch request within an unload or pagehide handler) does not always get set back into the cookie cache for the browser. I'm actually not sure if the handler is inconsistently firing, or if it is firing and not triggering the fetch in time, because I haven't figured out how to make HTTPS interception work with Fiddler. The example doesn't seem to go to the system proxy.

I think this is a race condition because of how inconsistent the issue is, though I'm trying my best to run the same test every time.

I'm not sure where to go from here, but I've attached a couple of debug logs: one from a successful test and one from a failed one . They both include log lines that mention messages sent to detached frames, and notably we are triggering these requests from within an iframed page:

[32136:43636:0919/130746.047:WARNING:frame_impl.cc(430)] virtual CefFrameImpl::SendProcessMessage sent to detached frame 9-53E0DFE128D63690C83B5C6757E12E88 will be ignored

Success_3_debug_log.txt Failure_2_debug_log.txt (PS - We are aware of the invalid CSP headers, but we see the issue in other environments with proper headers set)

Any advice here would be appreciated!

amaitland commented 2 months ago

pecifically, if I run my website within the CefSharp.WinForms.Example project, I can inconsistently reproduce an issue where my SetCookie response (triggered from a fetch request within an unload or pagehide handler) does not always get set back into the cookie cache for the browser.

From memory the example only calls Dispose, so that's a force close.

You can try deferring Dispose of the ChromiumWebBrowser control and instead call IBrowserHost.CloseBrowser(false). You'll likely need to hook LifeSpanHandler and then Dispose of the browser when it's completed the close.

Did you try this? IBrowserHost.CloseBrowser(false) that should give the browser more time to close.

madaster97 commented 1 week ago

Hey @amaitland , I have been trying to test out your recommendation but am not sure exactly how to modify the minimal example to achieve that. Mind taking a look?

The CloseTabToolStripMenuItemClick method in BrowserForm.cs seems to be doing the heavy lifting, and is triggered when I hit CTRL-W within the tab.

Naively, I tried replacing the call to control.Dispose there with control.Browser.GetBrowserHost().CloseBrowser(false). Is that what you had in mind for deferring the Dispose?

If that is the case, there is also some code below that which is responsible for cleaning up the list of tabPages. Is there somewhere we would move that code to trigger after/alongside the Hook in ILifeSpanHandler that will dispose of the browser?

amaitland commented 1 week ago

Naively, I tried replacing the call to control.Dispose there with control.Browser.GetBrowserHost().CloseBrowser(false). Is that what you had in mind for deferring the Dispose?

Pretty much. Stop the tab from closing first.

Might actually better to call https://cefsharp.github.io/api/130.1.x/html/M_CefSharp_IBrowserHost_TryCloseBrowser.htm

Is there somewhere we would move that code to trigger after/alongside the Hook in ILifeSpanHandler that will dispose of the browser?

https://cefsharp.github.io/api/130.1.x/html/M_CefSharp_ILifeSpanHandler_OnBeforeClose.htm should be called just before the browser is closed, then remove the tab.

API doc might be a little out of date, so reading the upstream doc is probably better at the moment.

https://cef-builds.spotifycdn.com/docs/131.2/classCefLifeSpanHandler.html#a4867b3d97d879d9996dec2cf1daa483d