Bug 121183

Summary:

Web Inspector: InjectedScriptSource_js and InjectedScriptCanvasModuleSource_js should be minified

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, buildbot, commit-queue, darin, eflews.bot, graouts, gyuyoung.kim, halton.huo, joepeck, kling, mrowe, paroga, rakuco, rniwa, simon.fraser, timothy, webkit-bug-importer, webkit-ews, zan

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	none
[PATCH] Proposed Fix	timothy: review+, commit-queue: commit-queue-
Archive of layout-test-results from webkit-cq-02 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2	none
[PATCH] Improved - All ports, update test, address InspectorOverlayPage.html	timothy: review+, webkit-ews: commit-queue-
[PATCH] For Bots 1	none
[PATCH] For Bots 2	eflews.bot: commit-queue-
[PATCH] For Bots 3	none

Ryosuke Niwa

Reported 2013-09-11 14:49:30 PDT

rniwa:webkit rniwa$ symbols WebKitBuild/Release/WebCore.framework/WebCore | sed -E 's/^ *0x[0-9a-f]+ *$( *)0x([^)]+)$/\1\2/' | sort -r | head -n 30 cd6ca8 __TEXT __text WebKitBuild/Release/WebCore.framework/WebCore [x86_64, 5.870281 seconds]: 27ff60 __TEXT __eh_frame 15a698 __DATA __const 5aba8 __TEXT __const 3806a __TEXT __cstring 3554e str [NameNList, MangledNameNList, NList] 2e228 __DATA __data 2663c __TEXT __unwind_info 1b018 __TEXT __gcc_except_tab 1a8d0 __DATA __objc_const 17e70 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] < This is 95.6KB! ... It's generated from WebCore/inspector/InjectedScriptCanvasModuleSource.js. I'm not sure why this code isn't compressed before generating the binary at least in release builds.

Attachments
[PATCH] Proposed Fix (14.80 KB, patch) 2013-09-12 17:11 PDT, Joseph Pecoraro	no flags	Details Formatted Diff Diff
[PATCH] Proposed Fix (15.76 KB, patch) 2013-09-12 17:15 PDT, Joseph Pecoraro	timothy: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-cq-02 for mac-mountainlion (2.10 MB, application/zip) 2013-09-12 19:21 PDT, WebKit Commit Bot	no flags	Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (1.05 MB, application/zip) 2013-09-12 19:22 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (1.33 MB, application/zip) 2013-09-12 20:02 PDT, Build Bot	no flags	Details
[PATCH] Improved - All ports, update test, address InspectorOverlayPage.html (67.66 KB, patch) 2013-09-13 13:03 PDT, Joseph Pecoraro	timothy: review+ webkit-ews: commit-queue-	Details Formatted Diff Diff
[PATCH] For Bots 1 (67.73 KB, patch) 2013-09-13 13:24 PDT, Joseph Pecoraro	no flags	Details Formatted Diff Diff
[PATCH] For Bots 2 (67.71 KB, patch) 2013-09-13 16:34 PDT, Joseph Pecoraro	eflews.bot: commit-queue-	Details Formatted Diff Diff
[PATCH] For Bots 3 (68.35 KB, patch) 2013-09-14 02:09 PDT, Joseph Pecoraro	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2013-09-11 14:49:43 PDT

<rdar://problem/14969065>

Radar WebKit Bug Importer

Comment 2 2013-09-11 14:49:43 PDT

<rdar://problem/14969066>

Timothy Hatcher

Comment 3 2013-09-11 15:54:08 PDT

Applies to InjectedScriptSource_js too, which is also large.

Joseph Pecoraro

Comment 4 2013-09-12 16:20:59 PDT

Also InspectorOverlayPage_html (which contains <style> and <script> that could be minified).

Joseph Pecoraro

Comment 5 2013-09-12 17:09:05 PDT

