Change the way that gzipped resources are loaded from resources.pak

Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091, 609219
Committed: https://crrev.com/49dab440c63881d09a313824a345ec1f8574b025
Cr-Commit-Position: refs/heads/master@{#413300}

[Dan Beam, 2016-07-15 01:40:06]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less resource bundle mmap'ing of
resources.pak and avoids potential collisions (though there was already
nifty code that'd throw if grit tried to compress something with a
header like that).

R=agreive@chromium.org, smaier@chromium.org
BUG=none
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less resource bundle mmap'ing of
resources.pak and avoids potential collisions (though there was already
nifty code that'd throw if grit tried to compress something with a
header like that).

R=agreive@chromium.org, smaier@chromium.org
BUG=none
==========

[Dan Beam, 2016-07-15 01:40:07]

dbeam@chromium.org changed reviewers:
+ agrieve@chromium.org
- agreive@chromium.org

[Dan Beam, 2016-07-15 02:23:23]

what do y'all think about this?

[Dan Beam, 2016-07-15 02:25:33]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 02:25:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-07-15 02:28:01]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less resource bundle mmap'ing of
resources.pak and avoids potential collisions (though there was already
nifty code that'd throw if grit tried to compress something with a
header like that).

R=agreive@chromium.org, smaier@chromium.org
BUG=none
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less resource bundle mmap'ing of
resources.pak and avoids potential collisions (though there was already
nifty code that'd throw if grit tried to compress something with a
header like that).

R=agreive@chromium.org, smaier@chromium.org
BUG=619091
==========

[Dan Beam, 2016-07-15 02:40:54]

btw, smaier@ and I were wondering whether all compress="gzip" resources were
requested by URLs. I /believe/ the answer is "yes"[1].

additionally, I think it's fairly easy to tell whether gzip is working:
1) Content-Encoding: gzip is added as a response header (though really not
necessary, because we short circuit the net/ stack, kind of just for local
debugging)
2) you can tell in chrome://net-internals[2]

[1] here's my audit:
https://gist.github.com/danbeam/13202765dcb59562de85538516600525
[2] here's the net-internals event log of loading chrome://signin-internals with
this CL: https://gist.github.com/danbeam/75d8fdd1880387c555b2eff9143b6506 (look
for filters)

https://codereview.chromium.org/2149323003/diff/1/content/public/browser/web_...
File content/public/browser/web_ui_data_source.h (right):

https://codereview.chromium.org/2149323003/diff/1/content/public/browser/web_...
content/public/browser/web_ui_data_source.h:88: virtual void
UseGzipForAllPaths() = 0;
also, I'm not married to this name (especially because of the implementation
special-casing the json path), but it felt like a decent starting point

[commit-bot: I haz the power, 2016-07-15 02:59:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 02:59:55]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Dan Beam, 2016-07-15 06:15:30]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 06:15:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 06:54:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 06:54:11]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[smaier, 2016-07-15 14:44:46]

smaier@chromium.org changed reviewers:
+ flackr@chromium.org

[smaier, 2016-07-15 14:44:47]

Adding flackr@, as he had some opinions on this change in the first iteration.

My primary concern is that both the .grd file and native code have to be updated
every time we want to compress a resource. Is this something we're okay with?

https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
File content/browser/webui/web_ui_data_source_impl.cc (right):

https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
content/browser/webui/web_ui_data_source_impl.cc:78: return
parent_->use_gzip_for_all_paths_ && path != parent_->json_path_;
Why do we care if we match the json_path_ of the parent?

https://codereview.chromium.org/2149323003/diff/20001/tools/grit/grit/node/in...
File tools/grit/grit/node/include.py (right):

https://codereview.chromium.org/2149323003/diff/20001/tools/grit/grit/node/in...
tools/grit/grit/node/include.py:97: if 'compress' in self.attrs and
self.attrs['compress'] == 'gzip':
Previously, one could reasonably add 'compress="gzip"' to a file which doesn't
go through the net stack whatsoever. I don't know how common (if at all) this
happens for resources in our .pak, but if it does happen, ideally we would have
some way of catching and erroing in this file if they did attempt to add
compression.

[agrieve, 2016-07-15 15:33:52]

