Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Funding Request - Release 63.0.0 #2234

Closed
26 tasks done
amaitland opened this issue Jan 17, 2018 · 34 comments
Closed
26 tasks done

Funding Request - Release 63.0.0 #2234

amaitland opened this issue Jan 17, 2018 · 34 comments

Comments

@amaitland
Copy link
Member

amaitland commented Jan 17, 2018

A big thank you to all those who had pledged funding to my Patreon campaign, unfortunately I was unable to resolve the matter with them, so have to look at other options. I've decided to give BountySource one more go, hopefully the change in ownership will resolve the support issues.

A revised set of details is below, I've already released the 63.0.0-pre01 set of packages as a thankyou for all those who pledged funding (even though I haven't received any money as yet).

Bountysource

Release 63.0.0-pre01 (Complete)

When 10% of target reached

  • Upgraded CEF version, no major features included
  • Add SwiftShader support (WebGL with GPU disabled), requires package changes, some cleanup required as well
  • JSB binding issue outlined in Javascript Bound objects no longer registered after cross-site navigation #1203 will be present, the issue text/content will be sanatized to clearly outlined the issue and outline the limited workarounds currently available
  • Review commits to master that weren't merged by myself, there are a few that need some follow up work
  • Branch master and prepare a Release branch as per previous releases.
  • Update changelog and create release notes
  • Release 63.0.0-pre01 packages to nuget.org
  • This set of packages won't resolve the JSB issue outlined above, it's simply for those who need an updated version of CEF and for testing purposes (will include all the changes to master since 57.0.0, assuming those changes pass code review)

Release 63.0.0-pre02 (Complete)

When 70% of target reached

  • Will include changes to restructure JavaScript binding(JSB) to work around Javascript Bound objects no longer registered after cross-site navigation #1203
    • A CefSharpSettings.LegacyJavascriptBindingEnabled option will be added that preserves the current behavior, this will only be viable for those users who limit their requests to one domain per browser instance, any cross-site navigation and the bound objects will no longer be automatically registered.
    • For the rewrite Bound objects will be registered upon request, this will be done in an async fashion, you will be able to Delete an object, defer object binding until later, register different objects in different frames/popups.
  • See more detailed description of issue at bottom of this document, I've included some basic samples from my working beta implementation.
  • Update changelog and create release notes
  • Release 63.0.0-pre02 set of packages to nuget.org

Release 63.0.0-pre03 (Complete)

When 90% of target reached

Release 63.0.0

When 100% of target reached

  • Bug fixes releaed to the JSB rewrites
  • Update changelog and create release notes
  • Update documentation to include details about changes to JSB
  • Release 63.0.0 pacakges to nuget.org

Support

Priority will be given to those individuals/companies who contribute directly to funding the new release.

JavaScript Binding(JSB) issue

See #1203 for the bug tracking this issue

Current implementation relies on process-per-tab command line flag to limit the number of browser process per ChromiumWebBrowser instance to one, this worked well up until version 58 where the flag no longer works. Chromium will likely re-add support, there is no timeframe on the open tickets as yet, see https://bugs.chromium.org/p/chromium/issues/detail?id=719961 and https://bugs.chromium.org/p/chromium/issues/detail?id=717459

A handler in the main process for notification of sub-process creation and termination, and to receive IPC messages originating from the sub-process.

CEF will likely add a handler that can be used for resolving this issue and maintain the exact same functionality as exists today without having to use process-per-tab command line see https://bitbucket.org/chromiumembedded/cef/issues/2279/expose-custom-sub-process-handlers

As both options have no set timeframe we can rewrite our currently implementation to work slightly differently, bound objects will need to be registered slightly later, so won't be available immediately after the V8Context has been created. To get around this I've changed how objects are registered, see the code below for some code excerpts.

//For legacy biding we'll still have support for
browser.RegisterJsObject("bound", new BoundObject(), BindingOptions.DefaultBinder);
var bindingOptions = new BindingOptions() 
{
	Binder = BindingOptions.DefaultBinder.Binder,
	MethodInterceptor = new MethodInterceptorLogger() // intercept .net methods calls from js and log it
};
browser.RegisterAsyncJsObject("boundAsync", new AsyncBoundObject(), bindingOptions);

//Ability to resolve an object, instead of forcing object registration before the browser has been initialized.
browser.JavascriptObjectRepository.ResolveObject += (sender, e) =>
{
	var repo = e.ObjectRepository;
	if (e.ObjectName == "boundAsync2")
	{
		repo.Register("boundAsync2", new AsyncBoundObject(), isAsync: true, options: bindingOptions);
	}
};
<script type="text/javascript">
(async function()
{
	// <embed user provided code here>
	await CefSharp.BindObjectAsync("boundAsync", "bound");
	
	boundAsync.hello('CefSharp').then(function (res)
	{
		assert.equal(res, "Hello CefSharp")
		asyncCallback();
	});
})();