Minifying with jsmin (keeping the license header) shows pretty good savings on disk: > 574K InjectedScriptCanvasModuleSource-before.h > 334K InjectedScriptCanvasModuleSource-after.h > > 280K InjectedScriptSource-before.h > 150K InjectedScriptSource-after.h I'll do a measurement with the command rniwa showed (need to do a release build and it takes a while to run the command on the binary). Inspector still looks good. Unfortunately this change to DerivedSources.make only really helps the few ports that use that script. Other ports will need to update their build scripts, or we should make one smart script that does everything.

Joseph Pecoraro

Comment 6 2013-09-12 17:11:02 PDT

Created attachment 211493 [details] [PATCH] Proposed Fix

Joseph Pecoraro

Comment 7 2013-09-12 17:11:38 PDT

Comment on attachment 211493 [details] [PATCH] Proposed Fix Oops, forgot ChangeLog.

WebKit Commit Bot

Comment 8 2013-09-12 17:12:15 PDT

Attachment 211493 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Total errors found: 11 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 9 2013-09-12 17:15:25 PDT

Created attachment 211494 [details] [PATCH] Proposed Fix

WebKit Commit Bot

Comment 10 2013-09-12 17:17:07 PDT

Attachment 211494 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Total errors found: 11 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 11 2013-09-12 17:38:47 PDT

Stats with rniwa's calculations: before: 95.61k 17e70 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] after: 55.59k de60 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] before: 46.77k bb10 InjectedScriptSource_js [NameNList, MangledNameNList, NList] after: 24.97k 63e0 InjectedScriptSource_js [NameNList, MangledNameNList, NList]

Joseph Pecoraro

Comment 12 2013-09-12 18:22:08 PDT

I'm going to tackle InspectorOverlayPage.html JS/CSS minification in: <https://webkit.org/b/121270> Web Inspector: InspectorOverlayPage_html should have minified css and js

WebKit Commit Bot

Comment 13 2013-09-12 19:21:13 PDT

Comment on attachment 211494 [details] [PATCH] Proposed Fix Rejecting attachment 211494 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: act-list-items-014.html [ ImageOnlyFailure Pass ] fast/regions/counters/extract-list-items-015.html [ ImageOnlyFailure Pass ] fast/regions/counters/extract-numbered-paragraphs-divs-002.html [ ImageOnlyFailure Pass ] Regressions: Unexpected text-only failures (1) inspector/console/command-line-api.html [ Failure ] ------------------------------------------------------------------------------ Failed to execute Tools/Scripts/new-run-webkit-tests at Tools/Scripts/run-webkit-tests line 100. Full output: http://webkit-queues.appspot.com/results/1862160

WebKit Commit Bot

Comment 14 2013-09-12 19:21:17 PDT

Created attachment 211497 [details] Archive of layout-test-results from webkit-cq-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: webkit-cq-02 Port: mac-mountainlion Platform: Mac OS X 10.8.4

Build Bot

Comment 15 2013-09-12 19:22:22 PDT

Comment on attachment 211494 [details] [PATCH] Proposed Fix Attachment 211494 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1845016 New failing tests: inspector/console/command-line-api.html

Build Bot

Comment 16 2013-09-12 19:22:25 PDT

Created attachment 211498 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.4

Joseph Pecoraro

Comment 17 2013-09-12 19:23:51 PDT

(In reply to comment #16) > Created an attachment (id=211498) [details] > Archive of layout-test-results from webkit-ews-05 for mac-mountainlion > > The attached test failures were seen while running run-webkit-tests on the mac-ews. > Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.4 Looks legit. I'll update the test and land it manually. - CONSOLE MESSAGE: line 1166: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s") + CONSOLE MESSAGE: line 208: The console function $() has changed from $=getElementById(id) to $=querySelector(selector). You might try $("#%s")

Joseph Pecoraro

Comment 18 2013-09-12 19:24:29 PDT

Oh, hmm, so because we minify but other ports don't right now that would be problematic. Really I just should remove the line number from outputting in that test, because it constantly needs to be updated.

Build Bot

Comment 19 2013-09-12 20:02:34 PDT

Comment on attachment 211494 [details] [PATCH] Proposed Fix Attachment 211494 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1871104 New failing tests: inspector/console/command-line-api.html

Build Bot

Comment 20 2013-09-12 20:02:37 PDT

Created attachment 211499 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4

Joseph Pecoraro

Comment 21 2013-09-13 13:03:06 PDT

Created attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html So, because of that failing test, I decided to go further: 1. No longer keep the license in the xxd output. Its just wasted space 2. Minify in other ports, should keep the test passing. 3. Fix InspectorOverlayPage.html here, instead of <https://webkit.org/b/121270> Since they are so related, might as well do them at the same time.

Joseph Pecoraro

Comment 22 2013-09-13 13:05:14 PDT

*** Bug 121270 has been marked as a duplicate of this bug. ***

Joseph Pecoraro

Comment 23 2013-09-13 13:06:01 PDT

Stats for InspectorOverlayPage_html: before: 18.13k 4880 InspectorOverlayPage_html [NameNList, MangledNameNList, NList] after: 13.97k 37e0 InspectorOverlayPage_html [NameNList, MangledNameNList, NList] And just the generated size on disk: 84K InspectorOverlayPage-after.h 109K InspectorOverlayPage-before.h Note this still includes a copyright comment at the top. Would be nice to strip that.

Early Warning System Bot

Comment 24 2013-09-13 13:06:32 PDT

Comment on attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html Attachment 211579 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1862242

WebKit Commit Bot

Comment 25 2013-09-13 13:06:59 PDT

Attachment 211579 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorOverlayPage.css', u'Source/WebCore/inspector/InspectorOverlayPage.html', u'Source/WebCore/inspector/InspectorOverlayPage.js', u'Source/WebCore/inspector/Scripts/cssmin.py', u'Source/WebCore/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/cssmin.py:28: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/cssmin.py:34: whitespace before ')' [pep8/E202] [5] Source/WebCore/inspector/Scripts/cssmin.py:33: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py:36: expected 2 blank lines, found 1 [pep8/E302] [5] Total errors found: 15 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Early Warning System Bot

Comment 26 2013-09-13 13:08:08 PDT

Comment on attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html Attachment 211579 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1819081

Joseph Pecoraro

Comment 27 2013-09-13 13:15:40 PDT

Comment on attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html View in context: https://bugs.webkit.org/attachment.cgi?id=211579&action=review > Source/WebCore/DerivedSources.pri:832 > +inspectorOverlayPage.depends $$INSPECTOR_OVERLAY_PAGE Missing `=`, should be: inspectorOverlayPage.depends = $$INSPECTOR_OVERLAY_PAGE Waiting on other bots. I'll continue to use the EWS to check their builds.

Joseph Pecoraro

Comment 28 2013-09-13 13:24:22 PDT

Created attachment 211582 [details] [PATCH] For Bots 1

WebKit Commit Bot

Comment 29 2013-09-13 13:27:38 PDT

Attachment 211582 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorOverlayPage.css', u'Source/WebCore/inspector/InspectorOverlayPage.html', u'Source/WebCore/inspector/InspectorOverlayPage.js', u'Source/WebCore/inspector/Scripts/cssmin.py', u'Source/WebCore/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/cssmin.py:28: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/cssmin.py:34: whitespace before ')' [pep8/E202] [5] Source/WebCore/inspector/Scripts/cssmin.py:33: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Total errors found: 14 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 30 2013-09-13 16:11:23 PDT

Hmm, looks like I'll need some help from EFL for CMakeLists.txt. The following build command is failing: > # Generate InjectedScriptSource.h > add_custom_command( > OUTPUT ${DERIVED_SOURCES_WEBCORE_DIR}/InjectedScriptSource.h > MAIN_DEPENDENCY inspector/InjectedScriptSource.js > COMMAND ${PYTHON_EXECUTABLE} ${WEBCORE_DIR}/inspector/Scripts/jsmin.py <${WEBCORE_DIR}/inspector/InjectedScriptSource.js ${DERIVED_SOURCES_WEBCORE_DIR}/InjectedScriptSource.min.js > COMMAND ${PERL_EXECUTABLE} ${WEBCORE_DIR}/inspector/xxd.pl InjectedScriptSource_js ${DERIVED_SOURCES_WEBCORE_DIR}/InjectedScriptSource.min.js ${DERIVED_SOURCES_WEBCORE_DIR}/InjectedScriptSource.h > VERBATIM) Failing with error: > Generating ../../DerivedSources/WebCore/InjectedScriptSource.h > Can't open file for read: /mnt/eflews/webkit/WebKit/WebKitBuild/Release/DerivedSources/WebCore/InjectedScriptSource.min.js No such file or directory at /mnt/eflews/webkit/WebKit/Source/WebCore/inspector/xxd.pl line 36. > make[2]: *** [DerivedSources/WebCore/InjectedScriptSource.h] Error 2 I would have though this would work, given what I've read about add_custom_command and COMMAND: <http://www.cmake.org/cmake/help/cmake2.6docs.html#command:add_custom_command> > add_custom_command(OUTPUT output1 [output2 ...] > COMMAND command1 [ARGS] [args1...] > [COMMAND command2 [ARGS] [args2...] ...] > [MAIN_DEPENDENCY depend] > [DEPENDS [depends...]] > [IMPLICIT_DEPENDS <lang1> depend1 ...] > [WORKING_DIRECTORY dir] > [COMMENT comment] [VERBATIM] [APPEND]) > > If more than one command is specified they will be executed in order. The optional ARGS argument is for backward compatibility and will be ignored. […] So I would have expected the first command to create DerivedSources/WebCore/InjectedScriptSource.min.js and it would exist with the second command. Apparently that is not the case. What would be the best way to fix this?

Joseph Pecoraro

Comment 31 2013-09-13 16:31:23 PDT

Patrick Gansterer or Halton Huo, any ideas on how I could get what I want in the CMakeList file?

Joseph Pecoraro

Comment 32 2013-09-13 16:34:31 PDT

Created attachment 211602 [details] [PATCH] For Bots 2

WebKit Commit Bot

Comment 33 2013-09-13 16:42:46 PDT

Attachment 211602 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorOverlayPage.css', u'Source/WebCore/inspector/InspectorOverlayPage.html', u'Source/WebCore/inspector/InspectorOverlayPage.js', u'Source/WebCore/inspector/Scripts/cssmin.py', u'Source/WebCore/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/cssmin.py:28: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/cssmin.py:34: whitespace before ')' [pep8/E202] [5] Source/WebCore/inspector/Scripts/cssmin.py:33: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Total errors found: 14 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 34 2013-09-13 16:49:47 PDT

Removing the copyright comment saved another 1.5k in each: before: 95.61k 17e70 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] after: 55.59k de60 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] no copyright comment: 54.08k d850 InjectedScriptCanvasModuleSource_js [NameNList, MangledNameNList, NList] before: 46.77k bb10 InjectedScriptSource_js [NameNList, MangledNameNList, NList] after: 24.97k 63e0 InjectedScriptSource_js [NameNList, MangledNameNList, NList] no copyright comment: 23.44k 5dc0 InjectedScriptSource_js [NameNList, MangledNameNList, NList]

