MicrosoftEdge / WebView2Feedback

Feedback and discussions about Microsoft Edge WebView2
https://aka.ms/webview2
449 stars 53 forks source link

downloadOperation.StateChanged is not occurring #2180

Open iragrollman opened 2 years ago

iragrollman commented 2 years ago

Description event handler for DownloadStarting sets e.Handled = True and adds event handlers for downloadOperation.StateChanged, downloadOperation.BytesReceivedChanged, and downloadOperation.EstimatedEndTimeChanged downloadOperation.StateChanged is not occurring

Version Microsoft.Web.WebView2.WinForms & Core 1.0.1108.44 Winforms using .NET Framework 4.7.2 Windows Server 2019 1809 (17763.2452), Visual Studio Professional 2019 16.11.9

When the program runs, the events logged are: WebView2_CoreWebView2InitializationCompleted WebView2_NavigationStarting about:blank WebView2_NavigationCompleted

(navigation complete to about:blank kicks off navigation to actual site using CreateWebResourceRequest and NavigateWithWebResourceRequest)

WebView2_NavigationStarting http://mysite CoreWebView2_WebResourceRequested CoreWebView2_WebResourceResponseReceived

(site response includes a file download)

Transfer-Encoding chunked WebView2_NavigationCompleted Webview2_DownloadStarting

(downloadOperation event handlers are added here)

EstimatedEndTimeChanged 2/15/2022 8:32:12 PM InProgress bytes received 42120 EstimatedEndTimeChanged 2/15/2022 8:32:12 PM InProgress EstimatedEndTimeChanged 2/15/2022 8:32:12 PM InProgress EstimatedEndTimeChanged 2/15/2022 8:32:12 PM InProgress EstimatedEndTimeChanged 2/15/2022 8:32:12 PM InProgress

AB#41754927

champnic commented 2 years ago

Hey @iragrollman - I just did a quick test and didn't get this result. Do you have a sample app that reproduces this issue, or an url you are trying to use for the download?

iragrollman commented 2 years ago

I can make my application available and if you can supply the IP address you would be coming in from, set my firewall to permit connection.

Regards, Ira Grollman, OMKT LLC, 603-493-9261

