Skip to content

Conversation

@amaitland
Copy link
Member

Includes a few changes around Resource Handling

  • Split GetResourceHandler out of IRequestHandler into it's own IResourceHandler interface.
  • Also include a DefaultResourceHandler implementation, so LoadHtml can easily be replaced with a ResourceHandler
  • Update WPF and WinForms examples to reflect the changes.
  • Add license header to CefTerminationStatus (unrelated commit)

@amaitland amaitland added this to the 37.0.0 milestone Dec 9, 2014
@amaitland
Copy link
Member Author

@jankurianski I decided to extract Resouce handling into it's own IResourceHandler interface, so it's easy for users to hook into and override functionality if required. I also replaced LoadHtml.

Thoughts?

@jankurianski
Copy link
Member

Cool! Taking a look now.

@jankurianski
Copy link
Member

Works great. I tested it out by updating the CefSharp.OffScreen.Example:
https://gist.github.com/jankurianski/e0ac2d1006f3a42216be

The only potential issue could be that UnregisterHandler is never called, but it is hard to know the right time to do this, and as long as the url is distinct it won't matter.

@jankurianski
Copy link
Member

Actually, there is a bit of a problem: if you call LoadHtml() twice with the same url, it crashes because the resource handler already exists.

@jankurianski
Copy link
Member

Perhaps LoadHtml() should simply unregister any existing handler first?

            handler.UnregisterHandler(url);
            handler.RegisterHandler(url, CefSharp.ResourceHandler.FromString(html));

…hrow and exception

All user to specify Comparer for dictionary
@amaitland
Copy link
Member Author

The only potential issue could be that UnregisterHandler is never called, but it is hard to know the right time to do this, and as long as the url is distinct it won't matter.

Users can always manually unregister the Url if required.

Actually, there is a bit of a problem: if you call LoadHtml() twice with the same url, it crashes because the resource handler already exists.

Nice pickup, thanks! 👍

@amaitland
Copy link
Member Author

Commit b104cd7 should fix the multiple registration bug

@jankurianski
Copy link
Member

Commit b104cd7 should fix the multiple registration bug

That should do the trick! 👍

@amaitland
Copy link
Member Author

@Jayman1305 As you submitted a PR related to ResourceHandler I though I'd give you a heads up about this PR.

@amaitland
Copy link
Member Author

@jornh Do you think a -pre03 package is necessary? I'd like to change the API before officially releasing 37.

@jornh
Copy link
Contributor

jornh commented Dec 9, 2014

@amaitland No let's go straight for the 37 👍

amaitland added a commit that referenced this pull request Dec 9, 2014
@amaitland amaitland merged commit ae8c153 into cefsharp:master Dec 9, 2014
@amaitland amaitland deleted the feature/rework-resourcehandler branch December 9, 2014 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants