Allow drag-and-drop of zipped extensions on chrome://extensions

Allow drag-and-drop of zipped extensions on chrome://extensions

TEST=drag zip file extension to chrome://extensions
BUG=395311
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288545

[elijahtaylor1, 2014-07-18 23:47:43]

can I get early comments on whether this is an ok approach?

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
File chrome/browser/extensions/unpacked_installer.cc (right):

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:186: if
(!base::CreateTemporaryDirInDir(
I didn't know of a great way to notify the user/developer that the dir is being
created, but since it is created along-side the original zip file I thought it
would be ok.

[asargent_no_longer_on_chrome, 2014-07-22 22:51:02]

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
File chrome/browser/extensions/unpacked_installer.cc (right):

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:186: if
(!base::CreateTemporaryDirInDir(
On 2014/07/18 23:47:43, elijahtaylor1 wrote:
> I didn't know of a great way to notify the user/developer that the dir is
being
> created, but since it is created along-side the original zip file I thought it
> would be ok.

I wonder if it would be better to unzip into a new directory created within the
system temporary directory?

We do show the location the item was loaded from in chrome://extensions, with a
clickable link to open up the native file manager program pointing at that
directory.

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:202: if (!zip::Unzip(zip_path,
temp_path)) {
For regular extension installation, we do the unzipping operation in a sandboxed
helper process to help mitigate exploits of security bugs in the zip format
library code. It might be worth asking someone on the security team like meacer@
if they think we should do the same here.

[elijahtaylor1, 2014-07-23 00:19:07]

+meacer, can you weigh in on our comments re: sandboxed unpacker?  This is
really just a dev workflow, so I was hoping not to have to use it.

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
File chrome/browser/extensions/unpacked_installer.cc (right):

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:186: if
(!base::CreateTemporaryDirInDir(
On 2014/07/22 22:51:01, Antony Sargent wrote:
> On 2014/07/18 23:47:43, elijahtaylor1 wrote:
> > I didn't know of a great way to notify the user/developer that the dir is
> being
> > created, but since it is created along-side the original zip file I thought
it
> > would be ok.
> 
> I wonder if it would be better to unzip into a new directory created within
the
> system temporary directory?
> 
> We do show the location the item was loaded from in chrome://extensions, with
a
> clickable link to open up the native file manager program pointing at that
> directory. 
> 

It's probably better to do in a temp dir, but it could lead to the Preferences
pollution we talked about (I guess that could happen in either case).  I will do
this in a future patchset, just want to get security to weigh in first

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:202: if (!zip::Unzip(zip_path,
temp_path)) {
On 2014/07/22 22:51:01, Antony Sargent wrote:
> For regular extension installation, we do the unzipping operation in a
sandboxed
> helper process to help mitigate exploits of security bugs in the zip format
> library code. It might be worth asking someone on the security team like
meacer@
> if they think we should do the same here.

Yeah, I first tried to use the sandboxed unpacker, but it was really specific to
unpacking a CRX for side-loading, and I didn't want to modify that code to do
this.  I'll check with security

[meacer, 2014-07-23 00:32:15]

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
File chrome/browser/extensions/unpacked_installer.cc (right):

https://codereview.chromium.org/406713002/diff/1/chrome/browser/extensions/un...
chrome/browser/extensions/unpacked_installer.cc:202: if (!zip::Unzip(zip_path,
temp_path)) {
On 2014/07/23 00:19:07, elijahtaylor1 wrote:
> On 2014/07/22 22:51:01, Antony Sargent wrote:
> > For regular extension installation, we do the unzipping operation in a
> sandboxed
> > helper process to help mitigate exploits of security bugs in the zip format
> > library code. It might be worth asking someone on the security team like
> meacer@
> > if they think we should do the same here.
> 
> Yeah, I first tried to use the sandboxed unpacker, but it was really specific
to
> unpacking a CRX for side-loading, and I didn't want to modify that code to do
> this.  I'll check with security

I agree with Antony, it's best to do this in the sandboxed unpacker. I think you
should be able to refactor it for your purposes?

[elijahtaylor1, 2014-07-28 21:29:59]

PTAL.  If the general direction is ok I'll split this CL up so I can land the
binary test files first

[meacer, 2014-07-28 23:01:05]

Thanks. Would dropping a zip file to install still show the permission dialog?

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/sandboxed_unpacker.cc (right):

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/sandboxed_unpacker.cc:265: is_zipfile_ = true;
Cleaner?

is_zipfile_ = crx_path_.Extension() == ".zip"

[asargent_no_longer_on_chrome, 2014-07-29 21:38:10]

I'm worried that by introducing some special edge cases into UnpackedInstaller
and SandboxedUnpacker, we're more likely to run into bugs in the future as
people refactor code in there. Already there are a few histograms for instance
that don't necessarily reflect the reality, and having an early out in
ValidateSignature kind of scares me on principle. 

Instead I think it would be better to introduce a new ZipfileInstaller class in
new .h/.cc files that inherits from public content::UtilityProcessHostClient,
and have it just do something pretty similar to 
SandboxedUnpacker::StartProcessOnIOThread, and then in its OnMessageReceived
handler, for the ChromeUtilityHostMsg_UnpackExtension_Succeeded message, have it
then create a UnpackedInstaller object and let it handle things from there on.

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/install_extension_handler.cc (right):

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/install_extension_handler.cc:106:
file_display_name_, base::ASCIIToUTF16(".zip"), kCaseSensitive)) {
should this be case insensitive?

[elijahtaylor1, 2014-07-31 17:26:28]

PTAL.  If the general idea is good, I will add tests.  I may need to re-org
zipfile_installer to make a separate client that handles the success and failure
cases to test, but otherwise I think this is largely there

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/install_extension_handler.cc (right):

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/install_extension_handler.cc:106:
file_display_name_, base::ASCIIToUTF16(".zip"), kCaseSensitive)) {
On 2014/07/29 21:38:10, Antony Sargent wrote:
> should this be case insensitive?

This is unfortunately named, but is set false above, so this is case insensitive

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/extension_loader_handler.cc (right):

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/extension_loader_handler.cc:301:
failure->Set("reason", new base::StringValue(base::UTF8ToUTF16(error)));
this is a bug fix for the unpacked extension loader error message box, unrelated
to this change

[elijahtaylor1, 2014-07-31 17:29:48]

On 2014/07/28 23:01:05, Mustafa Emre Acer wrote:
> Thanks. Would dropping a zip file to install still show the permission dialog?

This will behave like unpacked installations (since at the backend it uses the
unpacked installer).  The permission dialog is limited, IIUC it's mostly related
to file permissions.  But the extension is unpacked to a temp dir that can/will
be non-permanent, so I wonder if it's ok to use the same unpacked install
semantics here.

[elijahtaylor1, 2014-07-31 17:29:56]

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/sandboxed_unpacker.cc (right):

https://codereview.chromium.org/406713002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/sandboxed_unpacker.cc:265: is_zipfile_ = true;
On 2014/07/28 23:01:05, Mustafa Emre Acer wrote:
> Cleaner?
> 
> is_zipfile_ = crx_path_.Extension() == ".zip"

removed code

[asargent_no_longer_on_chrome, 2014-07-31 19:59:23]

lgtm

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/extension...
chrome/browser/extensions/zipfile_installer.cc:49:
host->SetExposedDir(temp_dir.DirName());
Instead of giving the host access to the entire temp directory, it might make
sense to instead create a new temporary directory (eg using
base::CreateNewTempDirectory) to limit its access further.

Also, this reminds me that we might want to have a strategy in place for garbage
collecting these directories of unzipped content at the time that the unpacked
extension gets uninstalled. I think it's ok to proceed for now without having
that, but can you file a bug so we can remember to do it and link to it here?

[elijahtaylor1, 2014-08-06 00:31:54]

PTAL

I have a testing question if you could please advise.

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/extension...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/60001/chrome/browser/extension...
chrome/browser/extensions/zipfile_installer.cc:49:
host->SetExposedDir(temp_dir.DirName());
On 2014/07/31 19:59:23, Antony Sargent wrote:
> Instead of giving the host access to the entire temp directory, it might make
> sense to instead create a new temporary directory (eg using
> base::CreateNewTempDirectory) to limit its access further.
> 

Done

> Also, this reminds me that we might want to have a strategy in place for
garbage
> collecting these directories of unzipped content at the time that the unpacked
> extension gets uninstalled. I think it's ok to proceed for now without having
> that, but can you file a bug so we can remember to do it and link to it here?

issue filed: crbug.com/400934

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer_unittest.cc (right):

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer_unittest.cc:117: //
TODO(elijahtaylor): Test that the installed extension has this ID. This
I tried to register an extension registry observer but it didn't get hit.  Is
there an easy way to wait for an install of an extension from the unpacked
installer so I can test this?  I would really like to do as part of this CL

[elijahtaylor1, 2014-08-06 00:32:39]

forgot to mention, here's the test data files I'd like to land ahead of time so
this CL can pass trybots: https://codereview.chromium.org/444743005/

[asargent_no_longer_on_chrome, 2014-08-06 20:39:01]

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:85: //
UnpackedInstaller::Create(extension_service_.get())->Load(unzipped_path);
Maybe the fact that this line is commented out is why the registry isn't
notifying you? =)

[elijahtaylor1, 2014-08-06 20:42:51]

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:85: //
UnpackedInstaller::Create(extension_service_.get())->Load(unzipped_path);
On 2014/08/06 20:39:01, Antony Sargent wrote:
> Maybe the fact that this line is commented out is why the registry isn't
> notifying you? =)

Oh you and your astute reading of my crap code... let me re-roll that registry
code and try again, I had this commented out for some other test problem I had
:/

[elijahtaylor1, 2014-08-07 17:02:17]

asargent: PTAL, test was re-written a bit

miket: OWNERS for:
  chrome/browser/extensions/zipfile_installer.cc
  chrome/browser/extensions/zipfile_installer.h
  chrome/browser/extensions/zipfile_installer_unittest.cc
  chrome/browser/resources/extensions/extensions.js
  chrome/browser/ui/webui/extensions/extension_loader_handler.cc
  chrome/browser/ui/webui/extensions/install_extension_handler.cc
  chrome/utility/extensions/extensions_handler.cc
  chrome/utility/extensions/extensions_handler.h

jschuh: OWNERS for IPC changes

[asargent_no_longer_on_chrome, 2014-08-07 17:28:25]

new version lgtm w/ a few suggestions

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:83:
UnpackedInstaller::Create(extension_service_.get())->Load(unzipped_path);
nit: since extension_service_ is a weak ptr, you should check it and bail out
early here if it is null

(you might consider renaming it "extension_service_weak_" to serve as a reminder
for any future refactoring, etc.)

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:90: zip_path_, error,
extension_service_->profile(), be_noisy_on_failure_);
need same weak ptr check here too

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer_unittest.cc (right):

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer_unittest.cc:50: };
It might be helpful to put this class into an anonymous namespace instead of
leaving it in the extensions namespace, so that this won't collide with any
other identically named class in other extension unit tests.

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer_unittest.cc:82: void
SetupInstaller(const std::string& zip_name) {
You might consider renaming this to "RunInstaller" or something like that, since
it seems to do more than setup.

[miket_OOO, 2014-08-07 18:34:50]

OWNER LGTM

[jschuh, 2014-08-07 20:57:59]

What's the flow on these new messages? Is the unzip request browser -> utility,
and the ack is utility -> extension renderer?

[elijahtaylor1, 2014-08-07 21:50:32]

On 2014/08/07 20:57:59, Justin Schuh wrote:
> What's the flow on these new messages? Is the unzip request browser ->
utility,
> and the ack is utility -> extension renderer?

I cribbed the existing sandboxed unpacker pattern.  I assumed it was unzip
request browser -> utility, then the ack utility -> browser.  I don't understand
why it would go to a renderer.  asargent@ / meacer@, can you chime in if I'm
misunderstanding how the sandboxed unpacker works?

[jschuh, 2014-08-07 21:58:40]

On 2014/08/07 21:50:32, elijahtaylor1 wrote:
> On 2014/08/07 20:57:59, Justin Schuh wrote:
> > What's the flow on these new messages? Is the unzip request browser ->
> utility,
> > and the ack is utility -> extension renderer?
> 
> I cribbed the existing sandboxed unpacker pattern.  I assumed it was unzip
> request browser -> utility, then the ack utility -> browser.  I don't
understand
> why it would go to a renderer.  asargent@ / meacer@, can you chime in if I'm
> misunderstanding how the sandboxed unpacker works?

Taking another pass, it looks like I misread up the flow. Looks fine. IPC
security lgtm.

[elijahtaylor1, 2014-08-07 22:41:26]

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer.cc (right):

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:83:
UnpackedInstaller::Create(extension_service_.get())->Load(unzipped_path);
On 2014/08/07 17:28:25, Antony Sargent wrote:
> nit: since extension_service_ is a weak ptr, you should check it and bail out
> early here if it is null
> 
> (you might consider renaming it "extension_service_weak_" to serve as a
reminder
> for any future refactoring, etc.)
> 

Done.

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer.cc:90: zip_path_, error,
extension_service_->profile(), be_noisy_on_failure_);
On 2014/08/07 17:28:24, Antony Sargent wrote:
> need same weak ptr check here too

Weird, I had it for this function actually.

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
File chrome/browser/extensions/zipfile_installer_unittest.cc (right):

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer_unittest.cc:50: };
On 2014/08/07 17:28:25, Antony Sargent wrote:
> It might be helpful to put this class into an anonymous namespace instead of
> leaving it in the extensions namespace, so that this won't collide with any
> other identically named class in other extension unit tests. 

Done.

https://codereview.chromium.org/406713002/diff/120001/chrome/browser/extensio...
chrome/browser/extensions/zipfile_installer_unittest.cc:82: void
SetupInstaller(const std::string& zip_name) {
On 2014/08/07 17:28:25, Antony Sargent wrote:
> You might consider renaming this to "RunInstaller" or something like that,
since
> it seems to do more than setup. 

Done.

[elijahtaylor1, 2014-08-07 22:51:35]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-08-07 23:01:53]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/406713002/16...

[commit-bot: I haz the power, 2014-08-08 01:09:43]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/44069)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-08 01:17:29]

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

