Bug 121183 - Web Inspector: InjectedScriptSource_js and InjectedScriptCanvasModuleSource_js should be minified
Summary: Web Inspector: InjectedScriptSource_js and InjectedScriptCanvasModuleSource_j...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
: 121270 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-09-11 14:49 PDT by Ryosuke Niwa
Modified: 2013-09-14 02:27 PDT (History)
19 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Radar WebKit Bug Importer 2013-09-11 14:49:43 PDT
<rdar://problem/14969065>
Comment 2 Radar WebKit Bug Importer 2013-09-11 14:49:43 PDT
<rdar://problem/14969066>
Comment 3 Timothy Hatcher 2013-09-11 15:54:08 PDT
Applies to InjectedScriptSource_js too, which is also large.
Comment 4 Joseph Pecoraro 2013-09-12 16:20:59 PDT
Also InspectorOverlayPage_html (which contains <style> and <script> that could be minified).
Comment 5 Joseph Pecoraro 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.
Comment 6 Joseph Pecoraro 2013-09-12 17:11:02 PDT
Created attachment 211493 [details]
[PATCH] Proposed Fix
Comment 7 Joseph Pecoraro 2013-09-12 17:11:38 PDT
Comment on attachment 211493 [details]
[PATCH] Proposed Fix

Oops, forgot ChangeLog.
Comment 8 WebKit Commit Bot 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.
Comment 9 Joseph Pecoraro 2013-09-12 17:15:25 PDT
Created attachment 211494 [details]
[PATCH] Proposed Fix
Comment 10 WebKit Commit Bot 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.
Comment 11 Joseph Pecoraro 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]
Comment 12 Joseph Pecoraro 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
Comment 13 WebKit Commit Bot 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
Comment 14 WebKit Commit Bot 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
Comment 15 Build Bot 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
Comment 16 Build Bot 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
Comment 17 Joseph Pecoraro 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")
Comment 18 Joseph Pecoraro 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.
Comment 19 Build Bot 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
Comment 20 Build Bot 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
Comment 21 Joseph Pecoraro 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.
Comment 22 Joseph Pecoraro 2013-09-13 13:05:14 PDT
*** Bug 121270 has been marked as a duplicate of this bug. ***
Comment 23 Joseph Pecoraro 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.
Comment 24 Early Warning System Bot 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
Comment 25 WebKit Commit Bot 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.
Comment 26 Early Warning System Bot 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
Comment 27 Joseph Pecoraro 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.
Comment 28 Joseph Pecoraro 2013-09-13 13:24:22 PDT
Created attachment 211582 [details]
[PATCH] For Bots 1
Comment 29 WebKit Commit Bot 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.
Comment 30 Joseph Pecoraro 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?
Comment 31 Joseph Pecoraro 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?
Comment 32 Joseph Pecoraro 2013-09-13 16:34:31 PDT
Created attachment 211602 [details]
[PATCH] For Bots 2
Comment 33 WebKit Commit Bot 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.
Comment 34 Joseph Pecoraro 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]
Comment 35 Joseph Pecoraro 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?
Comment 36 EFL EWS Bot 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
Comment 37 EFL EWS Bot 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
Comment 38 EFL EWS Bot 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
Comment 39 EFL EWS Bot 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
Comment 40 Patrick R. Gansterer 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.
Comment 41 Joseph Pecoraro 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.
Comment 42 Joseph Pecoraro 2013-09-14 02:09:28 PDT
Created attachment 211636 [details]
[PATCH] For Bots 3
Comment 43 WebKit Commit Bot 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.
Comment 44 Joseph Pecoraro 2013-09-14 02:27:53 PDT
Committed <http://trac.webkit.org/changeset/155763>.