Async DNS: Don't watch hosts file on Android

Async DNS: Don't watch hosts file on Android

The hosts file on Android should never change (it's on the read-only
/system partition), and attempting to watch it also tries to watch /,
which triggers a SELinux denial in Android N.

Therefore, don't watch it on Android.

BUG=600442
Committed: https://crrev.com/a2e7ea61ce75af85c4479c5af292383e3cf08341
Cr-Commit-Position: refs/heads/master@{#391799}

[Julia Tuttle, 2016-04-27 14:41:49]

juliatuttle@chromium.org changed reviewers:
+ rdsmith@chromium.org

[Julia Tuttle, 2016-04-27 14:41:50]

PTAL, rdsmith.

Disabled the unittest that was hanging before, since it was testing for the
watching that we're no longer doing.

[Randy Smith (Not in Mondays), 2016-04-27 22:25:34]

If iOS doesn't have a host file, should be be running the test on iOS?

[Randy Smith (Not in Mondays), 2016-04-27 22:26:01]

On 2016/04/27 22:25:34, Randy Smith - Not in Fridays wrote:
> If iOS doesn't have a host file, should be be running the test on iOS?

Also, why is this marked WIP?  Should the description be fixed?

[Julia Tuttle, 2016-04-27 22:34:32]

Description was changed from

==========
WIP: Async DNS: Don't watch hosts file on Android

The hosts file on Android should never change (it's on the read-only
/system partition), and attempting to watch it also tries to watch /,
which triggers a SELinux denial in Android N.

Therefore, don't watch it on Android.

BUG=600442
==========

to

==========
Async DNS: Don't watch hosts file on Android

The hosts file on Android should never change (it's on the read-only
/system partition), and attempting to watch it also tries to watch /,
which triggers a SELinux denial in Android N.

Therefore, don't watch it on Android.

BUG=600442
==========

[Julia Tuttle, 2016-05-04 15:30:43]

We shouldn't be running the test on iOS, but that *entire section of the file*
was only running on Android anyway. (There's a giant ifdef. I'm checking with
pauljensen to see if all of the stuff on either side should really be
!OS_ANDROID- and OS_ANDROID-specific, respectively.)

Thus, removed that entire unittest, and also the code that was checking the
mtime of the hosts file.

Fixed description; thanks.

[Randy Smith (Not in Mondays), 2016-05-04 21:31:41]

LGTM modulo below comment nit.

https://codereview.chromium.org/1906573002/diff/100001/net/dns/dns_config_ser...
File net/dns/dns_config_service_posix.cc (right):

https://codereview.chromium.org/1906573002/diff/100001/net/dns/dns_config_ser...
net/dns/dns_config_service_posix.cc:86: #else  // !defined(OS_ANDROID) &&
!defined(OS_IOS) && !defined(OS_MACOSX)
Could you make clear in the comment (can move it off this line) why the below
section is !defined(OS_IOS)?  I figure it has to be because _posix.cc isn't
compiled on IOS, but that makes the comment seem misleading, since that isn't
because of the ifdef.

[Julia Tuttle, 2016-05-05 12:58:04]

Thanks, Randy!

https://codereview.chromium.org/1906573002/diff/100001/net/dns/dns_config_ser...
File net/dns/dns_config_service_posix.cc (right):

https://codereview.chromium.org/1906573002/diff/100001/net/dns/dns_config_ser...
net/dns/dns_config_service_posix.cc:86: #else  // !defined(OS_ANDROID) &&
!defined(OS_IOS) && !defined(OS_MACOSX)
On 2016/05/04 21:31:40, Randy Smith - Not in Fridays wrote:
> Could you make clear in the comment (can move it off this line) why the below
> section is !defined(OS_IOS)?  I figure it has to be because _posix.cc isn't
> compiled on IOS, but that makes the comment seem misleading, since that isn't
> because of the ifdef.

The first block in the ifdef is the defined(OS_IOS) case. I'll reorder them to
match.

[Julia Tuttle, 2016-05-05 12:58:11]

The CQ bit was checked by juliatuttle@chromium.org

[Julia Tuttle, 2016-05-05 12:58:12]

The patchset sent to the CQ was uploaded after l-g-t-m from rdsmith@chromium.org
Link to the patchset: https://codereview.chromium.org/1906573002/#ps120001
(title: "Comment tweak.")

[commit-bot: I haz the power, 2016-05-05 12:58:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1906573002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1906573002/120001

[commit-bot: I haz the power, 2016-05-05 13:56:48]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-05-05 13:58:21]

Description was changed from

==========
Async DNS: Don't watch hosts file on Android

The hosts file on Android should never change (it's on the read-only
/system partition), and attempting to watch it also tries to watch /,
which triggers a SELinux denial in Android N.

Therefore, don't watch it on Android.

BUG=600442
==========

to

==========
Async DNS: Don't watch hosts file on Android

The hosts file on Android should never change (it's on the read-only
/system partition), and attempting to watch it also tries to watch /,
which triggers a SELinux denial in Android N.

Therefore, don't watch it on Android.

BUG=600442

Committed: https://crrev.com/a2e7ea61ce75af85c4479c5af292383e3cf08341
Cr-Commit-Position: refs/heads/master@{#391799}
==========

[commit-bot: I haz the power, 2016-05-05 13:58:22]

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