SoC14 wxWebViewChromium merge #15

Open
wants to merge 133 commits into
from

Projects

None yet

8 participants

@sjlamerton
Contributor

First attempt at a merge of the 2014 wxWebViewChromium Summer of Code work. Straight rebase of the Haojian's changes onto wxWidgets master. Conflicting commits were skipped as they related to build files that were later removed. One additional commit to fix the msw build files after their recent changes.

@vadz vadz and 1 other commented on an outdated diff Sep 6, 2014
interface/wx/webview_chromium.h
+ - Reuqire link frameworks: Chromium Embedded Framework.framework, AppKit.frameworks.
+ - Reuqire app bundle configuration: Info.plist
+ - Use system tool `install_name_tool -change` to correct `Chromium Embedded Framework.framework/Chromium Embedded Framework` location.
+ 3. Compiled/link/package the `webviewchromium` app:
+ - Require `webview.cpp`
+ - Reuqire link frameworks: Chromium Embedded Framework.framework, AppKit.frameworks.
+ - Reuqire app bundle configuration: Info.plist
+ - Use system tool `install_name_tool -change` to correct `Chromium Embedded Framework.framework/Chromium Embedded Framework` location.
+ 4. Create a `Contents/Frameworks` directory in `webviewchromium.app` bundle and copy `helper.app`, `webviewchromium.dylib`, 'webview.dylib'
+ and `Chromium Embedded Framework` in it.
+ 5. Use `samples/webview_chromium/mac/tools/make_more_helper.sh` to make sub-process helper app bundles based on `helper` app.
+
+
+ Below is the wxWebviewChromium sample app bundle directory structure:
+
+ webview_chromium.app
@vadz
vadz Sep 6, 2014 Contributor

Does this appear correctly in the Doxygen output? I'd expect it to be necessary to use @verbatim/@endverbatim tags around this tree.

@hokein
hokein Sep 7, 2014

I use the code fragment to show this directory tree here. See the output result.

