Disable various unused libxml features: catalogs, DocBook, legacy APIs, etc.

Disable various unused libxml features: catalogs, DocBook, legacy APIs, etc.

See third_party/libxml/README.chromium for the list.

BUG=604167
Review-Url: https://codereview.chromium.org/2788063002
Cr-Commit-Position: refs/heads/master@{#461657}
Committed: https://chromium.googlesource.com/chromium/src/+/519ead74e98302eabc1144ed9947e14d93adae95

[dominicc (has gone to gerrit), 2017-03-31 05:25:40]

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

[commit-bot: I haz the power, 2017-03-31 05:25:58]

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

[dominicc (has gone to gerrit), 2017-03-31 06:29:10]

dominicc@chromium.org changed reviewers:
+ dcheng@chromium.org

[dominicc (has gone to gerrit), 2017-03-31 06:29:10]

PTAL

Most interesting thing here is disabling catalogs. Blink actively tries to turn
that off anyway. The utility process won't be able to read catalog files I
assume.

[commit-bot: I haz the power, 2017-03-31 08:21:35]

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

[commit-bot: I haz the power, 2017-03-31 08:21:36]

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

[dominicc (has gone to gerrit), 2017-03-31 14:19:01]

On 2017/03/31 at 06:29:10, dominicc wrote:
> PTAL
> 
> Most interesting thing here is disabling catalogs. Blink actively tries to
turn that off anyway. The utility process won't be able to read catalog files I
assume.

Looks like Windows needs rebaselining. I should investigate where these error
messages about empty documents stem from.

[dcheng, 2017-03-31 18:03:03]

The CL lgtm -- do we have any way to test that these are disabled though?

[dcheng, 2017-03-31 18:03:55]

Btw, an incidental and not entirely related question: should we try to encourage
people to use the SAX parser rather than XMLReader?

[dominicc (has gone to gerrit), 2017-04-01 00:23:52]

> do we have any way to test that these are disabled though?

We don't have any tests. What do you have in mind? I guess we could have #ifdef
not_wanted_thing/#error. Would defining one of the structs which is #ifdefed out
break ODR and be detected if they came back?

On 2017/03/31 at 18:03:55, dcheng wrote:
> Btw, an incidental and not entirely related question: should we try to
encourage people to use the SAX parser rather than XMLReader?

Probably yes in the spirit of limiting dependencies. However that comes with
this caveat: don't set up a getParsedEntity callback. And there are probably
other options we need people to avoid.

[dcheng, 2017-04-01 00:26:03]

On 2017/04/01 00:23:52, dominicc wrote:
> > do we have any way to test that these are disabled though?
> 
> We don't have any tests. What do you have in mind? I guess we could have
#ifdef
> not_wanted_thing/#error. Would defining one of the structs which is #ifdefed
out
> break ODR and be detected if they came back?

I guess I was wondering if we could try using these features (e.g. XML
catalogs), and have runtime tests verifying that they don't work.

> 
> On 2017/03/31 at 18:03:55, dcheng wrote:
> > Btw, an incidental and not entirely related question: should we try to
> encourage people to use the SAX parser rather than XMLReader?
> 
> Probably yes in the spirit of limiting dependencies. However that comes with
> this caveat: don't set up a getParsedEntity callback. And there are probably
> other options we need people to avoid.

[dcheng, 2017-04-01 00:26:31]

On 2017/04/01 00:26:03, dcheng wrote:
> On 2017/04/01 00:23:52, dominicc wrote:
> > > do we have any way to test that these are disabled though?
> > 
> > We don't have any tests. What do you have in mind? I guess we could have
> #ifdef
> > not_wanted_thing/#error. Would defining one of the structs which is #ifdefed
> out
> > break ODR and be detected if they came back?
> 
> I guess I was wondering if we could try using these features (e.g. XML
> catalogs), and have runtime tests verifying that they don't work.
> 
> > 
> > On 2017/03/31 at 18:03:55, dcheng wrote:
> > > Btw, an incidental and not entirely related question: should we try to
> > encourage people to use the SAX parser rather than XMLReader?
> > 
> > Probably yes in the spirit of limiting dependencies. However that comes with
> > this caveat: don't set up a getParsedEntity callback. And there are probably
> > other options we need people to avoid.

(I'm not sure my question makes any sense, given my unfamiliarity with libxml,
so feel free to correct me)

[dominicc (has gone to gerrit), 2017-04-03 07:30:05]

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

[commit-bot: I haz the power, 2017-04-03 07:30:15]

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

[dominicc (has gone to gerrit), 2017-04-03 07:32:41]

Description was changed from

==========
Disable XML catalogs, DocBook, canonicalization and legacy libxml APIs.

BUG=604167
==========

to

==========
Disable various unused libxml features: catalogs, DocBook, legacy APIs, etc.

See third_party/libxml/README.chromium for the list.

BUG=604167
==========

[dominicc (has gone to gerrit), 2017-04-03 07:42:29]

PTAL, this has changed a bit.

I've added some things to this list: XPointer (we already removed range-to in
r403859, and navigating to XPointer anchors was never wired up); regexp and the
automata it uses (I guess I need to delete libxml_xml_regexp_compile_fuzzer?
mmoroz?); SCHEMATRON; etc.

