0

I'm creating an extension method for the WebBrowser control whose purpose is to wait for the page to have finished completely loading before returning.

By completely loading, I mean that one second has to have elapsed after the last DocumentCompleted event. This is to account for pages where multiple DocumentCompleted events are triggered during the loading of the page (which is something that's affecting my app currently).

I've written the current method which appears to work but I feel like could be improved: it returns a task when there's probably no need and also it suffers from the problem that an exception will be thrown if the application is closed down while it is waiting for the page to load.

    public static Task<bool> WaitLoad(this WebBrowser webBrowser, int wait)
    {
        var timerInternalWait = new Timer {Interval = 1000, Tag = "Internal"};
        var timerMaxWait = new Timer {Interval = wait};
        var tcs = new TaskCompletionSource<bool>();

        WebBrowserNavigatingEventHandler navigatingHandler = (sender, args) => timerInternalWait.Stop();
        webBrowser.Navigating += navigatingHandler;

        WebBrowserDocumentCompletedEventHandler documentCompletedHandler = (sender, args) => { timerInternalWait.Stop(); timerInternalWait.Start(); };
        webBrowser.DocumentCompleted += documentCompletedHandler;

        EventHandler timerHandler = null;
        timerHandler = (sender, args) =>
        {
            webBrowser.Navigating -= navigatingHandler;
            webBrowser.DocumentCompleted -= documentCompletedHandler;
            timerInternalWait.Tick -= timerHandler;
            timerMaxWait.Tick -= timerHandler;
            timerMaxWait.Stop();
            timerInternalWait.Stop();

            tcs.SetResult(((Timer) sender).Tag.ToString() == "Internal");
        };

        timerInternalWait.Tick += timerHandler;
        timerMaxWait.Tick += timerHandler;

        return tcs.Task;
    }

Is there something I can do to improve it?

JohnUbuntu
  • 699
  • 6
  • 19
  • Apparently there is some time between starting the Navigation and function WaitLoad subscribing to event WebBrowser.DocumentCompleted. What happens if event DocumentCompleted is raised before you subscribe to the event? – Harald Coppoolse Mar 17 '16 at 07:47
  • Check `NavigateAsync` from [here](http://stackoverflow.com/a/22262976/1768303). – noseratio Mar 17 '16 at 19:38
  • @HaraldDutch sort of right after I asked the question I realized that myself. And while a solution like what Noseratio proposed would help for the explicit navigates, what about a Click() and wait for the page to reload? So I'm going to create a wrapper for the WebBrowser control instead, that way the event subscriptions last for as long as the instance of the object does. – JohnUbuntu Mar 17 '16 at 22:52
  • @Noseratio, as mentioned in the comment above, your answer is only for navigating async, not for when, for example, a button is clicked which causes a new page to load. Also, I have to admit I'm not a big fan of the polling - if you poll it makes no sense to even use the `DocumentCompleted` at all. Besides a page might complete (issuing a `DocumentCompleted`), then issue a redirect and while the browser is reaching to the new server the DOM stays the same for more than 500ms, which using your method would incorrectly return as having completed the navigation. – JohnUbuntu Mar 29 '16 at 09:13
  • @Noseratio, in other words, the only way I've found to work around the issues in the comment above is to leverage both the `DocumentCompleted` and the `Navigating` events. Polling the DOM for changes will never work reliably. – JohnUbuntu Mar 29 '16 at 09:14
  • @JohnUbuntu, there's no perfect way of doing that. I explicitly mentioned that [here](http://stackoverflow.com/a/20934538/1768303). – noseratio Mar 29 '16 at 21:17

1 Answers1

0

The design guidelines state that asynchronous methods should be suffixed.

That timer is making your code harder to read. Try this instead:

public static Task WaitLoad(this WebBrowser webBrowser)
{
    var tcs = new TaskCompletionSource<object>();

    WebBrowserNavigatingEventHandler navigatingHandler = (sender, args) => timerInternalWait.Stop();
    webBrowser.Navigating += navigatingHandler;

    WebBrowserDocumentCompletedEventHandler documentCompletedHandler = (sender, args) => tcs.SetResult(null);

    try
    {
        webBrowser.DocumentCompleted += documentCompletedHandler;

        await tcs.Task;
    }
    finally
    {
        webBrowser.DocumentCompleted -= documentCompletedHandler;

        await Task.Delay(1000);
    }

    return tcs.Task;
}

public static Task<bool> WaitLoad(this WebBrowser webBrowser, int timeout)
{
    var webBrowserTask = webBrowser.WaitLoad();

    return await Task.WhenAny(webBrowserTask, Task.Delay(timeout)) == webBrowserTask;
}

But you should look into cancellation tokens instead of double timers.

Paulo Morgado
  • 14,111
  • 3
  • 31
  • 59
  • this doesn't quite achieve the same as what I had though. It only waits until the DocumentCompleted event is fired once, and then has a static wait of 1sec before returning. The problem is that the DocumentCompleted can fire multiple times. That was the reason for the 1 sec timer... – JohnUbuntu Mar 18 '16 at 11:40
  • It's been a while since I worked with the WebBrowser control, but I recall that. Picking up my code, you can put the delays where needed without managing timers. – Paulo Morgado Mar 18 '16 at 16:17
  • Sure, but then the question is... why even subscribe to the events at all? Could just use the IsBusy property on a while with a `Task.Delay(500);` in between. – JohnUbuntu Mar 18 '16 at 21:23
  • On another note, specially bearing in mind that this is the only answer to the question, if you want to me to potentially mark the answer is the right one you'll need to edit your code to do what I'm actually asking. You mentioned it's easy to pick your code and modify to do what I asked, then please submit it as an answer and if it does as expected I'll mark is as correct. – JohnUbuntu Mar 29 '16 at 09:58