Check that registry external extension records point to a readable CRX file.

Check that registry external extension records point to a readable CRX file.

Some refactoring to make all error paths consistent.

BUG=81687
TEST=Create registry values for every error case, watch that I can hit them in the debugger, and a key without any errors leads to an install.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=97134

[Finnur, 2011-08-16 10:39:32]

LGTM, with nits.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:1: //
Copyright (c) 2010 The Chromium Authors. All rights reserved.
nit: 2011

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:89:
LOG(ERROR) << "Path " << extension_path_str
nit: Path -> File

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:90: << " for
key " << key_path << " can not be read.  "
nit: One space before second closing double quote.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:101: << "
for key " << key_path << " .";
nit: Why space before period? (same below, two occurrences).

[Sam Kerner (Chrome), 2011-08-16 18:25:52]

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:1: //
Copyright (c) 2010 The Chromium Authors. All rights reserved.
On 2011/08/16 10:39:33, Finnur wrote:
> nit: 2011

Done.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:89:
LOG(ERROR) << "Path " << extension_path_str
On 2011/08/16 10:39:33, Finnur wrote:
> nit: Path -> File

Done.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:90: << " for
key " << key_path << " can not be read.  "
On 2011/08/16 10:39:33, Finnur wrote:
> nit: One space before second closing double quote.

Done.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:101: << "
for key " << key_path << " .";
On 2011/08/16 10:39:33, Finnur wrote:
> nit: Why space before period? (same below, two occurrences).

I was following existing practice, but I agree it is strange.  Removed.

[Finnur, 2011-08-17 10:46:25]

Stil LGTM (with nits)

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:101: << "
for key " << key_path << " .";
What do you mean existing practice? I've never noticed this being used...

On 2011/08/16 18:25:52, Sam Kerner (Chrome) wrote:
> On 2011/08/16 10:39:33, Finnur wrote:
> > nit: Why space before period? (same below, two occurrences).
> 
> I was following existing practice, but I agree it is strange.  Removed.

http://codereview.chromium.org/7650014/diff/7001/chrome/browser/extensions/ex...
chrome/browser/extensions/external_registry_extension_loader_win.cc:75:
LOG(ERROR) << "Path " << extension_path_str
I wouldn't change this one, or at least have it say 'File path', since it is the
path that needs to be absolute, not the file.

[Sam Kerner (Chrome), 2011-08-17 14:42:44]

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:101: << "
for key " << key_path << " .";
On 2011/08/17 10:46:25, Finnur wrote:
> What do you mean existing practice? I've never noticed this being used...
> 
> On 2011/08/16 18:25:52, Sam Kerner (Chrome) wrote:
> > On 2011/08/16 10:39:33, Finnur wrote:
> > > nit: Why space before period? (same below, two occurrences).
> > 
> > I was following existing practice, but I agree it is strange.  Removed.
> 

There were a few places in this function that did it.  I changed them all from
this: 
  <something> << " .";
to this:
  <something> << " .";

[Sam Kerner (Chrome), 2011-08-17 14:43:09]

http://codereview.chromium.org/7650014/diff/7001/chrome/browser/extensions/ex...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/7001/chrome/browser/extensions/ex...
chrome/browser/extensions/external_registry_extension_loader_win.cc:75:
LOG(ERROR) << "File " << extension_path_str
On 2011/08/17 10:46:25, Finnur wrote:
> I wouldn't change this one, or at least have it say 'File path', since it is
the
> path that needs to be absolute, not the file.

Done.

[Finnur, 2011-08-17 14:51:17]

LGTM.

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
File chrome/browser/extensions/external_registry_extension_loader_win.cc
(right):

http://codereview.chromium.org/7650014/diff/3/chrome/browser/extensions/exter...
chrome/browser/extensions/external_registry_extension_loader_win.cc:101: << "
for key " << key_path << " .";
to this:
  <something> << ".";

... you mean? :)