146135 2015-06-18 20:58:37 -0700 [EFL] Fix the minibrowser after r185725 2015-06-19 00:59:44 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 hs85.jeong hs85.jeong commit-queue gyuyoung.kim lucas.de.marchi ryuan.choi oldest_to_newest 1103148 0 hs85.jeong 2015-06-18 20:58:37 -0700 Minibrowser didn't work after r185725 because minibrowser didn't find the define of HAVE_ECORE_X. 1103150 1 255167 hs85.jeong 2015-06-18 21:05:32 -0700 Created attachment 255167 Patch 1103155 2 255167 gyuyoung.kim 2015-06-18 21:17:13 -0700 Comment on attachment 255167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255167&action=review > Tools/MiniBrowser/efl/CMakeLists.txt:15 > + ${CMAKE_BINARY_DIR} We should not include any binary output directory from application side. 1103160 3 hs85.jeong 2015-06-18 21:23:51 -0700 (In reply to comment #2) > Comment on attachment 255167 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > > Tools/MiniBrowser/efl/CMakeLists.txt:15 > > + ${CMAKE_BINARY_DIR} > > We should not include any binary output directory from application side. How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. 1103161 4 hs85.jeong 2015-06-18 21:25:38 -0700 You can check this problem when executing the minibrowser with gl backend after r185725. 1103162 5 255167 gyuyoung.kim 2015-06-18 21:39:18 -0700 Comment on attachment 255167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255167&action=review >>> Tools/MiniBrowser/efl/CMakeLists.txt:15 >>> + ${CMAKE_BINARY_DIR} >> >> We should not include any binary output directory from application side. > > How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. I think we need to check if generated "cmakeconfig.h" can be moved to ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at all. 1103163 6 gyuyoung.kim 2015-06-18 21:40:06 -0700 (In reply to comment #5) > Comment on attachment 255167 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > >>> Tools/MiniBrowser/efl/CMakeLists.txt:15 > >>> + ${CMAKE_BINARY_DIR} > >> > >> We should not include any binary output directory from application side. > > > > How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > > GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. > > I think we need to check if generated "cmakeconfig.h" can be moved to > ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to > include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at > all. I'm not 100% sure if we should open the cmakeconfig.h yet. 1103177 7 hs85.jeong 2015-06-18 22:53:43 -0700 You can check this problem when executing the minibrowser with gl backend after r185725.(In reply to comment #5) > Comment on attachment 255167 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > >>> Tools/MiniBrowser/efl/CMakeLists.txt:15 > >>> + ${CMAKE_BINARY_DIR} > >> > >> We should not include any binary output directory from application side. > > > > How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > > GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. > > I think we need to check if generated "cmakeconfig.h" can be moved to > ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to > include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at > all. Is it okay to move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? I have some doubt why we move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? Is cmakeconfig.h only used by files in ${DERIVED_SOURCES_WEBKIT2_DIR}/include? Already cmake based projects used the cmakeconfig.h in ${CMAKE_BINARY_DIR}. If moving the cmakeconfig.h is right, I have to change the many files. 1103186 8 gyuyoung.kim 2015-06-18 23:29:48 -0700 (In reply to comment #7) > You can check this problem when executing the minibrowser with gl backend > after r185725.(In reply to comment #5) > > Comment on attachment 255167 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > > > >>> Tools/MiniBrowser/efl/CMakeLists.txt:15 > > >>> + ${CMAKE_BINARY_DIR} > > >> > > >> We should not include any binary output directory from application side. > > > > > > How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > > > GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. > > > > I think we need to check if generated "cmakeconfig.h" can be moved to > > ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to > > include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at > > all. > > Is it okay to move the cmakeconfig.h to > ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > I have some doubt why we move the cmakeconfig.h to > ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > Is cmakeconfig.h only used by files in > ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > Already cmake based projects used the cmakeconfig.h in ${CMAKE_BINARY_DIR}. > If moving the cmakeconfig.h is right, I have to change the many files. I meant copy, not move :) Anyway, as I said, I'm not sure if we have to open cmakeconfig.h to third party application. We may let MiniBrowser to include CMAKE_BINARY_DIR. However I don't think this is correct use. It looks my recommendation is not correct use too. I think correct solution is that we define it to ewebkit.pc, application(e.g. MiniBrowser) reads it, then it works based on the definition. How about just defining HAVE_ECORE_X in main.c as below ? #define HAVE_ECORE_X 1 In other words, I think it is too excessive work that MiniBrowser includes cmakeconfig.h only in order to use HAVE_ECORE_X. 1103187 9 255167 ryuan.choi 2015-06-18 23:29:51 -0700 Comment on attachment 255167 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255167&action=review >>>>>> Tools/MiniBrowser/efl/CMakeLists.txt:15 >>>>>> + ${CMAKE_BINARY_DIR} >>>>> >>>>> We should not include any binary output directory from application side. >>>> >>>> How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? >>>> GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. >>> >>> I think we need to check if generated "cmakeconfig.h" can be moved to ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at all. >> >> I'm not 100% sure if we should open the cmakeconfig.h yet. > > Is it okay to move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > I have some doubt why we move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > Is cmakeconfig.h only used by files in ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > Already cmake based projects used the cmakeconfig.h in ${CMAKE_BINARY_DIR}. > If moving the cmakeconfig.h is right, I have to change the many files. No! In this case, We should not use HAVE_ECORE_X in MiniBrowser. HAVE_ECORE_X guard was introduced to set opengl_x11 as preferred engine but now we set opengl. I am not sure whether we should keep the macro. 1103188 10 gyuyoung.kim 2015-06-18 23:31:10 -0700 (In reply to comment #9) > Comment on attachment 255167 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > >>>>>> Tools/MiniBrowser/efl/CMakeLists.txt:15 > >>>>>> + ${CMAKE_BINARY_DIR} > >>>>> > >>>>> We should not include any binary output directory from application side. > >>>> > >>>> How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > >>>> GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. > >>> > >>> I think we need to check if generated "cmakeconfig.h" can be moved to ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at all. > >> > >> I'm not 100% sure if we should open the cmakeconfig.h yet. > > > > Is it okay to move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > > > I have some doubt why we move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > Is cmakeconfig.h only used by files in ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > > > Already cmake based projects used the cmakeconfig.h in ${CMAKE_BINARY_DIR}. > > If moving the cmakeconfig.h is right, I have to change the many files. > > No! > > In this case, > We should not use HAVE_ECORE_X in MiniBrowser. > > HAVE_ECORE_X guard was introduced to set opengl_x11 as preferred engine but > now we set opengl. > I am not sure whether we should keep the macro. +1 I think so ! @Hunseop, just define HAVE_ECORE_X in main.c or remove it ! 1103194 11 hs85.jeong 2015-06-18 23:48:41 -0700 (In reply to comment #10) > (In reply to comment #9) > > Comment on attachment 255167 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=255167&action=review > > > > >>>>>> Tools/MiniBrowser/efl/CMakeLists.txt:15 > > >>>>>> + ${CMAKE_BINARY_DIR} > > >>>>> > > >>>>> We should not include any binary output directory from application side. > > >>>> > > >>>> How can I find the "cmakeconfig.h" in main.c without adding the CMAKE_BINARY_DIR? > > >>>> GTK+port also added the CMAKE_BINARY_DIR at MiniBrowser_INCLUDE_DIRECTORIES. > > >>> > > >>> I think we need to check if generated "cmakeconfig.h" can be moved to ${DERIVED_SOURCES_WEBKIT2_DIR}/include, then we may make EwebKit2.h to include the cmakeconfig.h. If so, we don't need to modify MiniBrowser not at all. > > >> > > >> I'm not 100% sure if we should open the cmakeconfig.h yet. > > > > > > Is it okay to move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > > > > > I have some doubt why we move the cmakeconfig.h to ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > > Is cmakeconfig.h only used by files in ${DERIVED_SOURCES_WEBKIT2_DIR}/include? > > > > > > Already cmake based projects used the cmakeconfig.h in ${CMAKE_BINARY_DIR}. > > > If moving the cmakeconfig.h is right, I have to change the many files. > > > > No! > > > > In this case, > > We should not use HAVE_ECORE_X in MiniBrowser. > > > > HAVE_ECORE_X guard was introduced to set opengl_x11 as preferred engine but > > now we set opengl. > > I am not sure whether we should keep the macro. > > +1 I think so ! > > @Hunseop, just define HAVE_ECORE_X in main.c or remove it ! Okay, I will remove it! Thanks. 1103199 12 255181 hs85.jeong 2015-06-19 00:06:20 -0700 Created attachment 255181 Patch 1103205 13 255181 commit-queue 2015-06-19 00:59:37 -0700 Comment on attachment 255181 Patch Clearing flags on attachment: 255181 Committed r185740: <http://trac.webkit.org/changeset/185740> 1103206 14 commit-queue 2015-06-19 00:59:44 -0700 All reviewed patches have been landed. Closing bug. 255167 2015-06-18 21:05:32 -0700 2015-06-19 00:06:12 -0700 Patch bug-146135-20150619130454.patch text/plain 1694 hs85.jeong U3VidmVyc2lvbiBSZXZpc2lvbjogMTg1NzMxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOTkwZmZjZGQ0OWQ1OTJhYjE0MDVhZDY0Zjk4NmU4NTk0 ZjUyNjc3ZS4uMGE4YjQzMWNmZGY0MTUwZWZlYzdhNjYxN2RhZDUwY2VhNzFjNmMzYyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1 IEBACisyMDE1LTA2LTE4ICBIdW5zZW9wIEplb25nICA8aHM4NS5qZW9uZ0BzYW1zdW5nLmNvbT4K KworICAgICAgICBbRUZMXSBGaXggdGhlIG1pbmlicm93c2VyIGFmdGVyIHIxODU3MjUKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0NjEzNQorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFkZGVkIHRoZSBjbWFr ZWNvbmZpZy5oIHRvIGZpbmQgdGhlIGRlZmluZSBvZiBIQVZFX0VDT1JFX1guCisKKyAgICAgICAg KiBNaW5pQnJvd3Nlci9lZmwvQ01ha2VMaXN0cy50eHQ6IEFkZGVkIHRoZSBkaXJlY3Rvcnkgb2Yg Y21ha2Vjb25maWcuaAorICAgICAgICAqIE1pbmlCcm93c2VyL2VmbC9tYWluLmM6IEFkZGVkIHRo ZSBjbWFrZWNvbmZpZy5oCisKIDIwMTUtMDYtMTggIEpvbiBMZWUgIDxqb25sZWVAYXBwbGUuY29t PgogCiAgICAgICAgIFVucmV2aWV3ZWQuIEFkZCBNYXR0IERhaXRlciBhcyBhIGNvbnRyaWJ1dG9y LgpkaWZmIC0tZ2l0IGEvVG9vbHMvTWluaUJyb3dzZXIvZWZsL0NNYWtlTGlzdHMudHh0IGIvVG9v bHMvTWluaUJyb3dzZXIvZWZsL0NNYWtlTGlzdHMudHh0CmluZGV4IDg3ZWRlOWQwMGMyOGI2Njky YmI0NTc2MDgzNmY0NTUwNDg5ZmNhMDIuLmFjYzExMmJlZDFhMzU4MTExOTM3MWExMjllYTYwODZi NDAzMzFmZDIgMTAwNjQ0Ci0tLSBhL1Rvb2xzL01pbmlCcm93c2VyL2VmbC9DTWFrZUxpc3RzLnR4 dAorKysgYi9Ub29scy9NaW5pQnJvd3Nlci9lZmwvQ01ha2VMaXN0cy50eHQKQEAgLTEyLDYgKzEy LDcgQEAgc2V0KE1pbmlCcm93c2VyX1NPVVJDRVMKIAogc2V0KE1pbmlCcm93c2VyX0lOQ0xVREVf RElSRUNUT1JJRVMKICAgICAke0NBSVJPX0lOQ0xVREVfRElSU30KKyAgICAke0NNQUtFX0JJTkFS WV9ESVJ9CiAgICAgJHtERVJJVkVEX1NPVVJDRVNfV0VCS0lUMl9ESVJ9L2luY2x1ZGUKICAgICAk e0VDT1JFX0lOQ0xVREVfRElSU30KICAgICAke0VDT1JFX0VWQVNfSU5DTFVERV9ESVJTfQpkaWZm IC0tZ2l0IGEvVG9vbHMvTWluaUJyb3dzZXIvZWZsL21haW4uYyBiL1Rvb2xzL01pbmlCcm93c2Vy L2VmbC9tYWluLmMKaW5kZXggNzg4ZGNjODljNDk2NGFhMzEzMmI2YTczMzhiYTZhZjhmYWRkYWRl YS4uNmUwYTNlZjVjZWU4NWJlODNiN2MzMDNjNWVjYTllYjgzZDZjMDJjMCAxMDA2NDQKLS0tIGEv VG9vbHMvTWluaUJyb3dzZXIvZWZsL21haW4uYworKysgYi9Ub29scy9NaW5pQnJvd3Nlci9lZmwv bWFpbi5jCkBAIC0xNyw2ICsxNyw4IEBACiAgKiAgRm91bmRhdGlvbiwgSW5jLiwgNTEgRnJhbmts aW4gU3RyZWV0LCBGaWZ0aCBGbG9vciwgQm9zdG9uLCBNQSAgMDIxMTAtMTMwMSAgVVNBCiAgKi8K IAorI2luY2x1ZGUgImNtYWtlY29uZmlnLmgiCisKICNpbmNsdWRlICJFV2ViS2l0Mi5oIgogI2lu Y2x1ZGUgPEVjb3JlLmg+CiAjaW5jbHVkZSA8RWNvcmVfRXZhcy5oPgo= 255181 2015-06-19 00:06:20 -0700 2015-06-19 00:59:37 -0700 Patch bug-146135-20150619160542.patch text/plain 1406 hs85.jeong U3VidmVyc2lvbiBSZXZpc2lvbjogMTg1NzMxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOTkwZmZjZGQ0OWQ1OTJhYjE0MDVhZDY0Zjk4NmU4NTk0 ZjUyNjc3ZS4uZDlmMTU0ZTMwZTlkNDQyNGMxOTVlZDY4ODE5MGVlYTliZDEzYzdiZiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDE1LTA2LTE4ICBIdW5zZW9wIEplb25nICA8aHM4NS5qZW9uZ0BzYW1zdW5nLmNvbT4K KworICAgICAgICBbRUZMXSBGaXggdGhlIG1pbmlicm93c2VyIGFmdGVyIHIxODU3MjUKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0NjEzNQorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJlbW92ZWQgdGhlIEhB VkVfRUNPUkVfWCBndWFyZCBpbiBNaW5pQnJvd3NlciBiZWNhdXNlIGl0IGlzIGFuIHVubmVjZXNz YXJ5IGd1YXJkCisgICAgICAgIGZvciB0aGUgZWxtX2NvbmZpZ19hY2NlbF9wcmVmZXJlbmNlX3Nl dCgpIHRvIHNldCB0aGUgY29uZmlnIG9mIGFjY2VsZXJhdGlvbiBwcmVmZXJlbmNlLgorCisgICAg ICAgICogTWluaUJyb3dzZXIvZWZsL21haW4uYzoKKyAgICAgICAgKGVsbV9tYWluKTogRGVsZXRl ZCB0aGUgSEFWRV9FQ09SRV9YIGd1YXJkLgorCiAyMDE1LTA2LTE4ICBKb24gTGVlICA8am9ubGVl QGFwcGxlLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLiBBZGQgTWF0dCBEYWl0ZXIgYXMgYSBj b250cmlidXRvci4KZGlmZiAtLWdpdCBhL1Rvb2xzL01pbmlCcm93c2VyL2VmbC9tYWluLmMgYi9U b29scy9NaW5pQnJvd3Nlci9lZmwvbWFpbi5jCmluZGV4IDc4OGRjYzg5YzQ5NjRhYTMxMzJiNmE3 MzM4YmE2YWY4ZmFkZGFkZWEuLmUzNWRlYzA4NzMxZTgwMjQ5OThlZGQxNmY2NDA2MzRkNjM5YjQx NjAgMTAwNjQ0Ci0tLSBhL1Rvb2xzL01pbmlCcm93c2VyL2VmbC9tYWluLmMKKysrIGIvVG9vbHMv TWluaUJyb3dzZXIvZWZsL21haW4uYwpAQCAtMjQzNywxMiArMjQzNywxMCBAQCBlbG1fbWFpbihp bnQgYXJnYywgY2hhciAqYXJndltdKQogCiAgICAgaWYgKGV2YXNfZW5naW5lX25hbWUpCiAgICAg ICAgIGVsbV9jb25maWdfYWNjZWxfcHJlZmVyZW5jZV9zZXQoZXZhc19lbmdpbmVfbmFtZSk7Ci0j aWYgZGVmaW5lZChIQVZFX0VDT1JFX1gpCiAgICAgZWxzZSB7CiAgICAgICAgIGV2YXNfZW5naW5l X25hbWUgPSAib3BlbmdsIjsKICAgICAgICAgZWxtX2NvbmZpZ19hY2NlbF9wcmVmZXJlbmNlX3Nl dChldmFzX2VuZ2luZV9uYW1lKTsKICAgICB9Ci0jZW5kaWYKIAogICAgIEV3a19Db250ZXh0ICpj b250ZXh0ID0gZXdrX2NvbnRleHRfZGVmYXVsdF9nZXQoKTsKIAo=