On 2016/07/15 14:44:47, smaier wrote:
> Adding flackr@, as he had some opinions on this change in the first iteration.
> 
> My primary concern is that both the .grd file and native code have to be
updated
> every time we want to compress a resource. Is this something we're okay with?
> 
>
https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
> File content/browser/webui/web_ui_data_source_impl.cc (right):
> 
>
https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
> content/browser/webui/web_ui_data_source_impl.cc:78: return
> parent_->use_gzip_for_all_paths_ && path != parent_->json_path_;
> Why do we care if we match the json_path_ of the parent?
> 
>
https://codereview.chromium.org/2149323003/diff/20001/tools/grit/grit/node/in...
> File tools/grit/grit/node/include.py (right):
> 
>
https://codereview.chromium.org/2149323003/diff/20001/tools/grit/grit/node/in...
> tools/grit/grit/node/include.py:97: if 'compress' in self.attrs and
> self.attrs['compress'] == 'gzip':
> Previously, one could reasonably add 'compress="gzip"' to a file which doesn't
> go through the net stack whatsoever. I don't know how common (if at all) this
> happens for resources in our .pak, but if it does happen, ideally we would
have
> some way of catching and erroing in this file if they did attempt to add
> compression.

I do like using netstack here, and I think webui is big enough that it's worth
special-casing it.

The reason we didn't do it before was just that not everything goes through it.
E.g. IDR_NET_ERROR_HTML is the 4th largest resource, and it doesn't use webui:
https://cs.chromium.org/chromium/src/chrome/renderer/net/net_error_helper.cc?...

Perhaps it's worth doing it both ways, and adding a compress="webui"?

[Dan Beam, 2016-07-15 16:44:40]

fyi: the test failures are tests loading some internals pages differently that
users would :(

> how do we do this for resources without URLs?

welllllll, I personally think rather than special header + header detection +
transparent decompression, it'd be worthwhile to create a
ResourceBundle::DecompressResource() for the specific cases that we need to load
a gzipped resource (and not decompress it via a net::Filter).

also note: early uncompression of resources will continue to be a big deal for
webui as well, I'd guess.  specifically,  our new-ish $i18n{} preprocessor will
require this.

> can we only update a .grd and not have to touch C++?

I mean, I understand the appeal to this, but I don't think it's the end of the
world.  Lots of times a .grd change accompanies a C++ change (and vice versa). 
I guess it is a little more error-prone, though (many performance optimizations
make a tradeoff of less convenience).

https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
File content/browser/webui/web_ui_data_source_impl.cc (right):

https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
content/browser/webui/web_ui_data_source_impl.cc:78: return
parent_->use_gzip_for_all_paths_ && path != parent_->json_path_;
On 2016/07/15 14:44:47, smaier wrote:
> Why do we care if we match the json_path_ of the parent?

because things like string.js aren't gzipped

[mmenke, 2016-07-15 17:29:20]

Just from a net/ standpoint, and avoiding duplicating code, I think that this is
an improvement.

If adding the logic to the cc files to tag things as gzips seems like a bad
idea, could also look for gzip headers at the start of the bodies...Assuming you
never want the class to server still-compressed gzips, of course.

[Dan Beam, 2016-07-15 17:33:38]

another thing I learned from mmenke@ off thread: the data still gets
decompressed before it goes over the IPC tubes (I was hoping it was sent
compressed, oh well), but using net::Filter likely uses less memory and may be
less janky because it's smarter

[Dan Beam, 2016-07-15 18:45:08]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less resource bundle mmap'ing of
resources.pak and avoids potential collisions (though there was already
nifty code that'd throw if grit tried to compress something with a
header like that).

R=agreive@chromium.org, smaier@chromium.org
BUG=619091
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

R=agreive@chromium.org, smaier@chromium.org
BUG=619091
==========

[flackr, 2016-07-18 13:43:25]

I'm definitely in favor of removing the magic reserved header and encoding which
resources are gzipped.

On 2016/07/15 at 16:44:40, dbeam wrote:
> fyi: the test failures are tests loading some internals pages differently that
users would :(
> 
> > how do we do this for resources without URLs?
> 
> welllllll, I personally think rather than special header + header detection +
transparent decompression, it'd be worthwhile to create a
ResourceBundle::DecompressResource() for the specific cases that we need to load
a gzipped resource (and not decompress it via a net::Filter).

Agreed, I think this is better than using a reserved header. Another approach I
had brought up before was encoding into a generated header which resources are
compressed which could be consulted when loading resources but this would result
in either a different pack format or additional build artifacts which isn't
ideal.
> 
> also note: early uncompression of resources will continue to be a big deal for
webui as well, I'd guess.  specifically,  our new-ish $i18n{} preprocessor will
require this.
> 
> > can we only update a .grd and not have to touch C++?
> 
> I mean, I understand the appeal to this, but I don't think it's the end of the
world.  Lots of times a .grd change accompanies a C++ change (and vice versa). 
I guess it is a little more error-prone, though (many performance optimizations
make a tradeoff of less convenience).

Hopefully forgetting to change the C++ would usually cause an error in that
component's tests.
> 
>
https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
> File content/browser/webui/web_ui_data_source_impl.cc (right):
> 
>
https://codereview.chromium.org/2149323003/diff/20001/content/browser/webui/w...
> content/browser/webui/web_ui_data_source_impl.cc:78: return
parent_->use_gzip_for_all_paths_ && path != parent_->json_path_;
> On 2016/07/15 14:44:47, smaier wrote:
> > Why do we care if we match the json_path_ of the parent?
> 
> because things like string.js aren't gzipped

[Dan Beam, 2016-08-04 05:12:25]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 05:12:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-08-04 05:43:43]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 05:43:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-08-04 05:48:10]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[Dan Beam, 2016-08-04 05:48:24]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-08-04 05:48:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-08-04 05:48:36]

Patchset #3 (id:40001) has been deleted

[Dan Beam, 2016-08-04 05:55:20]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 05:55:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-08-04 05:59:00]

Patchset #3 (id:80001) has been deleted

[commit-bot: I haz the power, 2016-08-04 06:25:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-04 06:25:47]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[mmenke, 2016-08-15 18:06:28]

Is this CL going anywhere?  Just noticed that URLRequestSimpleJob::StartAsync
was a top cause of jank on all pre-release channels (Though not stable), and I
assume this is the cause.  A number of other classes use it, but none use
memory-mapped files.  Data URLs do a bit of work there, but seems less likely
the problem than this.

[Dan Beam, 2016-08-15 18:11:58]

On 2016/08/15 18:06:28, mmenke wrote:
> Is this CL going anywhere?  Just noticed that URLRequestSimpleJob::StartAsync
> was a top cause of jank on all pre-release channels (Though not stable), and I
> assume this is the cause.  A number of other classes use it, but none use
> memory-mapped files.  Data URLs do a bit of work there, but seems less likely
> the problem than this.

It's boldly going somewhere slowly.

I think the current blocker is that: WebUIResourceBrowserTest.MediaInternals_*
tests are loading resources in a way that bypasses decompression (i.e. they'd
need to load them from some chrome:// URL, but instead are loading them in such
a way that the gzipped contents are run as JS).

I've slowly been working on fixes (as you can see the newest patchset is 3w
newer than when I posted this CL).

Maybe I can get to this soon, but it's lower on the priority totem poll.

[Dan Beam, 2016-08-16 21:06:47]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 21:07:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 22:11:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-16 22:11:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dan Beam, 2016-08-16 22:21:37]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 22:22:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 23:51:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-16 23:51:51]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-08-17 00:01:14]

dbeam@chromium.org changed reviewers:
- agrieve@chromium.org, flackr@chromium.org, smaier@chromium.org

[Dan Beam, 2016-08-17 01:48:36]

dbeam@chromium.org changed reviewers:
+ creis@chromium.org, mmenke@chromium.org, sky@chromium.org

[Dan Beam, 2016-08-17 01:48:37]

Allllrighty then, got things working.

+creis@ for content/
+mmenke@ for net/
+sky@ for ui/, chrome/

smaier@/agrieve@ to CC (feel free to chime in)

[Dan Beam, 2016-08-17 01:51:19]

dbeam@chromium.org changed reviewers:
+ pfeldman@chromium.org
- creis@chromium.org

[Dan Beam, 2016-08-17 01:51:19]

-creis@ (ooo), +pfeldman for content/

[Dan Beam, 2016-08-17 01:51:45]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

R=agreive@chromium.org, smaier@chromium.org
BUG=619091
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091
==========

[Dan Beam, 2016-08-17 01:54:13]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091,609219
==========

[smaier, 2016-08-17 13:59:11]

On 2016/08/17 01:51:19, Dan Beam wrote:
> -creis@ (ooo), +pfeldman for content/

lgtm

[sky, 2016-08-17 15:34:32]

LGTM

[mmenke, 2016-08-18 14:40:36]

net/ and the use of Filter LGTM.

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
File content/public/test/browser_test_utils.cc (right):

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:383: // TODO(dbeam): why do we even
have this lever^Wparameter?
^W?