From: Nic Champagne Williamson [MSFT] @.*** Sent: Thursday, February 17, 2022 6:31 PM To: MicrosoftEdge/WebView2Feedback Cc: iragrollman; Mention Subject: Re: [MicrosoftEdge/WebView2Feedback] downloadOperation.StateChanged is not occurring (Issue #2180)

Hey @iragrollman https://github.com/iragrollman - I just did a quick test and didn't get this result. Do you have a sample app that reproduces this issue, or an url you are trying to use for the download?

— Reply to this email directly, view it on GitHub https://github.com/MicrosoftEdge/WebView2Feedback/issues/2180#issuecomment-1043620902 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJHLT7JYIR6FDX7PPTWO6TU3WAMNANCNFSM5OQHJXHA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub . You are receiving this because you were mentioned.Image removed by sender.Message ID: @.***>

HarriVa commented 2 years ago

I have the same issue in my application. My best guess is that if the downloaded file is very small, then in some cases the StateChanged event does no fire. In my application also the DownloadStarting event is sometimes not fired.

Environment: Windows 10, WinForms, WebView2 version 1.0.1150.38, MS Edge Runtime 99.0.1150.46

How to reproduce

I created a simple WinForms application with button1 and related click-function. When I click the button, sometimes the message box "Completed" appears, sometimes not (even though the file is loaded). The sample file is something that I found with Google, probably not optimal for testing.

public partial class Form1 : Form
    {
        private string currFilePath = "";
        private string currFileState = "";
        long bytesReceived = 0;

        private WebView2 webView = null;
        private CoreWebView2DownloadOperation downloadOperation;

        public Form1()
        {
            InitializeComponent();
        }

        private async void button1_Click(object sender, EventArgs e)
        {
            if (webView != null)
                webView.Dispose();

            webView = new WebView2();
            await webView.EnsureCoreWebView2Async();
            webView.CoreWebView2.DownloadStarting += CoreWebView2_DownloadStarting;

            // Random sample file found using Google
            webView.Source = new Uri("https://filesamples.com/samples/document/csv/sample1.csv");
        }

        private void CoreWebView2_DownloadStarting(object sender, CoreWebView2DownloadStartingEventArgs e)
        {
            e.Handled = true;
            downloadOperation = e.DownloadOperation;
            downloadOperation.BytesReceivedChanged += DownloadOperation_BytesReceivedChanged;
            downloadOperation.StateChanged += DownloadOperation_StateChanged;
            currFilePath = e.ResultFilePath;
        }

        private void DownloadOperation_BytesReceivedChanged(object sender, object e)
        {
            bytesReceived = downloadOperation.BytesReceived;
        }

        private void DownloadOperation_StateChanged(object sender, object e)
        {
            currFileState = downloadOperation.State.ToString();
            if (currFileState == "Completed")
                MessageBox.Show("Completed");
        }
    }
champnic commented 2 years ago

Hey @HarriVa - I was unable to reproduce this using that CSV path as well. I wonder if maybe the "Source" isn't causing a navigation because it's navigating to the same location. Can you see if you reproduce the issue using webView.CoreWebView2.Navigate("https://filesamples.com/samples/document/csv/sample1.csv");?

darbid commented 2 years ago

@champnic I have now got this problem where neither StateChange nor EstimatedEndTime happens. Unfortunately I am using an intRAnet page so I cannot give you an example, I will post more if I can but there are 3 things which might (am not sure) be relevant.

  1. I think it happens where the file is very small.
  2. I also think it happens when the Operation does not have a value for TotalBytesToReceive
  3. related to 2 there is no Content-Length in the headers for example here is an example of the CoreWebView2WebResourceResponseView where the downloadstarting event fired but then nothing else.

    [Cache-Control, no-store]
    [Connection, keep-alive, Keep-Alive]
    [Content-Disposition, attachment; filename="Miscellaneous,_Note.msg"]
    [Content-Encoding, gzip]
    [Content-Security-Policy-Report-Only, default-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; style-src-elem 'self' 'unsafe-inline'; style-src-attr 'self' 'unsafe-inline'; img-src * data:;frame-src 'self' https://login.eu1.birst.com unydav: ms-excel: ms-powerpoint: ms-word: mailto:; frame-ancestors 'self'; report-uri /ipms/csp_report]
    [Content-Type, application/octet-stream;charset=UTF-8]
    [Date, Mon, 19 Sep 2022 12:28:18 GMT]
    [Expires, Thu, 01 Jan 1970 00:00:00 GMT]
    [Keep-Alive, timeout=10, max=100]
    [Server, Apache]
    [Strict-Transport-Security, max-age=31536000; includeSubDomains]
    [Transfer-Encoding, chunked]
    [Vary, Accept-Encoding,User-Agent]
    [X-Content-Type-Options, nosniff]
    [X-Frame-Options, SAMEORIGIN]
    [X-ServiceTime, D=29490 microseconds]
    [X-XSS-Protection, 1; mode=block]

Couple of other things.

The file is actually properly downloaded.

One other thing, which may be specific to my project, once an operation fails for a webview2 instance then a download of a another known file with a totally new instance of WebVie2 with a file that works, now no longer sets off the download starting event.

darbid commented 2 years ago

Another thing I am noticing is that the Download Operation properties TotalBytesToReceive and BytesReceived can have the same value (both can be zero) when reading them in the DownloadStarting event but it will have an InProgress state. ie. It instantly has received all the bytes, but shows an incorrect state and does not fire any events.

I am currently testing the assumption that if TotalBytesToReceive == BytesReceived even when they are zero then operation is complete. RESULT: NOPE Cannot do this. Sometimes the TotalBytesToReceive == BytesReceived, are zero but then the events fire and suddenly TotalBytesToReceive changes in a BytesReceived event.

Only thing I can say is that no events are fired SOMETIMES when TotalBytesToReceive == BytesReceived, however the values can be both zero or both the final amount.

darbid commented 2 years ago

@champnic I have now made a test app. TestApp Link

Unfortunately I cannot get the test website to do exactly the same as reported here. Where the download takes place in a new window I do not get the DownloadStarting event at all. I assume I have a timing/code error.

In my Test App I link to a page where you choose to have an attachment and for it to be in a NewWindow. In my test app I have a check box at the top. The choice is either to delay setting the source of the new window or not. As far as I am aware it is valid to set the Source url after await this.EnsureCoreWebView2Async(null);

You will see that if setting the source is delays then the DownloadStarting event is fired, if no delay then in most cases the Webview2 just navigates. In a few rare occasions in my testing the download is done and the default notification of download is shown.

Of course my actual project works if I also add a delay before the Source is set.

darbid commented 2 years ago

@HarriVa Based on my test can you try this in your code. Add in a await Task.Delay(500); before you add a url to Source.

await webView.EnsureCoreWebView2Async();
webView.CoreWebView2.DownloadStarting += CoreWebView2_DownloadStarting;
await Task.Delay(500);
webView.Source = new Uri("https://filesamples.com/samples/document/csv/sample1.csv");
Optimierungswerfer commented 2 years ago

I am also experiencing this issue. I am using the C++/WinRT projection of the WebView2 Core WinRT API. The SDK version is 1.0.1343.22.

It seems that none of the DownloadOperation events (BytesReceivedChanged, StateChanged, EstimatedEndTimeChanged) get fired at all regardless the circumstances. I have tried both small and large downloads. None of the event handlers I register inside the DownloadStarting event handler on the DownloadOperation object get executed.

I could also observe that at time of registering the event handlers on the DownloadOperation object, the DownloadOperation.State is InProgress and if I artificially introduce a few seconds long asynchronous wait and check DownloadOperation.State again, it is changed to Complete, but without the StateChanged event having been fired in the meantime. The files themselves download normally without issue.

This is a big problem, since I need to work with the file once it finishes downloading. It is a bit odd that there is no DownloadCompleted event in DownloadOperation to begin with, so you have to instead listen on the StateChanged event and check if State is now Completed.

darbid commented 2 years ago

@champnic 21 days ago I supplied an example of the error I see, is my code ok to illustrate the issue?

champnic commented 2 years ago

Hey @darbid - I don't think we've had a chance to revisit this issue yet, but thanks for the code and repro app.

@Optimierungswerfer Thanks for the extra details. We'll take a look at why the WinRT projection is running into this.

darbid commented 2 years ago

Hey @darbid - I don't think we've had a chance to revisit this issue yet, but thanks for the code and repro app.

@Optimierungswerfer Thanks for the extra details. We'll take a look at why the WinRT projection is running into this.

All good, just wanted to make sure it's on your todo list somewhere.

Optimierungswerfer commented 2 years ago

@champnic I didn't mean to say that the issue is specific to WinRT. If anything, wouldn't this rather indicate that the issue is not specific to one platform, since this thread was initially about .NET/Winforms? Personally I just did not try another platform than C++/WinRT.

champnic commented 2 years ago

It is likely that the issue is in the underlying COM implementation (which both the WinRT and .NET projections are built on) though you described some symptoms that seemed unique so far.

srwei commented 1 year ago

@darbid I tried to repro what you were seeing on the test app you posted (attachment + open in new window), and I'm still seeing the Operation_StateChanged output changed to Complete in the small and large files after downloading. I've tried it with both No delay and With task delay. Is there something I'm missing to repro so that the StateChanged doesn't get triggered?

darbid commented 1 year ago

in the small and large files after downloading. I've tried it with both No delay and With task delay. Is there something I'm missing to repro so that the StateChanged doesn't get triggered?

Nice that you have tested it out. I opened it back up and it actually showed correct output the first 3 times I did it with the PDF. I then looked at my code and saw my BeginInvoke in there. I have now updated my code with the BeginInvoke removed. I also believe that the small hello.txt file and dat repro 100% of the time if there is no delay. With the first pdf it has mixed results whereby most times it shows the events and 1 in 5 it may not. I am choosing attachement and new window.

If this still does not repro for you where do we go from here, Do I ask MS for a windows 10 refund? :-)

srwei commented 1 year ago

Went back trying to repro but was unable to. I'm thinking it's possible some sort of antivirus software or security settings may have caused the events to be raised extremely late, which may be why it's not 100% reproducible. Looking through your sample app when you were able to repro, did you see that the navigation completed as well as DownloadStarting event get hit as well? Or in the repro it also did not raise the DownloadStarting event despite the download completing?

HarriVa commented 1 year ago

@HarriVa Based on my test can you try this in your code. Add in a await Task.Delay(500); before you add a url to Source.

await webView.EnsureCoreWebView2Async();
webView.CoreWebView2.DownloadStarting += CoreWebView2_DownloadStarting;
await Task.Delay(500);
webView.Source = new Uri("https://filesamples.com/samples/document/csv/sample1.csv");

Sorry about the delay in getting back to this. Adding the delay as suggested seems to correct the problem in my case. I tried with different delay values, and in my test application a delay of 200 ms seems to be enough to avoid the problem. 100 ms is not enough.

darbid commented 1 year ago

What is not planned?

Optimierungswerfer commented 6 months ago

So I figured out why the StateChanged event did not fire in our case: After getting the DownloadOperation object from the DownloadStartingEventArgs to register the StateChanged event handler, you need to manually store the DownloadOperation reference to keep the object alive, because otherwise it would be destroyed when the reference goes out of scope of the DownloadStarting event handler, despite the download still running in the background and having a registered event handler. Effectively what happens is that calling the CoreWebView2DownloadStartingEventArgs::DownloadOperation method transfers the ownership to me. I find this highly unintuitive and this is documented nowhere. I should not need to manually keep alive an internal DownloadOperation object for a running download when there are registered event handlers on it. I want to register my event handler and forget about it. The plumbing necessary to keep alive these internal objects and delete them when the download finished to not leak memory serves absolutely no use. We essentially now keep a global std::list of these DownloadOperation objects and pass around iterators to manage their lifetime.

This StackOveflow question pointed me to what actually needed to be done.

srwei commented 4 months ago

Is this still reproable with the latest runtime? We made some changes to StateChanged and BytesReceivedChanged.

Optimierungswerfer commented 4 months ago

Thanks for working on this. Unfortunately, even with the latest stable runtime 125.0.2535.92 the StateChanged event handler does not get called when the DownloadOperation reference is not preserved.

srwei commented 3 months ago

@Optimierungswerfer I'm not able to get this to repro for the win32 sample app, I'm not seeing the DownloadOperation get destroyed. Are you doing some action that particular webview2 while the download is in progress? Do you happen to have a repro you can share?

Optimierungswerfer commented 3 months ago

I also was not able to reproduce the issue in the Win32 sample app using the Win32 API.

However, I did manage to reproduce the issue in this very basic project based on Visual Studio's MFC template project using WebView's WinRT API. I split the commits to the few steps required to reproduce, so the commit log should give a good overview. The final commit adds the issue demonstration. I hope this helps.

srwei commented 2 months ago

@Optimierungswerfer Thank you for putting this together. I was able to repro from your project. It looks like you did not capture the downloadOperation within the lambda for the StateChanged event handler. To ensure that the downloadOperation remains valid during the event and is reachable by the event handler, you will need to include it like this:

  m_pWebViewImpl->m_webView2.DownloadStarting(
    [this](auto&&, const CoreWebView2DownloadStartingEventArgs& args)
    {
      TRACE(_T("\n[Info] DownloadStarting! Registering broken StateChanged event handler...\n"));
      auto downloadOperation = args.DownloadOperation();
      downloadOperation.StateChanged([this, downloadOperation](auto&& sender, auto&& args) {
        TRACE(_T("\n[Info] Broken StateChanged event handler was called!\n")); // Does get executed this time!
      });
    });

This way, you won't need to store the downloadOperations in a global list. I can see it resolved locally for me. Can you try it yourself?

Optimierungswerfer commented 2 months ago

I have tried this before and got a memory leak. I just tried this again in the demo project and still get the memory leak. Capturing the download operation would be weird anyway, because the first argument to the lambda (sender) is a reference to the download operation. Somewhere in your documentation it is even explicitly said to avoid capturing the sender of events in lambdas and instead just use the provided sender parameter.