[commit-bot: I haz the power, 2014-08-08 01:17:30]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/44122)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[elijahtaylor1, 2014-08-08 17:17:41]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-08-08 17:20:01]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/406713002/18...

[elijahtaylor1, 2014-08-08 20:27:29]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-08-08 20:28:32]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/406713002/20...

[commit-bot: I haz the power, 2014-08-08 22:55:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-08 23:00:13]

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

[commit-bot: I haz the power, 2014-08-08 23:00:14]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tri...)

[elijahtaylor1, 2014-08-09 00:02:05]

The CQ bit was checked by elijahtaylor@chromium.org

[commit-bot: I haz the power, 2014-08-09 00:04:05]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/elijahtaylor@chromium.org/406713002/22...

[commit-bot: I haz the power, 2014-08-09 08:51:45]

Change committed as 288545

[Devlin, 2014-08-11 15:58:01]

https://codereview.chromium.org/406713002/diff/220001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/extensions/extension_loader_handler.cc (right):

https://codereview.chromium.org/406713002/diff/220001/chrome/browser/ui/webui...
chrome/browser/ui/webui/extensions/extension_loader_handler.cc:307:
failure->Set("reason", new base::StringValue(base::UTF8ToUTF16(error)));
This looks like a merge conflict resolution gone wrong, and I think this caused
crbug.com/402342.  Lemme know if you'll have time to push through a quick patch
(if not, I can).  Cheers! :)