Skip to content

Conversation

@bjarteskogoy
Copy link

No description provided.

@bjarteskogoy bjarteskogoy mentioned this pull request Sep 24, 2014
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented out Contract

@amaitland amaitland added the cef3 label Sep 28, 2014
@amaitland amaitland added this to the 33.0.0 milestone Sep 28, 2014
@amaitland
Copy link
Member

I'm wondering if it's worth detecting the .Net framework version and using the Begin/End approach as a fallback option.

As per http://msdn.microsoft.com/en-us/library/hh925568%28v=vs.110%29.aspx we can check the registry to see what version is installed. (There is a fairly simple example near the bottom of the page)

Idea raises a couple of questions

  • Am I over thinking things?
  • Does checking the registry require any additional security rights that previously weren't required?
  • Should KISS be applied?

Thoughts?

@bjarteskogoy
Copy link
Author

Would there be any benefits of the Task<> approach over Begin/End performance-wise?
If not I don't see any good reasons for that, as long as the developer anyway is exposed to the methods returning Tasks; https://github.com/bjarteskogoy/CefSharp/blob/JsBinding_WIP/CefSharp.Wpf/ChromiumWebBrowser.cs#L1133 and https://github.com/bjarteskogoy/CefSharp/blob/JsBinding_WIP/CefSharp.Wpf/ChromiumWebBrowser.cs#L1138.

@amaitland
Copy link
Member

Unsure about the perf benefits, it just seems a step backwards! It just offends my sense of aesthetics! lol

I'm probably over thinking it 😄

@bjarteskogoy
Copy link
Author

:)

@peters
Copy link
Contributor

peters commented Oct 1, 2014

@amaitland I agree, it looks awful :(

@JanEggers
Copy link
Contributor

hi guys why do we need this ?

task is .net 4.0 compatible you just need to use continue on the task instead of await.

did i miss something ?

@amaitland
Copy link
Member

@JanEggers Apparently it's a Task/WCF thing in pure .Net 4.0. The error reported is in #490

"Unhandled exception with message Type 'System.Threading.Tasks.Task`1[CefSharp.JavascriptResponse]' cannot be serialized. Consider marking it with the DataContractAttribute attribute, and marking all of its members you want serialized with the DataMemberAttribute attribute. If the type is a collection, consider marking it with the CollectionDataContractAttribute. See the Microsoft .NET Framework documentation for other supported types.. "

More suggestions on solutions are most welcome 😄

@JanEggers
Copy link
Contributor

ouch i wasnt aware of that.

no idea on my side

@amaitland
Copy link
Member

We can still use Task, just have to wrap old style Begin/End Async calls on the WCF interface.

Looks ugly, it's a viable solution though. Can always improve it later if someone comes up with a brilliant idea.

@amaitland
Copy link
Member

I have reviewed this, I'll make one minor formatting change after it's merged, apart from that thanks for the fantastic contribution @bjarteskogoy merging now 😄

amaitland added a commit that referenced this pull request Oct 1, 2014
Use Begin/End pattern to not break .Net 4 compatibility.
@amaitland amaitland merged commit 9c6a1dc into cefsharp:JsBinding_WIP Oct 1, 2014
@jornh jornh modified the milestones: 33.0.0, 33.0.0-pre3 Oct 3, 2014
@amaitland amaitland modified the milestones: 33.1.0-pre, 33.0.0 Oct 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants