[Extensions] Validate file existence for chrome url overrides

[Extensions] Validate file existence for chrome url overrides

BUG=612320
Committed: https://crrev.com/005e330d08fcf87393e8c996b06161b18823c117
Cr-Commit-Position: refs/heads/master@{#394531}

[Devlin, 2016-05-17 19:32:07]

Patchset #1 (id:1) has been deleted

[Devlin, 2016-05-17 19:34:24]

rdevlin.cronin@chromium.org changed reviewers:
+ asargent@chromium.org

[Devlin, 2016-05-17 19:34:24]

This isn't perfect.  Two issues:
- This won't help if the file is destroyed during chrome running - the user will
have to restart chrome.  But, that's a lot better than a perma-broken NTP.
- When an extension fails to load, we don't show it anywhere.  It just silently
drops off the radar.  This spans more than just this issue, but I wonder if we
should make them visible in chrome://extensions ("The following extensions
couldn't be loaded: ").

[asargent_no_longer_on_chrome, 2016-05-17 21:20:59]

lgtm

We should keep an eye out for performance impacts, since this introduces new
disk read activity in some cases (eg for a NTP-overriding extension, I think we
wouldn't try to read files from the extension directory until the user opened
the NTP). 

If there are performance problems, one crazy idea is that we could instead just
watch for load failures of override pages in extension_protocols.cc and if we
see one, treat it as a content verification failure, which pretty much has a UI
pretty close to what we'd want for this (extension gets disabled, user is
offered option to repair on chrome://extensions).

[Devlin, 2016-05-18 19:13:20]

On 2016/05/17 21:20:59, Antony Sargent wrote:
> lgtm
> 
> We should keep an eye out for performance impacts, since this introduces new
> disk read activity in some cases (eg for a NTP-overriding extension, I think
we
> wouldn't try to read files from the extension directory until the user opened
> the NTP). 
> 
> If there are performance problems, one crazy idea is that we could instead
just
> watch for load failures of override pages in extension_protocols.cc and if we
> see one, treat it as a content verification failure, which pretty much has a
UI
> pretty close to what we'd want for this (extension gets disabled, user is
> offered option to repair on chrome://extensions).

I'm optimistic that this shouldn't hurt us much - this doesn't read the whole
file, and we check a lot of other files for existence, too.  But we can keep an
eye out.  I was wondering if we'd also want to do something in
extension_protocols to address this case (since that would help during runtime),
but that's a much more complicated patch.

[Devlin, 2016-05-18 20:19:29]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-05-18 20:20:06]

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

[commit-bot: I haz the power, 2016-05-18 20:26:48]

Committed patchset #1 (id:20001)

[commit-bot: I haz the power, 2016-05-18 20:28:06]

Description was changed from

==========
[Extensions] Validate file existence for chrome url overrides

BUG=612320
==========

to

==========
[Extensions] Validate file existence for chrome url overrides

BUG=612320

Committed: https://crrev.com/005e330d08fcf87393e8c996b06161b18823c117
Cr-Commit-Position: refs/heads/master@{#394531}
==========

[commit-bot: I haz the power, 2016-05-18 20:28:06]

Patchset 1 (id:??) landed as
https://crrev.com/005e330d08fcf87393e8c996b06161b18823c117
Cr-Commit-Position: refs/heads/master@{#394531}