Bug 106210 - [EFL][CMAKE] Compress resource files of inspector
Summary: [EFL][CMAKE] Compress resource files of inspector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-06 23:47 PST by Seokju Kwon
Modified: 2013-01-17 10:39 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.71 KB, patch)
2013-01-08 07:27 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (1.79 KB, patch)
2013-01-09 02:43 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (1.79 KB, patch)
2013-01-09 16:39 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (1.85 KB, patch)
2013-01-17 00:31 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seokju Kwon 2013-01-06 23:47:49 PST
Add compressing JavaScript
for smaller package and faster connection of remote web inspector.

-219 files (Default JavaScript files of inspector except derived file and localizedString file.)
-Reduced size : Approximately 1.39MB (3.28MB -> 1.89MB with default option)
Comment 1 Seokju Kwon 2013-01-08 07:27:46 PST
Created attachment 181691 [details]
Patch
Comment 2 Ryuan Choi 2013-01-08 18:47:22 PST
Comment on attachment 181691 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181691&action=review

> Source/PlatformEfl.cmake:16
> +        file(GLOB all_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")

Doesn't we need to compress other js file?

> Source/PlatformEfl.cmake:19
> +            add_custom_command(

I think that it should have dependency to execute this after copied js files.
Comment 3 Seokju Kwon 2013-01-08 19:16:18 PST
(In reply to comment #2)
> (From update of attachment 181691 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181691&action=review
> 
> > Source/PlatformEfl.cmake:16
> > +        file(GLOB all_js_files "${WEBCORE_DIR}/inspector/front-end/*.js")
> 
> Doesn't we need to compress other js file?
In an initial patch, I just want to compress JavaScripts related to inspector except derived file and localizedString file. No special reason, (but it seems that we can't access derived source at this time.)
I will add them.

> 
> > Source/PlatformEfl.cmake:19
> > +            add_custom_command(
> 
> I think that it should have dependency to execute this after copied js files.
Does this command become part of the target with "TARGET web-inspector-resources"
(Need POST_BUILD?)
Comment 4 Seokju Kwon 2013-01-09 00:55:55 PST
I will make it to be part of installation in the next patch.
lgombos, Thanks for your feedback
Comment 5 Seokju Kwon 2013-01-09 02:43:58 PST
Created attachment 181884 [details]
Patch
Comment 6 Seokju Kwon 2013-01-09 16:39:51 PST
Created attachment 182009 [details]
Patch
Comment 7 Gyuyoung Kim 2013-01-16 03:18:43 PST
Comment on attachment 182009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182009&action=review

Laszlo and Ryuan, any other comments ?

> ChangeLog:3
> +        [EFL] Compress resource files of inspector

Please add [CMAKE] prefix as well.
Comment 8 Gyuyoung Kim 2013-01-16 17:57:24 PST
Comment on attachment 182009 [details]
Patch

r=me, but please fix my comment.
Comment 9 Seokju Kwon 2013-01-17 00:31:36 PST
Created attachment 183133 [details]
Patch
Comment 10 Seokju Kwon 2013-01-17 00:33:09 PST
(In reply to comment #8)
> (From update of attachment 182009 [details])
> r=me, but please fix my comment.

Done(In reply to comment #8)
> (From update of attachment 182009 [details])
> r=me, but please fix my comment.

Done. Thanks :)
Comment 11 Chris Dumez 2013-01-17 00:34:03 PST
(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 182009 [details] [details])
> > r=me, but please fix my comment.
> 
> Done(In reply to comment #8)
> > (From update of attachment 182009 [details] [details])
> > r=me, but please fix my comment.
> 
> Done. Thanks :)

You just need to ask for cq?, not r? since gyuyoung r+'d it already and you added his name in the Changelog.
Comment 12 Seokju Kwon 2013-01-17 01:31:40 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > (From update of attachment 182009 [details] [details] [details])
> > > r=me, but please fix my comment.
> > 
> > Done(In reply to comment #8)
> > > (From update of attachment 182009 [details] [details] [details])
> > > r=me, but please fix my comment.
> > 
> > Done. Thanks :)
> 
> You just need to ask for cq?, not r? since gyuyoung r+'d it already and you added his name in the Changelog.

seems that webkit bot is not working. :(
Comment 13 WebKit Review Bot 2013-01-17 10:39:20 PST
Comment on attachment 183133 [details]
Patch

Clearing flags on attachment: 183133

Committed r139992: <http://trac.webkit.org/changeset/139992>
Comment 14 WebKit Review Bot 2013-01-17 10:39:25 PST
All reviewed patches have been landed.  Closing bug.