As far as I can tell none of this stuff is used but there's a degree of
guesswork here.

Re: catalogs specifically, catalogs are bad because they will expose platform
differences; but since we don't do DTD validation all the catalogs I found on my
Linux box looked harmless/useless because they don't define entities. FWIW Blink
was blocking those anyway so this is web compatible.

I changed the tests for default catalog URLs to be CHECKs. Not ideal because you
can trivially trigger it with document('file:///etc/catalog') or something, but
that was never going to load anything useful anyway.

[commit-bot: I haz the power, 2017-04-03 08:03:42]

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

[commit-bot: I haz the power, 2017-04-03 08:03:42]

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

[mmoroz, 2017-04-03 15:15:57]

On 2017/04/03 07:42:29, dominicc wrote:
> PTAL, this has changed a bit.
> 
> I've added some things to this list: XPointer (we already removed range-to in
> r403859, and navigating to XPointer anchors was never wired up); regexp and
the
> automata it uses (I guess I need to delete libxml_xml_regexp_compile_fuzzer?
> mmoroz?); SCHEMATRON; etc.
> 
> As far as I can tell none of this stuff is used but there's a degree of
> guesswork here.
> 
> Re: catalogs specifically, catalogs are bad because they will expose platform
> differences; but since we don't do DTD validation all the catalogs I found on
my
> Linux box looked harmless/useless because they don't define entities. FWIW
Blink
> was blocking those anyway so this is web compatible.
> 
> I changed the tests for default catalog URLs to be CHECKs. Not ideal because
you
> can trivially trigger it with document('file:///etc/catalog') or something,
but
> that was never going to load anything useful anyway.

Removing libxml_xml_regexp_compile_fuzzer since we don't use regexp -- SGTM.
Thanks a lot!

[dcheng, 2017-04-03 19:39:13]

On 2017/04/03 07:42:29, dominicc wrote:
> PTAL, this has changed a bit.
> 
> I've added some things to this list: XPointer (we already removed range-to in
> r403859, and navigating to XPointer anchors was never wired up); regexp and
the
> automata it uses (I guess I need to delete libxml_xml_regexp_compile_fuzzer?
> mmoroz?); SCHEMATRON; etc.
> 
> As far as I can tell none of this stuff is used but there's a degree of
> guesswork here.
> 
> Re: catalogs specifically, catalogs are bad because they will expose platform
> differences; but since we don't do DTD validation all the catalogs I found on
my
> Linux box looked harmless/useless because they don't define entities. FWIW
Blink
> was blocking those anyway so this is web compatible.
> 
> I changed the tests for default catalog URLs to be CHECKs. Not ideal because
you
> can trivially trigger it with document('file:///etc/catalog') or something,
but
> that was never going to load anything useful anyway.

Still LGTM

I think I was hoping we could write unit tests which would behave differently in
the presence of these features being enabled, but if that's hard, hopefully
libxml's config defines are semi-reliable.

[dominicc (has gone to gerrit), 2017-04-04 01:51:09]

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

[commit-bot: I haz the power, 2017-04-04 01:51:38]

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

[dominicc (has gone to gerrit), 2017-04-04 01:52:00]

dominicc@chromium.org changed reviewers:
+ mmoroz@chromium.org

[dominicc (has gone to gerrit), 2017-04-04 01:52:00]

+mmoroz, PTAL testing/libfuzzer

[commit-bot: I haz the power, 2017-04-04 05:33:14]

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

[commit-bot: I haz the power, 2017-04-04 05:33:15]

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

[mmoroz, 2017-04-04 07:23:30]

LGTM, thanks!

[dominicc (has gone to gerrit), 2017-04-04 08:00:23]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2017-04-04 08:00:23]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2788063002/#ps40001
(title: "Remove files from build, remove regexp fuzzer.")

[commit-bot: I haz the power, 2017-04-04 08:00:33]

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, 2017-04-04 08:04:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491292823167400,
"parent_rev": "de33c94cc211c5ee1994ce3fa0d43abc355a4e9c", "commit_rev":
"519ead74e98302eabc1144ed9947e14d93adae95"}

[commit-bot: I haz the power, 2017-04-04 08:05:15]

Description was changed from

==========
Disable various unused libxml features: catalogs, DocBook, legacy APIs, etc.

See third_party/libxml/README.chromium for the list.

BUG=604167
==========

to

==========
Disable various unused libxml features: catalogs, DocBook, legacy APIs, etc.

See third_party/libxml/README.chromium for the list.

BUG=604167

Review-Url: https://codereview.chromium.org/2788063002
Cr-Commit-Position: refs/heads/master@{#461657}
Committed:
https://chromium.googlesource.com/chromium/src/+/519ead74e98302eabc1144ed9947...
==========

[commit-bot: I haz the power, 2017-04-04 08:05:16]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/519ead74e98302eabc1144ed9947...