GTK uses the copy-user-interface-resources.pl to combine inspector resources. Windows should do something similar to speed up inspector load times in Release builds.
Created attachment 397399 [details] patch
Created attachment 397400 [details] patch
Comment on attachment 397400 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=397400&action=review > Source/WebInspectorUI/CMakeLists.txt:3 > + ${CMAKE_SOURCE_DIR}/Source/WebInspectorUI/UserInterface/*.html I prefer ${CMAKE_CURRENT_SOURCE_DIR} to ${CMAKE_SOURCE_DIR}/Source/WebInspectorUI, becuase WebKit might be built as submodule of other project.
Comment on attachment 397400 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=397400&action=review > CMakeLists.txt:36 > +endif () If I understand CMake properly, 'option' command is the CMake way to do that. option(ENABLE_WEBINSPECTOR_UI "help string describing option" ON)
Comment on attachment 397400 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=397400&action=review >> CMakeLists.txt:36 >> +endif () > > If I understand CMake properly, 'option' command is the CMake way to do that. > option(ENABLE_WEBINSPECTOR_UI "help string describing option" ON) Ah, the default values are different for ports. Ignore this comment.
Comment on attachment 397400 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=397400&action=review Other than the below LGTM >> Source/WebInspectorUI/CMakeLists.txt:3 >> + ${CMAKE_SOURCE_DIR}/Source/WebInspectorUI/UserInterface/*.html > > I prefer ${CMAKE_CURRENT_SOURCE_DIR} to ${CMAKE_SOURCE_DIR}/Source/WebInspectorUI, > becuase WebKit might be built as submodule of other project. Seems more like you should follow along with what happens in WebKitFS.cmake if (NOT WEBINSPECTORUI_DIR) set(WEBINSPECTORUI_DIR "${CMAKE_SOURCE_DIR}/Source/WebInpsectorUI") endif ()
Created attachment 397501 [details] Patch for landing
Created attachment 397502 [details] Patch for landing
ChangeLog entry in ChangeLog contains OOPS!.
Created attachment 397507 [details] Patch for landing
Committed r260672: <https://trac.webkit.org/changeset/260672> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397507 [details].
This patch broke the compilation of GTK and WPE EWS build bots. Fixed by https://bugs.webkit.org/show_bug.cgi?id=211014
(In reply to Diego Pino from comment #12) > This patch broke the compilation of GTK and WPE EWS build bots. Fixed by > https://bugs.webkit.org/show_bug.cgi?id=211014 I am getting GTK build errors since r260672 that are still present as of this moment (r260723). The build error is this: make[3]: *** No rule to make target 'DerivedSources/JavaScriptCore/inspector/InspectorBackendCommands.js', needed by 'DerivedSources/WebInspectorUI/UserInterface/Protocol/InspectorBackendCommands.js'. Stop. r260671 was the last I could build the GTK port without any errors.
Jim Manson: The GTK build bot is green on r260723, and the GTK EWS build bots are able to build too. https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20(Build)?numbuilds=50 https://ews-build.webkit.org/#/builders/36 I've also tried to build r260723 locally and it worked OK. I would suggest to try a clean build. What command are you using to build WebKitGTK? If I have time I can give it a try.
Hello Diego, thank you for the prompt reply. I believe I have found the problem: It seems the new WebInspectorUI target has an implicit dependency on JavaScriptCore for generation of 'DerivedSources/JavaScriptCore/inspector/InspectorBackendCommands.js' referenced above and maybe more. When I add JavaScriptCore as a dependency to the WebInspectorUI target in Source/WebInspectorUI/CMakeLists.txt, it builds with no problem. I am building with -j 8, which may have helped expose the dependency.
Reopening.
(In reply to Jim Mason from comment #15) > When I add JavaScriptCore as a dependency to the WebInspectorUI target in > Source/WebInspectorUI/CMakeLists.txt, it builds with no problem. Did r260728 fix it?
This also appears to be broken for not ENABLE_WEBINSPECTOR_UI ports that build WebCore (i.e. not jsc only) as it unconditionally adds the dependency to WebCore whether the enable is on or not. From https://build.webkit.org/builders/PlayStation-Release-Build/builds/2043/steps/compile-webkit/logs/stdio CMake Error at Source/cmake/WebKitMacros.cmake:179 (add_dependencies): The dependency target "WebInspectorUI" of target "WebCore" does not exist. Call Stack (most recent call first): Source/cmake/WebKitMacros.cmake:184 (_WEBKIT_TARGET) Source/WebCore/CMakeLists.txt:2033 (WEBKIT_FRAMEWORK)
Reverted r260672 for reason: [GTK] WebInspector tests are timing out after r260672 Committed r260743: <https://trac.webkit.org/changeset/260743>
I reverted the patch since WebInspector tests are timing out after this change. I also reverted related build fixes changesets (r260728 and r260696). Ticket of the issue: https://bugs.webkit.org/show_bug.cgi?id=211060 Latest GTK test builds: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20(Tests)?numbuilds=50 I was able to detect the test regression after a clean build.
Created attachment 397778 [details] Fixing GTK timeouts This patch should fix the timeouts. The problem was WebInspectorUI_RESOURCES_DIR wasn't pointing to the InspectorResources folder on GTK/WPE, sorry about that. I tried a clean build with these changes on my local GTK environment and the only tests timing out now are: > 17:05:49.058 3 Tests that timed out or crashed: > 17:05:49.058 3 inspector/debugger/nested-inspectors.html took 17.0 seconds > 17:05:49.058 3 inspector/debugger/breakpoints/resolved-dump-each-line.html took 16.9 seconds > 17:05:49.058 3 inspector/dom-debugger/dom-breakpoints.html took 15.2 seconds > 17:05:49.058 3 inspector/console/heapSnapshot.html took 15.1 seconds These timeouts look expected.
Looks good to me.
Created attachment 397867 [details] Patch for landing
Committed r260844: <https://trac.webkit.org/changeset/260844> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397867 [details].
I think this patch made incremental builds super slow because inspector files are always copied now. I guess it's because inspector-resources.stamp is no longer used.
(In reply to Carlos Garcia Campos from comment #25) > I think this patch made incremental builds super slow because inspector > files are always copied now. I guess it's because inspector-resources.stamp > is no longer used. I'm looking in to fixing this now and created https://bugs.webkit.org/show_bug.cgi?id=211212 for it
*** Bug 211014 has been marked as a duplicate of this bug. ***