(async () =>
{
	await CefSharp.BindObjectAsync("boundAsync", "bound");
	
	boundAsync.hello('CefSharp').then(function (res)
	{
		assert.equal(res, "Hello CefSharp")
		asyncCallback();
	});
})();

CefSharp.BindObjectAsync("boundAsync2").then(function(result)
{
	boundAsync2.hello('CefSharp').then(function (res)
	{
		assert.equal(res, "Hello CefSharp")
                // NOTE the ability to delete a bound object
		assert.equal(true, CefSharp.DeleteBoundObject("boundAsync2"), "Object was unbound");
		assert.ok(window.boundAsync2 === undefined, "boundAsync2 is now undefined");
		asyncCallback();
	});
});
</script>
@amaitland
Copy link
Member Author

#2157 and #2025 have been reviewed and merged already. Also basic support for dynamic was added to JavaScript binding in commit 7a0b06a

A number of other small improvements have been made, you can look through the commit history at https://github.com/cefsharp/CefSharp/commits/master if your interested.

@mitchcapper
Copy link
Contributor

If I understand correctly with LegacyJavascriptBindingEnabled RegisterJsObject will work as currently but has the current bug issue (no crosssite binding).
Without it enabled then
RegisterJsObject and RegisterAsyncJsObject will register at first available time (is that expected at page load complete or roughly when?)

When legacy mode is disabled then both page code and injected code should first call:
await CefSharp.BindObjectAsync("boundAsync", "bound");
to make sure the object is loaded before trying to call it.

There is also the new dynamic resolution option which is fairly straightforward.
Could I also recommend adding a .NET waiter too, like browser.EnsureObjectBound("objectName") that could be waited on in .net code before injecting code into the page (so that injected code does not need to be async and can be sure the bound object is loaded). Depending on the answer to the roughly when question above this may be un-necessary (as frameloadend or similar may be usable).

@amaitland
Copy link
Member Author

If I understand correctly with LegacyJavascriptBindingEnabled RegisterJsObject will work as currently but has the current bug issue (no crosssite binding).

Will work exactly the same as currently, objects will be bound correctly within the same render process. If you do happen to switch render processes, it'll be possible to do something like

if(window.boundAsync2 === undefined)
{
    CefSharp.BindObjectAsync("boundAsync2 ").then(function
    (
    )};
}

Without it enabled then
RegisterJsObject and RegisterAsyncJsObject will register at first available time (is that expected at page load complete or roughly when?)

Not sure I understand this one? Legacy mode will still bind objects when the V8Context is created, so they'll be available immediately in the global scope. The newer method objects will be bound on demand, soon as the V8 engine executes the method the process will get underway.

When legacy mode is disabled then both page code and injected code should first call:
await CefSharp.BindObjectAsync("boundAsync", "bound");
to make sure the object is loaded before trying to call it.

The call to CefSharp.BindObjectAsync will create a promise and then send an IPC message to the browser process requesting the bound object information, a response with the relevant info will be sent back over IPC and the promise will be resolve/rejected.

You'll actually be able to use CefSharp.BindObjectAsync when legacy mode is enabled, so you can mix and match in that scenario.

Could I also recommend adding a .NET waiter too, like browser.EnsureObjectBound("objectName") that could be waited on in .net code before injecting code into the page (so that injected code does not need to be async and can be sure the bound object is loaded)

So you'd like to be able to wait for a Task that's executed when the object has finished binding in javascript? If I'm understanding correctly it sounds like a responsible idea, let me have a think about how to implement that.

I'm not totally sold on any of the naming that I've gone with, so suggestions are welcome.

@amaitland
Copy link
Member Author

In relation to a BindingComplete notification in C# I'll add an event to the JavascriptObjectRepository, can add a TaskCompletionSource based wrapper around that for those that require an awaitable version.

@amaitland
Copy link
Member Author

I've created #2246 to track the JSB changes, also I've merged #2247 which includes my new JSB implementation.

Will merge into the cefsharp/63 branch shortly and release a new -pre release `ASAP.

@amaitland
Copy link
Member Author

Add basic example for WinForms SetAsChild to host browser in tab

An example of this will be added shortly, it will be based on https://gist.github.com/amaitland/fb2d14af2796aceb37e20cddad99459a

@amaitland
Copy link
Member Author

Is anyone interested in seeing the VC++ version upgraded to VC++2015? (Currently `VC++2013).