I assume it's so ReadMore can continue where it left off.  See 
GZipFilter::CheckGZipHeader -> GZipFilter::DoInflate

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:384: const char* header_end = NULL;
nullptr

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:395: std::unique_ptr<net::Filter>
filter = net::Filter::GZipFactory();
Need to make net::Filter NET_EXPORT (It's NET_EXPORT_PRIVATE, currently)

[Dan Beam, 2016-08-18 20:56:51]

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
File content/public/test/browser_test_utils.cc (right):

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:383: // TODO(dbeam): why do we even
have this lever^Wparameter?
On 2016/08/18 14:40:35, mmenke wrote:
> ^W?

means "remove word"
https://en.wikipedia.org/wiki/Backspace#.5EW_and_.5EU

> 
> I assume it's so ReadMore can continue where it left off.  See 
> GZipFilter::CheckGZipHeader -> GZipFilter::DoInflate

ah, i understand now.  comment removed

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:384: const char* header_end = NULL;
On 2016/08/18 14:40:36, mmenke wrote:
> nullptr

Done.

https://codereview.chromium.org/2149323003/diff/180001/content/public/test/br...
content/public/test/browser_test_utils.cc:395: std::unique_ptr<net::Filter>
filter = net::Filter::GZipFactory();
On 2016/08/18 14:40:36, mmenke wrote:
> Need to make net::Filter NET_EXPORT (It's NET_EXPORT_PRIVATE, currently)

Done.

[Dan Beam, 2016-08-18 21:02:22]

Patchset #9 (id:220001) has been deleted

[Dan Beam, 2016-08-18 21:10:41]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 21:11:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 23:10:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 23:10:53]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dan Beam, 2016-08-19 03:19:40]

ping => pfeldman@


FYI: I've found a minor issue with this patch -- we sometimes do substitutions
on the entire, decoded contents of a file, changing:

  <title>$i18n{title}</title>

to this

  <title>Translation Of Title</title>

before being sent to the renderer.

PROBLEM: that no longer works for gzipped resources, and the streaming gzip only
sends parts of the file to the renderer (i.e. we're never guaranteed to have the
whole decoded contents, which the current template expressions replacement
system assumes).

HOWEVER, none of the currently gzipped pages use this $i18n{} mechanism (yet). 
Nor does the history page on Android, which would also benefit from this[1].

THE PLAN IS: yours truly writes a streaming implementation of $i18n{} (or
perhaps find a poor soul^W^Wwilling volunteer).

IN THE FUTURE: I'd like to gzip both downloads and history's vulcanized output
if we find this helpful.  These are big blobs of html/css and js (separately),
and they would probably both benefit from being gzipped on-disk.

[Dan Beam, 2016-08-19 03:19:55]

[1] https://codereview.chromium.org/2152143002/

[Dan Beam, 2016-08-19 03:22:14]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 03:22:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 04:42:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 04:42:18]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pfeldman, 2016-08-19 18:14:54]

Code looks good, but relying upon no $i18n templating for the gzipped htmls is
no good. Do you want to land that streaming bit before this goes in?

[pfeldman, 2016-08-19 18:26:31]

lgtm given it gets renamed to DisableI18nAndUseGzipForAllPaths for clarity.

[Dan Beam, 2016-08-19 19:13:12]

On 2016/08/19 18:26:31, pfeldman wrote:
> lgtm given it gets renamed to DisableI18nAndUseGzipForAllPaths for clarity.

done! thanks for the great, simple suggestion!

[Dan Beam, 2016-08-19 19:18:23]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-08-19 19:18:25]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
smaier@chromium.org, mmenke@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2149323003/#ps250030
(title: "rename")

[commit-bot: I haz the power, 2016-08-19 19:18:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 19:25:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 19:25:50]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2016-08-19 19:40:37]

dbeam@chromium.org changed reviewers:
+ thestig@chromium.org

[Dan Beam, 2016-08-19 19:40:38]

+thestig@ for tools/grit

[Lei Zhang, 2016-08-19 22:27:14]

lgtm

[Dan Beam, 2016-08-19 22:53:07]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-08-19 22:54:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-20 00:50:12]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091,609219
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091,609219
==========

[commit-bot: I haz the power, 2016-08-20 00:50:14]

Committed patchset #11 (id:250030)

[commit-bot: I haz the power, 2016-08-20 00:53:14]

Description was changed from

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091,609219
==========

to

==========
Change the way that gzipped resources are loaded from resources.pak

Before, gzipped resources were given a special file header and resource
bundles inspected the first couple bytes to determine if the resources
was gzipped and expanded it on the current thread and responded with
the decompressed resource transparently.

This is pretty awesome.

But you know what's cooler?

Transmitting the resource gzipped (reduces over IPC size) and using the
same code that handles remote network resources with:

  Content-Encoding: gzip

via net::GZipFilter. This also dirties less memory, avoids potential
collisions (though there was already nifty code that'd throw if grit
tried to compress something with a header like that), and adds less
code for a more tried and true, weathered gzip implementation.

BUG=619091,609219

Committed: https://crrev.com/49dab440c63881d09a313824a345ec1f8574b025
Cr-Commit-Position: refs/heads/master@{#413300}
==========

[commit-bot: I haz the power, 2016-08-20 00:53:16]

Patchset 11 (id:??) landed as
https://crrev.com/49dab440c63881d09a313824a345ec1f8574b025
Cr-Commit-Position: refs/heads/master@{#413300}