cloudtrends / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
1 stars 1 forks source link

Break out cef.h into multiple headers #142

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It seems a bit unwieldy to have to include all of the irrelevant classes in 
cef.h, if for instance, you just want to implement CefDownloadHandler or a new 
version of CefBase/CefSchemeHandler/etc... for some reason.

Would it be alright to separate these into separate headers? cef.h would 
probably still be the only main required header, but I'd like to implement my 
own instances of these objects without having to include cef.h as well. 

Thanks.

Original issue reported on code.google.com by udam.sa...@gmail.com on 5 Nov 2010 at 11:22

GoogleCodeExporter commented 9 years ago
I've been thinking about this also.  Before breaking up cef.h we'll need to 
answer the following questions:

1. How do we indicate which API (C++ vs C vs wrapper) certain classes belong 
to?  Do we name files in a certain way, use subdirectories, etc?
2. How do we group classes in headers? Does each class get it's own header, are 
related classes get grouped together, etc?
3. Do we break cef_capi.h into separate headers? 

Original comment by magreenb...@gmail.com on 5 Nov 2010 at 11:55

GoogleCodeExporter commented 9 years ago
This is just what I feel personally.

1). Id rather use subdirectories for C++ vs C vs wrapper just to help reduce 
the clutter in directories.
2) I'd rather have 90% of classes in separate header files. Only really small 
related classes should probably be grouped together. (CefBase is pretty small, 
so having that grouped with the ThreadSafeBase class probably makes sense). 
3). Do we know how extensive the C API use is? Should that be a separate 
endeavor?

Original comment by udam.sa...@gmail.com on 8 Nov 2010 at 7:31

GoogleCodeExporter commented 9 years ago
I agree with #1 and #2. For #3, the C API is required for passing data across 
the DLL boundary. See issue #1 for details.

Original comment by magreenb...@gmail.com on 16 Nov 2010 at 4:54

GoogleCodeExporter commented 9 years ago

Original comment by magreenb...@gmail.com on 23 Nov 2011 at 11:49

GoogleCodeExporter commented 9 years ago
Done in revision 442.

Original comment by magreenb...@gmail.com on 23 Dec 2011 at 5:39

GoogleCodeExporter commented 9 years ago

Original comment by magreenb...@gmail.com on 23 Dec 2011 at 5:39

GoogleCodeExporter commented 9 years ago
Would it not make sense to include an umbrella header for backwards 
compatibility?

Original comment by j...@kirkconsulting.co.uk on 24 Jan 2012 at 2:06

GoogleCodeExporter commented 9 years ago
@comment#7: I wouldn't want to add an umbrella file in the tree (because that 
would encourage misuse) but we could potentially generate it for the binary 
releases. If we started providing an umbrella file when would a good time/goal 
to stop providing it?

Original comment by magreenb...@gmail.com on 24 Jan 2012 at 2:59

GoogleCodeExporter commented 9 years ago
I would say its a choice right?  You have made it possible for people to just 
include a subset but it doesnt hurt to add the umbrella.  In our case I'm 
switching between trunk and older versions and I need to doctor my code to do 
this because the headers differ completely.

Original comment by j...@kirkconsulting.co.uk on 24 Jan 2012 at 3:01

GoogleCodeExporter commented 9 years ago
@comment#9: Including an umbrella header in trunk gives people the impression 
that using the umbrella in trunk CEF code is OK, which it is not. Adding a tool 
for generating the umbrella header that could then be called stand-alone and/or 
executed as part of the release build process seems reasonable.

Original comment by magreenb...@gmail.com on 24 Jan 2012 at 3:06

GoogleCodeExporter commented 9 years ago
Yeah. I'm having a lot of troubles to get the headers right now. We really miss 
a good documentation that tells us what headers to include to get what result. 
And there are some cross headers missing. For example, including cef_scheme.h 
required CefBrowser class to be defined but does not containg a "#include 
"cef_browser.h" statement. That means you have to include both together to get 
it working... The same observation goes for CefFrame...

I believe the umbrella file is needed at least until each include provides the 
full symbols needed by the functionality it names.

Original comment by raynaudq...@gmail.com on 1 Feb 2012 at 4:12

GoogleCodeExporter commented 9 years ago
@comment#11: Many headers cannot include cef_frame.h or cef_browser.h because 
that would cause header include recursion. CEF takes the approach of using 
forward class declarations instead of header includes wherever possible. 
Determining what to include from your code is quite simple in most cases -- if 
you see a forward declaration for a class then you need to include the header 
for that class. For example, if you see "class CefBrowser" then you need to 
include cef_browser.h.

Discussion of an umbrella header has been moved to issue 487.

Original comment by magreenb...@gmail.com on 1 Feb 2012 at 4:38

GoogleCodeExporter commented 9 years ago
Revision 506 changes the forward class declarations into includes.

Original comment by magreenb...@gmail.com on 17 Feb 2012 at 5:33

GoogleCodeExporter commented 9 years ago
Thanks a lot, this one will really ease a lot everything! You're a life saviour!

Original comment by raynaudq...@gmail.com on 17 Feb 2012 at 5:58