Please vote with a thumbs up to upgrade and a thumbs down if you would like things to stay the same.

@amaitland
Copy link
Member Author

I've created the 63.0.0-pre02 packages and they're on the MyGet feed.

https://www.myget.org/feed/cefsharp/package/nuget/CefSharp.WinForms
https://www.myget.org/feed/cefsharp/package/nuget/CefSharp.Wpf
https://www.myget.org/feed/cefsharp/package/nuget/CefSharp.OffScreen

The MinimalExample branch has been upgraded see https://github.com/cefsharp/CefSharp.MinimalExample/tree/cefsharp/63

There appears to be a small problem with the CefSharp.MinimalExample.WinForms project where the ChromiumWebBrowser isn't loading the default url. Problem was present in the -pre01 release as well, just totally forgot about it. The problem doesn't reproduce in the Tabbed example that's part of the main project and it's not reproducing in cefsimple native sample app. It's easy enough to work around, just load the Url after the browser has been initialized and all works correctly. A fix can be implemented in CefSharp it's self as a workaround, I'm still curious as to what's causing it.

Will investigate a little further before pushing the 63.0.0-pre02 packages to Nuget.org.

@amaitland
Copy link
Member Author

The changes implemented to support #2156 mean the Nuget package structure has changed quite a lot, I've tested all the standard scenarios, x86, x64 and AnyCpu. Everything appears to be working correctly, if you experience problems please comment on #2156 with as much information about what's happening, or what's not happening.