Joseph Pecoraro

Comment 35 2013-09-13 18:52:11 PDT

Is there something I need to do to get the EFL EWS bots to run the last patch?

EFL EWS Bot

Comment 36 2013-09-13 22:08:21 PDT

Comment on attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html Attachment 211579 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1861301

EFL EWS Bot

Comment 37 2013-09-13 22:36:12 PDT

Comment on attachment 211602 [details] [PATCH] For Bots 2 Attachment 211602 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1817271

EFL EWS Bot

Comment 38 2013-09-13 22:47:22 PDT

Comment on attachment 211579 [details] [PATCH] Improved - All ports, update test, address InspectorOverlayPage.html Attachment 211579 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1878245

EFL EWS Bot

Comment 39 2013-09-14 00:20:54 PDT

Comment on attachment 211602 [details] [PATCH] For Bots 2 Attachment 211602 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1911235

Patrick R. Gansterer

Comment 40 2013-09-14 00:50:29 PDT

(In reply to comment #31) > Patrick Gansterer or Halton Huo, any ideas on how I could get what I want in the CMakeList file? Replace the " && " with "\nCOMMAND " is better style, since CMake can run more than one command for the target (http://www.cmake.org/cmake/help/v2.8.11/cmake.html#command:add_custom_command). But the problem is more that the redirection of the output ">" is missing in the CMakeLists.txt files. So no CMake problem AFAIKS ;-). I know that it's not your fault, but please also add the scripts (${WEBCORE_DIR}/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py and ${WEBCORE_DIR}/inspector/xxd.pl) to DEPENDS, so they get regenerated when the scripts change. Adding *.combined.html and *.min.js files to OUTPUT so a "make clean" will remove them. I'd create new add_custom_commands for the *.combined.html and *.min.js instead of adding them into the other one, but I have no strong opinion on it.

Joseph Pecoraro

Comment 41 2013-09-14 02:04:15 PDT

(In reply to comment #40) > (In reply to comment #31) > > Patrick Gansterer or Halton Huo, any ideas on how I could get what I want in the CMakeList file? > > Replace the " && " with "\nCOMMAND " is better style, since CMake can run more than one command for the target (http://www.cmake.org/cmake/help/v2.8.11/cmake.html#command:add_custom_command). Yah that was my first attempt. > But the problem is more that the redirection of the output ">" is missing in the CMakeLists.txt files. So no CMake problem AFAIKS ;-). Aarrrrgghhhhh!!!! Thanks for pointing that out. =) > I know that it's not your fault, but please also add the scripts (${WEBCORE_DIR}/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py and ${WEBCORE_DIR}/inspector/xxd.pl) to DEPENDS, so they get regenerated when the scripts change. Adding *.combined.html and *.min.js files to OUTPUT so a "make clean" will remove them. Good ideas. > I'd create new add_custom_commands for the *.combined.html and *.min.js instead of adding them into the other one, but I have no strong opinion on it. I'm going to stick with a few COMMANDs now, because it keeps the lines together, like other port builds.

Joseph Pecoraro

Comment 42 2013-09-14 02:09:28 PDT

Created attachment 211636 [details] [PATCH] For Bots 3

WebKit Commit Bot

Comment 43 2013-09-14 02:13:41 PDT

Attachment 211636 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/mac/inspector/console/command-line-api-expected.txt', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/DerivedSources.pri', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/inspector/InjectedScriptCanvasModuleSource.js', u'Source/WebCore/inspector/InjectedScriptSource.js', u'Source/WebCore/inspector/InspectorOverlayPage.css', u'Source/WebCore/inspector/InspectorOverlayPage.html', u'Source/WebCore/inspector/InspectorOverlayPage.js', u'Source/WebCore/inspector/Scripts/cssmin.py', u'Source/WebCore/inspector/Scripts/inline-and-minify-stylesheets-and-scripts.py', u'Source/WebCore/inspector/Scripts/jsmin.py']" exit_code: 1 Source/WebCore/inspector/Scripts/cssmin.py:28: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/cssmin.py:34: whitespace before ')' [pep8/E202] [5] Source/WebCore/inspector/Scripts/cssmin.py:33: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:35: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:44: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:49: multiple statements on one line (semicolon) [pep8/E702] [5] Source/WebCore/inspector/Scripts/jsmin.py:51: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:54: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:57: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:60: expected 2 blank lines, found 1 [pep8/E302] [5] Source/WebCore/inspector/Scripts/jsmin.py:64: expected 1 blank line, found 0 [pep8/E301] [5] Source/WebCore/inspector/Scripts/jsmin.py:78: at least two spaces before inline comment [pep8/E261] [5] Source/WebCore/inspector/Scripts/jsmin.py:139: too many blank lines (2) [pep8/E303] [5] Source/WebCore/inspector/Scripts/jsmin.py:163: too many blank lines (2) [pep8/E303] [5] Total errors found: 14 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 44 2013-09-14 02:27:53 PDT

Committed <http://trac.webkit.org/changeset/155763>.

Note You need to log in before you can comment on or make changes to this bug.