@vadz vadz commented on an outdated diff Sep 6, 2014
interface/wx/webview_chromium.h
+ `-- wxmac.icns <= resources.
+
+
+**/
+class wxWebViewChromium : public wxWebView
+{
+public:
+ wxWebViewChromium(wxWindow* parent,
+ wxWindowID id,
+ const wxString& url = wxWebViewDefaultURLStr,
+ const wxPoint& pos = wxDefaultPosition,
+ const wxSize& size = wxDefaultSize,
+ long style = 0,
+ const wxString& name = wxWebViewNameStr);
+
+ void OnSize(wxSizeEvent &event);
@vadz
vadz Sep 6, 2014 Contributor

What's the point of having these methods in the documentation header if they're not overloaded nor documented anyhow? They should be just omitted IMO.

@vadz vadz commented on an outdated diff Sep 6, 2014
interface/wx/webview_chromium.h
+ | |-- PkgInfo
+ | `-- Resources
+ | `-- wxmac.icns
+ |-- Info.plist
+ |-- MacOS
+ | `-- webview_chromium <= webviewchromium sample executable
+ |-- PkgInfo
+ `-- Resources
+ `-- wxmac.icns <= resources.
+
+
+**/
+class wxWebViewChromium : public wxWebView
+{
+public:
+ wxWebViewChromium(wxWindow* parent,
@vadz
vadz Sep 6, 2014 Contributor

Ctor (and Create()) should be documented, even if just to say that they take the same parameters as their base class equivalents.

@vadz
Contributor
vadz commented Sep 6, 2014

We don't have separate wx_vc1[012]_*.vcxproj files in the trunk any more, so the projects should be merged and be made to resemble the existing ones, i.e. contain

  <ImportGroup Label="PropertySheets">
    <Import Project="wx_setup.props" />
    <Import Project="wx_local.props" Condition="Exists('wx_local.props')" />
  </ImportGroup>

fragment. They should also be added to the solution (.sln) file(s).

@hokein
hokein commented Sep 7, 2014

@vadz, can you explain more about how to update the vs project here? From my understanding, we need to remove the wx_vc1[012]_webviewchromium.vcxproj files and replace them with wx_webviewchromium.vcxproj file like the wx_webview.vcxproj in trunk.

Besides, how does the vcxproj files update in trunk?

@vadz
Contributor
vadz commented Sep 7, 2014

@hokein yes, this is correct, I said "merged" and not "removed and replaced" because the new vcxproj files are very similar to the old ones, just look at the diff between the build/msw/wx_base.vcxproj in master and 946fbf4d1430e5ebfd0cab54b68d035ed2ece66d~:build/msw/wx_vc12_base.vcxproj for example. Just do the same changes to this project in a text editor -- and test that it still works.

@hokein
hokein commented Sep 7, 2014

@vadz, Thanks for your explaination.
@steve-lamerton, could you please add me in your repository so that I have write priviledge?

@hokein
hokein commented Sep 10, 2014

I have updated the vcxproj files.

@sjlamerton
Contributor

On 10 September 2014 05:16, Haojian Wu notifications@github.com wrote:

I have updated the vcxproj files.

Thanks for updating the project files, I will update the interface files
over the next couple of days and try and get the merge done by the end of
the weekend.

Steven

@TcT2k TcT2k commented on the diff Sep 15, 2014
samples/webview_chromium/webview.cpp
+ wxCommandEventHandler(WebFrame::OnSelectAll), NULL, this );
+ Connect(loadscheme->GetId(), wxEVT_COMMAND_MENU_SELECTED,
+ wxCommandEventHandler(WebFrame::OnLoadScheme), NULL, this );
+ Connect(usememoryfs->GetId(), wxEVT_COMMAND_MENU_SELECTED,
+ wxCommandEventHandler(WebFrame::OnUseMemoryFS), NULL, this );
+ Connect(m_find->GetId(), wxEVT_COMMAND_MENU_SELECTED,
+ wxCommandEventHandler(WebFrame::OnFind), NULL, this );
+ Connect(m_context_menu->GetId(), wxEVT_COMMAND_MENU_SELECTED,
+ wxCommandEventHandler(WebFrame::OnEnableContextMenu), NULL, this );
+
+ //Connect the idle events
+ Connect(wxID_ANY, wxEVT_IDLE, wxIdleEventHandler(WebFrame::OnIdle), NULL, this);
+ Connect(wxID_ANY, wxEVT_CLOSE_WINDOW, wxCloseEventHandler(WebFrame::OnClose), NULL, this);
+ Connect(wxID_ANY, wxEVT_TIMER, wxTimerEventHandler(WebFrame::OnTimer), NULL, this);
+
+ m_timer = new wxTimer(this);
@TcT2k
TcT2k Sep 15, 2014 Contributor

Couldn't this timer be integrated in the backend?
I don't see why the required wxWebViewChromium::DoCEFWork() couldn't just be done by an internal timer in the backend.

@danielgindi

Guys there's a serious problem here. On every page load, you automatically request GetSource and GetText of the document, to store for viewing source.
This can be a huge load on the browser for absolutely no reason at all!
It should be done lazily on request, with a callback to supply the user with the requested data.

@sjlamerton
Contributor

I believe that this was done to allow the current wxWebView API to work, which only supports the synchronous request of page source and page text. Is this correct @hokein ?

@hokein
hokein commented Oct 8, 2014

Yes.
It aims to support current wxWebView API since the GetSource and GetText API are synchronous while in CEF they are async. Storing the source&text is the only way I figure out to cross the gaps.

@sjlamerton sjlamerton referenced this pull request in sjlamerton/wxWebViewChromium Nov 10, 2014
Closed

This project is dead? #16

hokein and others added some commits May 11, 2014
@hokein @sjlamerton hokein Integrate wxWebChromium to wxwidgets
Also add webview_chromium usage samples.
0c75942
@hokein @sjlamerton hokein Remove CEF1 specific code since we only focus on CEF3. 0042d0c
@hokein @sjlamerton hokein Send WEBVIEW_ERROR event after CEF finishing loading.
Currently, WEBVIEW_ERROR event is sent to client on CEF::OnLoadError(),
which may cause the error message of wxInfoBar disapear.

CEF::OnLoadEnd will be invoked after CEF::OnloadError, if WEBVIEW_ERROR
event get processed before CEF::LoadEnd, the wxInfoBar's message will
disappear since the webpage will get loaded in CEF::LoadEnd.

CEF will return "data:text/html,chromewebdata" when url is invalid.
9e68dcf
@hokein @sjlamerton hokein Nits: fix code style. c5368af
@hokein @sjlamerton hokein Specify the link to Debug/Release CEF library. 731428a
@hokein @sjlamerton hokein Nits: Fix code style. 7eb890a
@hokein @sjlamerton hokein Add Building Instructions Document. fae28cf
@hokein @sjlamerton hokein Remove unnecessary subprocess.exe during webview startups. c5e62d9
@hokein @sjlamerton hokein Correct building link to CEF library. 9415d43
@hokein @sjlamerton hokein Update readme.md 23aa20c
@hokein @sjlamerton hokein Add webview_chromium.bkl to samples.bkl 0fc659b
@hokein @sjlamerton hokein Enable wxWebviewChromium dynamic builds. 19e6ec0
@hokein @sjlamerton hokein Move CEF specific variables to config.bkl to avoid duplicate. 08a6b0d
@hokein @sjlamerton hokein Add a CEF_INCLUDE_DIR check in webview_chromium sample. 5ad9c9b
@hokein @sjlamerton hokein Add CEF link path on Linux. 1cb6399
@hokein @sjlamerton hokein Remove lib prefix in multilib.bkl
There is no need to add lib prefix, since g++ `-l` option will add it
automatically.

This patch does no effect on windows platform.
34d2dcc
@hokein @sjlamerton hokein Split CEF_LIB_DIR into LIBCEF_PATH and LIBCEF_WRAPPER_PATH. 1f8ff10
@hokein @sjlamerton hokein Add webview_chromium sample in configure.in
Fix configure can not generate Makefile in samples/webview_chromium directory.
e82ecb1
@hokein @sjlamerton hokein Correct link path in webview_chromium sample. 996ad53
@hokein @sjlamerton hokein Correct cef link name on Windows.
Linux link option does not require 'lib' prefix, while windows
require.
922cfd6
@hokein @sjlamerton hokein Integrate CEF MessageLoop to wxWidgets MessageLoop.
* Enable webview_chromium on Linux, webview_chromium runs normally,
  but browser window is split out of wxWidget window now and we need to
  host it in wxWidgets window later.
* Make cef messageloop processing in wx::ProcessIdle period, this way
works on windows and linux.
* Don't use multiple_message_loop on Windows.
1854254
@hokein @sjlamerton hokein Fix autoconf generated error in bakefile_gen. 9b0cfd9
@hokein @sjlamerton hokein Fix some macro mistake issues. 79834c4
@hokein @sjlamerton hokein Fix build error on windows. 6785d28
@hokein @sjlamerton hokein Set cef messageloop running inside wxWebviewChromium instead of client
side.
b82a448
@hokein @sjlamerton hokein Set wxWidgets_src/cef as CEF default path. 5e7c075
@hokein @sjlamerton hokein Update readme. 1f302ea
@hokein @sjlamerton hokein Host CEF browser window in wxWidgets window. ac44ff4
@hokein @sjlamerton hokein Enable CEF debug log. 0470cde
@hokein @sjlamerton hokein Remove CEF message loop running to client.
Running CEF message loop inside wxWebviewChromium internal will get
segument fault, while moving to client doesn't.

Reset to the previous client solution to avoid segument falut, since I
don't figure out the segument fault reason.
92b0887
@hokein @sjlamerton hokein Fix argument inconsistency issue. a96ab5e
@hokein @sjlamerton hokein Fix windows build error. 0efa20f
@hokein @sjlamerton hokein Cleanup: remove unused m_timer. 568de3a
@hokein @sjlamerton hokein Add more building instruction in readme. 8e13d95
@hokein @sjlamerton hokein Fix gtk_scrolled_window_add() warning.
Using gtk viewport instead of vbox, to avoid the following waring: "
Gtk: gtk_scrolled_window_add(): cannot add non scrollable widget use
gtk_scrolled_window_add_with_viewport() instead".
e987c60
@hokein @sjlamerton hokein Fix rename issue. 8843b71
@hokein @sjlamerton hokein Add webview xcode project file.
Copy and modify from samples/minimal/minimal_cocoa.xcodeproj
d0963ba
@hokein @sjlamerton hokein Use double quotes instead of angle brackets to include wx header files. 35ec62d
@hokein @sjlamerton hokein Cleanup: remove StartUpSubprocess() since it no longer used. 7b94223
@hokein @sjlamerton hokein Add launch helper for executing separate processes on mac OS X. b31f600
@hokein @sjlamerton hokein Upgrade to CEF3.1750.1738. 4dd5e41
@hokein @sjlamerton hokein Enable webview_chromium building through xcode on mac OS X. c6cafef
@hokein @sjlamerton hokein [Mac] Host cef browser window to wxwidgets windows. 2abb025
@hokein @sjlamerton hokein [Mac] Enable webview_chromium dynamic builds. 1a495df
@hokein @sjlamerton hokein [Mac] Rename webview_chromium helper to xcode project. 0306424
@hokein @sjlamerton hokein [Mac] Correct dynamic dependent project. 7cdab5f
@hokein @sjlamerton hokein [Mac] Fix find bar is overrided by webview window issue. 22f50a1
@hokein @sjlamerton hokein [Mac] Resize browser window. 7967612
@hokein @sjlamerton hokein Fix a crash issue when close wxwebview_chromium sample.
On Mac OS X, it will trigger a crash issue when user close the window
(It also appear in other platforms, but the crash doesn't appear.)

The crash is caused by calling `CefShutdown` in wxApp::OnExit(). When
wx application running in wxApp::OnExit function, the webframe has been
deleted, we need to call `CefShutdown` in webframe `Close` period.
bfcb34a
@hokein @sjlamerton hokein Remove unnecessary code. 279cbd4
@hokein @sjlamerton hokein [Win] Add VS2010 wxWebviewChromium sample static build supports. 9523947
@hokein @sjlamerton hokein [Win] Add VS2010 wxWebviewChromium sample dynamic build supports. dafc999
@hokein @sjlamerton hokein Update readme.md
Add `libcef_dll_wrapper` building steps for wxWidgets.
29e6b84
@hokein @sjlamerton hokein Update Readme.md. 19e8975
@hokein @sjlamerton hokein Add Mac OS X building tutorial. 366753b
@hokein @sjlamerton hokein Disable sandbox in CEF3.1750.1738. 267f7ef
@hokein @sjlamerton hokein Use Connect method instead of EVENT_TABLE to hook WebFrame::OnClose e…
…vent.
6587f7c
@hokein @sjlamerton hokein [Win/Linux] A temporary fix crashing when closing WebFrame. 511dd8f
@hokein @sjlamerton hokein Add custom handler support. 0886ae6
@hokein @sjlamerton hokein Correct help/doc.zip loading path.
Due to webview sample building directory is different from platforms and
compilers, we need to retrieve the `<wxwidgets-src>/help/` absolute path
from building directory.
2cdc1a2
@hokein @sjlamerton hokein Nits: Correct code style. 89d7f5c
@hokein @sjlamerton hokein [Mac] Add MimeType empty check.
wxFileSystemHandler::GetMimeTypeFromExt returns empty when
passing css location url on Mac OS X, which will trigger DCHECK failure in CEF.

This patch is adding empty check to avoid DCHECK failure.
054c2a8
@hokein @sjlamerton hokein [Mac] Fix a warning: '_' is not allowed in bundle file. 22c497f
@hokein @sjlamerton hokein Nits: Fix some compile warnings. 5906cfb
@hokein @sjlamerton hokein Update readme: Upgrade to CEF3.1750.1738. df97bbe
@hokein @sjlamerton hokein Rename RunCEFMessageLoopOnIdle to DoCEFWork for better understanding. 094eea1
@hokein @sjlamerton hokein Running CEFMessageLoop in wxTimer instead of OnIdle.
Running CEFMessageLoop form OnIdle, which may pontentially starve CEF
messageloop when application is busy with other UI-operation.

Change it to timer to avoid the starvation.
543559a
@hokein @sjlamerton hokein Restore OnExit method.
Althrough there is fine to exit the program without calling CEFShutdown on Win/Linux platforms, it's better to
call it explicitly.
155b2e7
@hokein @sjlamerton hokein Cleanup some commented code. 4e7b999
@hokein @sjlamerton hokein [Mac] Add CEF path to bakefile system. f6c57fd
@hokein @sjlamerton hokein Nits: Correct code indent. 24a8f62
@hokein @sjlamerton hokein [Mac] Add CEF framework link in configure.in. ed5236c
@hokein @sjlamerton hokein [Mac] Add building scripts from CEF. 668cd4b
@hokein @sjlamerton hokein [Mac] Support Makefile builds. 15f1a1c
@hokein @sjlamerton hokein Add wxUSE_WEBVIEW_CHROMIUM macro. 3f59a5b
@hokein @sjlamerton hokein Add --enable-webviewchromium option in configure. edd8d17
@hokein @sjlamerton hokein Specify CEF Framework link in bakefile system instead configure.
Add CEF framework link in configure will cause each binary builds with
CEF framework path option. We only need to link CEF framework on
building webview library.
94db69e
@hokein @sjlamerton hokein Add wxUSE_WEBVIEW_CHROMIUM macro check in webview_chromium.h/.cpp. f50e47b
@hokein @sjlamerton hokein Disable webview Chromium feature by Default. e882685
@hokein @sjlamerton hokein Set wxUSE_WEBVIEW_CHROMIUM marco in configure.in. 7ebc127
@hokein @sjlamerton hokein Use wxUSE_WEBVIEW_CHROMIUM marco check in sample. 0fc176b
@hokein @sjlamerton hokein Update readme. 3c47998
@hokein @sjlamerton hokein [Mac] Only build webview_chromium helper on OS X. 420f0d7
@hokein @sjlamerton hokein Fix bakefile system generation error, only build helper on OS X. acb7a78
@hokein @sjlamerton hokein Create an independent webview_chromium library. b57d961
@hokein @sjlamerton hokein Use new webview_chromium library instead of webview. f080abf
@hokein @sjlamerton hokein Remove chromium related code from webview library. df32454
@hokein @sjlamerton hokein [Win] Add Visual Studio 2010 webview_chromium library project files. e9dfc9f
@hokein @sjlamerton hokein Move cef directory from <wxwidget_dir>/cef to <wxwidget_dir>/src/cef. b62ff82
@hokein @sjlamerton hokein [Linux] Fix bakefile generation error of webview_chromium sample. 2ad4fe6
@hokein @sjlamerton hokein Add USE_WEBVIEW_CHROMIUM option in bkl file to allow build/unbuild
webview_chromium library.
881efcc
@hokein @sjlamerton hokein [Mac] Change webview_chromium sample to use webview_chromium library. 69d5f69
@hokein @sjlamerton hokein Fix bakefile generation error on win/linux platforms. a22eff8
@hokein @sjlamerton hokein [Win] Add Visual Studio 2012/2013 webviewchromium library project files. e7fe1a4
@hokein @sjlamerton hokein [Win] Update webviewchromium Visual Studio project files. e2b352a
@hokein @sjlamerton hokein Add CEF check in configure. e18bd56
@hokein @sjlamerton hokein [Linux] Add libcef.so path in webview_chromium sample no need to specify
in LD_LIBRARY_PATH.
f1832ab
@hokein @sjlamerton hokein [Mac] Move mac related resource files to mac directory. 2ea3b68
@hokein @sjlamerton hokein Add wxWebViewChromium interface document. f81b493
@hokein @sjlamerton hokein Add wxWebViewFactoryChromium docs and API differences section. d6eb641
@hokein @sjlamerton hokein Remove cef directory check in configure.in. 01873d0
@hokein @sjlamerton hokein Avoid compiler's warning of unused parameters. b2978cb
@hokein @sjlamerton hokein Remove cef lower version support.
Only supports 3.1650.1562(Win/Linux) and 3.1750.1738(Win/Linux/Mac).
f463a8d
@hokein @sjlamerton hokein [Win] Correct webview_chromium sample cef dir path. 391a6ea
@hokein @sjlamerton hokein [Mac] Enable static builds. 37c3d25
@hokein @sjlamerton hokein Update docs: Add required libraries on Linux. f84cd40
@hokein @sjlamerton hokein [Mac] Remove webview sample xcode project. 3d236b9
@hokein @sjlamerton hokein [Win] Remove wxUSE_WEBVIEW_CHROMIUM in VS project files to avoid macro
redefinition.
3b1f0f5
@hokein @sjlamerton hokein Update docs. 12e6ca2
@hokein @sjlamerton hokein [Mac] Make libwebview_chromium depends on libwebview.
This will avoid include webview common code in both two library, and
allow end user to link both two libs for using two backends at the same
times.
6935aae
@hokein @sjlamerton hokein Update docs. 63a5c58
@hokein @sjlamerton hokein [Windows] Update to use the new webviewchromium library.
* Update VS2010 project file.
* Update webview_chromium sample VS2010 project file.
* Introduce a WXDLLIMPEXP_WEBVIEW_CHROMIUM and WXDLLIMPEXP_DATA_WEBVIEW_CHROMIUM
  macro to enable dynamic builds on windows.
17a2a25
@hokein @sjlamerton hokein Remove wxUSE_WEBVIEW_CHROMIUM check in webview.cpp 83baf3c
@hokein @sjlamerton hokein [Win] Fix Debug build error in VS2010. 154efe2
@hokein @sjlamerton hokein [Windows] Update VS2012&2013 project files. fd668a4
@hokein @sjlamerton hokein [Win] Fix wxWebViewChromium sample debug build error in VS2010. af0ff3e
@hokein @sjlamerton hokein [Win] Add VS2012&VS2013 build supports for wxWebViewChromium sample. a6ff3bf
@sjlamerton sjlamerton Update wxWebViewChromium MSW build files. 03f5cc9
@hokein @sjlamerton hokein Update Vistual Studio project files.
Remove wx_vc1[012]_webviewchromium files, and use wx_vc_webviewchromium.vcxproj for VS2010/2012/2013 which
follows the trunk.
95d5a71
@hokein @sjlamerton hokein Remove wxWebViewChromium sample xcode project file. 215719c
@sjlamerton sjlamerton Tidy up wxWebViewChromium documentation. 16357c7
@sjlamerton
Contributor

@vadz I rebased this earlier today with a few changes since the last time we discussed this, any chance you could take a look again?

@vadz vadz commented on an outdated diff Feb 8, 2015
@@ -8233,7 +8239,7 @@ echo " zlib ${wxUSE_ZLIB}"
echo " expat ${wxUSE_EXPAT}"
echo " libmspack ${wxUSE_LIBMSPACK}"
echo " sdl ${wxUSE_LIBSDL}"
-
+echo " webview_chromium ${wxUSE_WEBVIEW_CHROMIUM}"
@vadz
vadz Feb 8, 2015 Contributor

Ideal would be to have something like

webview         yes (with the following backends: WebKit Chromium)

to provide maximum of information on a single line.

@vadz vadz commented on an outdated diff Feb 8, 2015
@@ -7047,6 +7048,11 @@ if test "$wxUSE_WEBVIEW" = "yes"; then
USE_WEBVIEW=1
AC_DEFINE(wxUSE_WEBVIEW)
SAMPLES_SUBDIRS="$SAMPLES_SUBDIRS webview"
+ if test "$wxUSE_WEBVIEW_CHROMIUM" = "yes"; then
+ USE_WEBVIEW_CHROMIUM=1
@vadz
vadz Feb 8, 2015 Contributor

This doesn't seem right, the option is "yes" by default (not sure if it should be, if not, DEFAULT_wxUSE_WEBVIEW_CHROMIUM should be set to "no") and we don't check that we have the required headers/libraries here, so it looks like the compilation will fail out of the box.

We should have AC_TRY_LINK check here.

@vadz vadz commented on the diff Feb 8, 2015
include/wx/webview_chromium.h
+
+#include "wx/defs.h"
+
+#if wxUSE_WEBVIEW && wxUSE_WEBVIEW_CHROMIUM
+
+#include "wx/control.h"
+#include "wx/webview.h"
+#include "wx/sharedptr.h"
+#include "wx/vector.h"
+
+#ifdef __VISUALC__
+#pragma warning(push)
+#pragma warning(disable:4100)
+#endif
+
+#include "include/cef_browser.h"
@vadz
vadz Feb 8, 2015 Contributor

This would probably represent a significant change, but it would be great if we could avoid including all this CEF stuff from this public header. AFAICS the only thing actually needed here is m_clientHandler and we could just wrap it into some wxWebViewChromiumImpl to avoid having it here.

There would be quite a few advantages of keeping this header free of CEF stuff, in particular we could even allow people to use this backend without installing CEF headers at all if we just build the official binaries with it, it would be enough to distribute just the DLLs.

@vadz vadz commented on an outdated diff Feb 8, 2015
interface/wx/webview_chromium.h
+ match those you are using to compile wxWidgets. Specifically on Windows
+ it is likely you will need to adjust the following properties in the CEF3
+ provided build files:
+
+ - C/C++ - Code Generation - Runtime Library - Multithreaded [Debug] DLL
+ - C/C++ - Code Generation - Enable C++ Exceptions - Yes
+ - C/C++ - Language - Enable Run-Time Type Information - Yes
+
+ The following release branches of CEF have been tested: 1650, 1750, 1916,
+ 2062 and 2171.
+
+ @section instructions Build Instructions
+
+ __General__
+
+ The wxWebViewChromium backend is built into a seperate webview_chromium
@vadz
vadz Feb 8, 2015 Contributor

Typo s/seperate/separate/

@vadz vadz commented on an outdated diff Feb 8, 2015
interface/wx/webview_chromium.h
+ - C/C++ - Code Generation - Runtime Library - Multithreaded [Debug] DLL
+ - C/C++ - Code Generation - Enable C++ Exceptions - Yes
+ - C/C++ - Language - Enable Run-Time Type Information - Yes
+
+ The following release branches of CEF have been tested: 1650, 1750, 1916,
+ 2062 and 2171.
+
+ @section instructions Build Instructions
+
+ __General__
+
+ The wxWebViewChromium backend is built into a seperate webview_chromium
+ library which depends on the webview library.
+
+ Once you have a copy of CEF3, either from compiling it yourself or using
+ prebuild binaries you should copy it into `wx_root/src/cef`. To run the
@vadz
vadz Feb 8, 2015 Contributor

s/prebuild/prebuilt/

@vadz vadz commented on the diff Feb 8, 2015
samples/webview/Info_cocoa.plist
+ <string>$(PRODUCT_NAME) version 3.1.0, (c) 2005-2013 wxWidgets</string>
+ <key>CFBundleIconFile</key>
+ <string>wxmac.icns</string>
+ <key>CFBundleIdentifier</key>
+ <string>org.wxwidgets.samples.$(PRODUCT_NAME:rfc1034identifier)</string>
+ <key>CFBundleInfoDictionaryVersion</key>
+ <string>6.0</string>
+ <key>CFBundleLocalizations</key>
+ <array>
+ <string>de</string>
+ <string>en</string>
+ <string>fr</string>
+ <string>it</string>
+ </array>
+ <key>CFBundleLongVersionString</key>
+ <string>3.1.0, (c) 2005-2013 wxWidgets</string>
@vadz
vadz Feb 8, 2015 Contributor

Very minor, but copyright year is off by 2 by now...

(here and below)

@vadz vadz commented on an outdated diff Feb 8, 2015
src/common/webview_chromium.cpp
+#include "include/cef_app.h"
+#include "include/cef_browser.h"
+#include "include/cef_string_visitor.h"
+
+#ifdef __VISUALC__
+#pragma warning(pop)
+#endif
+
+extern WXDLLIMPEXP_DATA_WEBVIEW_CHROMIUM(const char) wxWebViewBackendChromium[] = "wxWebViewChromium";
+
+wxIMPLEMENT_DYNAMIC_CLASS(wxWebViewChromium, wxWebView);
+
+class wxStringVisitor : public CefStringVisitor
+{
+public:
+ enum StringType {
@vadz
vadz Feb 8, 2015 Contributor

Minor: { should be on its own line.

@vadz vadz commented on an outdated diff Feb 8, 2015
src/common/webview_chromium.cpp
+ }
+private:
+ StringType m_type;
+ wxWebViewChromium *m_webview;
+ IMPLEMENT_REFCOUNTING(wxStringVisitor);
+};
+
+bool wxWebViewChromium::Create(wxWindow* parent,
+ wxWindowID id,
+ const wxString& url,
+ const wxPoint& pos,
+ const wxSize& size,
+ long style,
+ const wxString& name)
+{
+#ifdef __WXMSW__
@vadz
vadz Feb 8, 2015 Contributor

What's the difference between the MSW and OSX case? I.e. why don't we check for the return value of Create() in the latter and why don't we just have #ifdef __WXGTK__ and #else branches instead of checking for each platform?

@vadz vadz commented on an outdated diff Feb 8, 2015
src/common/webview_chromium.cpp
+ wxWindowID id,
+ const wxString& url,
+ const wxPoint& pos,
+ const wxSize& size,
+ long style,
+ const wxString& name)
+{
+#ifdef __WXMSW__
+ if ( !wxControl::Create(parent, id, pos, size, style,
+ wxDefaultValidator, name) )
+ {
+ return false;
+ }
+#endif
+
+#ifdef __WXGTK__
@vadz
vadz Feb 8, 2015 Contributor

A comment explaining why is this needed and, in particular, why can't we just call base class Create() with style | wxHSCROLL | wxVSCROLL would be helpful.

@vadz vadz commented on an outdated diff Feb 8, 2015
src/common/webview_chromium.cpp
+}
+
+#ifdef __WXMSW__
+bool wxWebViewChromium::StartUp(int &code, const wxString &path)
+#else
+bool wxWebViewChromium::StartUp(int &code, const wxString &path,
+ int argc, char* argv[])
+#endif
+{
+#ifdef __WXMSW__
+ CefMainArgs args(wxGetInstance());
+#else
+ CefMainArgs args(argc, argv);
+#endif
+ // If there is no subprocess then we need to execute on this process
+ if ( path == "" )
@vadz
vadz Feb 8, 2015 Contributor

Minor: should be path.empty()

@vadz vadz commented on the diff Feb 8, 2015
src/common/webview_chromium.cpp
+ {
+#if CHROME_VERSION_BUILD >= 1750
+ code = CefExecuteProcess(args, NULL, NULL);
+#else
+ code = CefExecuteProcess(args, NULL);
+#endif
+ if ( code >= 0 )
+ return false;
+ }
+
+ CefSettings settings;
+#if CHROME_VERSION_BUILD >= 1750
+ settings.no_sandbox = true;
+#endif
+
+#ifdef __WXDEBUG__
@vadz
vadz Feb 8, 2015 Contributor

This is not appropriate, __WXDEBUG__ is defined even in release build by default in wx3.

It would probably be better to either use some wxWEBVIEW_DEBUG environment variable or provide a method for enabling debug logging.

@vadz
Contributor
vadz commented Feb 8, 2015

Thanks again for your work, I can't really comment on the interesting parts, i.e. the code actually using CEF API, but globally it seems good except for the few things pointed out in the comments.

I also wonder why do we need a separate sample for this, could we just allow using this backend as an option in the normal webview sample? I'd really like to have just one sample instead of two similar but different ones.

@sjlamerton
Contributor

I have made most of the fixes, the Impl changes would be quite big but I'll take a look later this week, I'll also update the debug output then. As for the samples I think the reason that there are two is because it isn't possible to compile the webview_chromium library on Windows if CEF is not present which would limit the number of people who could build the sample as it would fail to link.

@vadz
Contributor
vadz commented Feb 15, 2015

About the sample: could we at least reuse the sources? I.e. we could still have a different directory with its own makefiles, but just #include "../webview/webview.cpp" from webview_chromium.cpp or something like this. Duplicating so much code really doesn't sit well with me.

@tierra tierra referenced this pull request in sjlamerton/wxWebViewChromium Feb 22, 2015
Closed

Retiring the wxWebViewChromium Repository #17

@mths0x5f
mths0x5f commented May 6, 2015

Sorry for the intromission, but how is this going? What can someone do to help?

@sjlamerton
Contributor

The only item left to resolve before the merge was the duplicated code in the samples, it wouldn't take much to resolve that and then I think we should be able to close this pull request.

@mths0x5f

Well... I do not know if it helps, but I have edited some code and made a pull request to you, @steve-lamerton. I'm sorry if inconvenient. sjlamerton#1

@sjlamerton
Contributor

Thanks, I will take a look later this week.

@tierra
Member
tierra commented Feb 15, 2016

It's probably too late to look into getting this into 3.1.0, but it would still be great if @sjlamerton or @hokein could look at rebasing this, resolving conflicts, and testing this out on latest master. I'd still love to see this make it into a future 3.1.x release even if it's not 3.1.0.

@mtangoo
Contributor
mtangoo commented Feb 16, 2016

I would also love to see this merged. It will relieve developers relying on web a lot of pain especially with versions of Native engines available (some are still using very old MS browsers in their systems)

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