The unmanaged dependencies will now be copied when your using CefSharp in a library project (one level deep is all msbuild supports and publishing with ClickOnce should be much easier now. When you Clean Solution, or Clean a build the unmanaged dependencies will be removed, much nicer experience.

@amaitland
Copy link
Member Author

Add new sections to General Usage Guide

I've added some new sections to the General Usage Guide, starting at https://github.com/cefsharp/CefSharp/wiki/General-Usage#win32-out-of-memory

If anyone has suggestions on topic they'd like to see covered then comment below,

@chris-araman
Copy link
Contributor

chris-araman commented Jan 23, 2018

FWIW, VC++ 2017 would be fine with us as well. It would allow us to remove the VC++ 2015 payload from our install bundle, as we already use the 2017 runtime for our other binaries.

Good to see confirmation of the /LARGEADDRESSAWARE issue. We'd added this flag a while back on a hunch after seeing an increase in crashes that appeared to be due to out-of-memory conditions. Good to know we weren't off track.

@dandotevans
Copy link

What is the situation with #1983 . Will VC++2015 runtime be required?

@amaitland
Copy link
Member Author

What is the situation with #1983 . Will VC++2015 runtime be required?

VC++ 2015 would be required. It's possible to bin deploy the VC++ dlls rather than having to install it on every machine, only downside is you are then responsible for keeping the dlls up to date.

On reflection I'm thinking this upgrade probably should have happened before the first -pre release, so unless there's a strong objection I'll make the changes in master soon as a 3282 (version 64) build is available on http://opensource.spotify.com/cefbuilds/index.html

Upgrading the cefsharp/63 branch would be an unexpected change for anyone who has already installed the 63.0.0-pre01 packages.

So for clarity I'm proposing delaying this until vNext

Feedback welcome.

@chris-araman
Copy link
Contributor

RE: -pre
Sounds reasonable to me. Thanks, @amaitland.

@amaitland
Copy link
Member Author

I've pushed the 63.0.0-pre02 packages to Nuget.org, I have a workaround for the WinForms issue mentioned above details as follows.

The Url/address passed into the ChromiumWebBrowser instance isn't always being loaded, it's not happening in all cases, the CefSharp.WinForms.Example project works fine for the Tabbed browser, the MinimalExample which is just a single instance is failing to load.

See cefsharp/CefSharp.MinimalExample@562bfee for the changes I made to the MinimalExample project to workaround the problem.

If I'm unable to resolve the issue before the official 63.0.0 release then I'll build the workaround into CefSharp.

The -pre02 packages include the new Javascript Binding changes aka #2246 please comment on that issue if you have any questions/suggestions/problems.

@dandotevans
Copy link

pre02 resolves many issues for me. Only issue I have found is I am unable to see nested class bindings. sent more detail in a gitter pm.

@amaitland
Copy link
Member Author

pre02 resolves many issues for me. Only issue I have found is I am unable to see nested class bindings. sent more detail in a gitter pm.

I've resolved the binding issue and will release a new set of -pre packages hopefully tomorrow.

@amaitland
Copy link
Member Author

The 63.0.0-pre03 packages should appear on Nuget.org shortly.

The MinimalExample has been updated see https://github.com/cefsharp/CefSharp.MinimalExample/tree/cefsharp/63

@amaitland
Copy link
Member Author

Add basic example for WinForms SetAsChild to host browser in tab

An example of this will be added shortly, it will be based on https://gist.github.com/amaitland/fb2d14af2796aceb37e20cddad99459a

Example has been updated see commit fbce2d6

Code is currently commented out to preserve default behavior, can be set as default if required.

@amaitland
Copy link
Member Author

Upgrading the cefsharp/63 branch would be an unexpected change for anyone who has already installed the 63.0.0-pre01 packages.

So for clarity I'm proposing delaying this until vNext

master has been upgraded to CEF 3.3282.1731.gfc9a4fa / Chromium 64.0.3282.119, support for building with VS2013 has been removed, now VS2015 will be the default. (You should also be able to build locally with VS2017).

@amaitland
Copy link
Member Author

Anyone have any bugs to report for 63.0.0-pre03? Happy to take feedback on the method names also, if they're going to change then it needs to be now 😄

@chris-araman
Copy link
Contributor

We haven't found anything new yet, but we are still using the legacy binding mechanism because the new binding mechanism requires us to make some changes on the Javascript side.

@joewen
Copy link

joewen commented Feb 6, 2018

I have a question. From 57 to 63 -pre03 all I need to change for object binding is below?

// v57
browser.RegisterJsObject("external", new JSCallback(), new BindingOptions { CamelCaseJavascriptNames = false });

// v63
browser.JavascriptObjectRepository.Register("external", new JSCallback(), false, new BindingOptions { CamelCaseJavascriptNames = false });

I can't see my object from web dev tool console. window.external is not my object. Is there something I need to change also?

@amaitland
Copy link
Member Author

We haven't found anything new yet, but we are still using the legacy binding mechanism because the new binding mechanism requires us to make some changes on the Javascript side.

@chris-araman Thanks for the feedback.

I have a question. From 57 to 63 -pre03 all I need to change for object binding is below?

@joewen Details are #2246

@amaitland
Copy link
Member Author

I've generated the 63.0.0 packages and they're currently sitting on the MyGet feed, the following branches of the MinimalExample have been upgraded:

https://github.com/cefsharp/CefSharp.MinimalExample/tree/demo/anycpu
https://github.com/cefsharp/CefSharp.MinimalExample/tree/cefsharp/63

Will do a little more testing before releasing to Nuget.org, will post when they're up. I would usually like to wait a little longer before releasing the packages give everyone a chance to test a little more. The pending arrival of my first child (boy for those who are interested) means I don't quite have the luxury of time, could arrive any day. Once the dust settles will look at doing a 63.0.1 bug fix release if it's called for.

@chris-araman
Copy link
Contributor

Congratulations! 🍼🐥

@amaitland
Copy link
Member Author

The 63.0.0 packages have been pushed to Nuget.org, they should appear in the next hour or two.

@dandotevans
Copy link

Thats great. thank you for your efforts in updating the lib. I'd get some rest now whilst the coast is clear!

@cztomczak
Copy link

@amaitland A bit off-topic, in regards to patreon/bountysource. Have you considered using https://opencollective.com/ ?

@amaitland
Copy link
Member Author

Just an FYI I'll release a 63.0.2 set of packages shortly that will fix a few of the problems that have been reported.

@amaitland A bit off-topic, in regards to patreon/bountysource. Have you considered using https://opencollective.com/ ?

@cztomczak I've not heard of that one before, will have a look. Have you tried them personally?

@cztomczak
Copy link

@amaitland I just stumbled upon it recently and it looks good, the webpack project uses it. I haven't used it myself. And here is another one link you might find interesting, also found it just a few days ago: https://github.com/nayafia/lemonade-stand ("A handy guide to financial support for open source").

@amaitland
Copy link
Member Author

The General Usage Guide has been updated, see https://github.com/cefsharp/CefSharp/wiki/General-Usage#3-how-do-you-expose-a-net-class-to-javascript

Comments welcome. That's the last piece before I close this.

Will give it a few days for people to provide feedback on the General Usage Guide.

Thanks for everyone's support!

@amaitland
Copy link
Member Author

63.03 has been released, see https://github.com/cefsharp/CefSharp/releases/tag/v63.0.3 for details

@amaitland
Copy link
Member Author

Closing this issue now, if anyone has further feedback on the JSB General Usage Guide changes, then comments welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants