Resolve: IsLoaded is unreliable

[amaitland]

http://magpcss.org/ceforum/apidocs3/projects/%28default%29/CefLoadHandler.html#OnLoadStart%28CefRefPtr%3CCefBrowser%3E,CefRefPtr%3CCefFrame%3E%29

For notification of overall browser load status use OnLoadingStateChange instead.

Initially just migrating WPF control. When it's configured, update the other two flavours.

[amaitland]

When complete this will fix #909

[rassilon]

I can push this through WinForms separately if you like.
Looks like it might be the right thing, but I haven't done much WPF programming.

Bill

[rassilon]

Oh, If we do this push this through, we should add a depreciated attribute to SetIsLoading or something similar.

Bill

[amaitland]

Oh, If we do this push this through, we should add a depreciated attribute to SetIsLoading or something similar.

Hmm, yeah maybe making the IsLoadingChanged event as deprecated is the way to go for now. For WPF it makes sense to have a bind able property. For the others I think removing the event is better long term, people can still subscribe to FrameLoadStart and FrameLoadEnd which is would give the same output if absolutely nessicary. Otherwise NavStateChanged seems to be the way to go.

Looking at OnLoadStart and OnLoadEnd, I wonder if we still need the AutoLock lock_scope(_syncRoot);? I'm not actually sure what their purposes is. For all three flavors, they simply fire events, so I wouldn't have though locking was necessary.

So I think the short term question is, do we remove IsLoadingChanged from WinForms and OffScreen or mark them as deprecated?

[amaitland]

If we remove IsLoadingChanged then NavStateChanged could be renamed to LoadingStateChanged to bring it inline with the CEF Api. Thoughts?

[rassilon]

I think the lock_scope(_syncRoot) bit might be trying to prevent the users event callbacks being called simultaneously on two different threads.

Well, at a minimum depreciated I think, although if you're ok with the renaming to LoadingStateChanged for 41.x that's fine by me, it certainly is a cleaner naming.

If we were going to rename in 41.x I think it would be ok to depreciate in 39.0.1 (or whatever) if we wanted.

Bill

[amaitland]

I think the lock_scope(_syncRoot) bit might be trying to prevent the users event callbacks being called simultaneously on two different threads.

As they basically just fire events, I wonder if that's necessary? My initial though would be that locking maybe slowing things down, by how much I'm not sure.

If we were going to rename in 41.x I think it would be ok to depreciate in 39.0.1 (or whatever) if we wanted.

Sounds like a plan 👍

[rassilon]

I don't know if the lock is necessary or not, its certainly nicer to the consumer of CefSharp if you can have the two events fire on different threads from CEF and not have to care about it at the level of CefSharp's users.

[amaitland]

I think if we remove the call to SetIsLoading which would be part of this PR then it should be safe enough. If we change in 2272 early enough then hopefully any issues will arise before the release of 41.0.0

[amaitland]

#931 has changes for 2272 branch.

Just a matter of making the IsLoadingChanged as deprecated.