Support running ChromeVox in uncompressed mode and compress ChromeVox next by default.

Support running ChromeVox in uncompressed mode and compress ChromeVox next by default.

By adding chromevox_compress_js=0 to your GYP_DEFINES, you can run
ChromeVox directly from its sources. The source files are symlinked
into the extension directory in the output directory.  This is good for
faster development and debugging, but ChromeVox itself becomes slow
to load in this mode.

This also changes ChromeVox next to follow the same flag, hence being
compressed by default.  When doing so, the classic background
page and ChromeVox next's background page are compiled together instead
of being loaded one after the other. This is necessary because they import
the same modules and the ones from Chromevox next will overwrite
the ones from classic after classic gets initialized.

Some of the targets in chromevox.gyp are consolidated.  The main reason is
because many targets that copy or compress scripts need to have the whole
picture to not copy over the same files more than once
which is racy.

BUG=421136
Committed: https://crrev.com/c7c386446db323e3b5df17c162a24d3bc5330fb7
Cr-Commit-Position: refs/heads/master@{#299876}

[Peter Lundblad, 2014-10-14 14:48:04]

plundblad@chromium.org changed reviewers:
+ dtseng@chromium.org

[Peter Lundblad, 2014-10-14 14:48:05]

Caveat: if you want to try this and switch to uncompressed chromevox, remember
to clear out out/*/resources/chromeos/chromevox first. This is needed since the
html loader files load both uncompiled and compiled scritps if present and that
can only be bad.

[David Tseng, 2014-10-14 23:10:11]

Some comments below.

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/chromevox.gyp (right):

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:15:
'chromevox2_background_script_loader_file': 'cvox2/background/loader.js',
nit: sort

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:55:
'chromevox1_options_script',
nit: sort

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:60: 'dependencies': [
# else chromevox_compress_js==0

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:105: 'conditions': [
What happened to api_util.js above? It should always be included as part of web
accessible resources which you didn't change.

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:154:
'<(chromevox2_background_script_loader_file)',
nit: sort

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox.gyp:248: 'targets': [
nit: # else chromevox_compress_js==0

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:161:
var listOfFiles;
I think this can now fit above.

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi (right):

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi:17: #
chromevox_compress_js: 1 or 0; whether the javascript is compressed.
nit: Next

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi:18: 
nit: convention of is_* prefix for boolean vars?

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/tools/generate_manifest.py
(right):

https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/tools/generate_manifest.py:51:
parser.add_option(
nit: Next

[Peter Lundblad, 2014-10-15 14:31:03]

dtseng@chromium.org writes:
> Some comments below.
> 
> 
> 
>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> File chrome/browser/resources/chromeos/chromevox/chromevox.gyp (right):
> 
>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:15:
> 'chromevox2_background_script_loader_file':
> 'cvox2/background/loader.js',
> nit: sort

Done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:55:
> 'chromevox1_options_script',
> nit: sort

Done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:60:
> 'dependencies': [
> # else chromevox_compress_js==0

Done without the else to follow other files.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:105:
> 'conditions': [
> What happened to api_util.js above? It should always be included as part
> of web accessible resources which you didn't change.

Added comment, it is copied in the uncompressed case because it is
required by one of the files then, but needs to be copied in the compressed
case.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:154:
> '<(chromevox2_background_script_loader_file)',
> nit: sort

Done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/chromevox.gyp:248:
> 'targets': [
> nit: # else chromevox_compress_js==0

Done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> File
>
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
> (right):
> 
>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
>
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:161:
> var listOfFiles;
> I think this can now fit above.

It did, done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> File chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi
> (right):
> 
>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi:17: #
> chromevox_compress_js: 1 or 0; whether the javascript is compressed.
> nit: Next

Done.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi:18:
> nit: convention of is_* prefix for boolean vars?

Not sure what you're asking.  I am not aware of such a convention in gyp
files.

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> File
> chrome/browser/resources/chromeos/chromevox/tools/generate_manifest.py
> (right):
> 
>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/tools/generate_manifest.py:51:
> parser.add_option(
> nit: Next

Done.

> https://codereview.chromium.org/656813002/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2014-10-15 15:25:15]

LGTM

On Wed, Oct 15, 2014 at 7:31 AM, <plundblad@chromium.org> wrote:

> dtseng@chromium.org writes:
> > Some comments below.
>

>
https://codereview.chromium.org/656813002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/generate_manifest.gypi:18:
> nit: convention of is_* prefix for boolean vars?

Not sure what you're asking.  I am not aware of such a convention in gyp

Just saw you changed use_chromevox_next to is_chromevox_next, so thought
perhaps we wanted to use is_* for most things in the future including
is_compressed etc (which could simplify the file further); also dropping
the chromevox part whenever not ambiguous since this is all chromevox
(except when specific to next or classic).

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2014-10-15 15:45:18]

lgtm

[Peter Lundblad, 2014-10-16 10:52:21]

The CQ bit was checked by plundblad@chromium.org

[commit-bot: I haz the power, 2014-10-16 10:53:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/656813002/40001

[commit-bot: I haz the power, 2014-10-16 11:20:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-16 11:21:26]

Patchset 3 (id:??) landed as
https://crrev.com/c7c386446db323e3b5df17c162a24d3bc5330fb7
Cr-Commit-Position: refs/heads/master@{#299876}