Add a private API method to install a webstore app from Files.app

Add a private API method to install a webstore app from Files.app

This patch is a part of the Webstore-Files.app integration (crbug.com/240152).

BUG=272245
TEST=manually tested

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220581

[yoshiki, 2013-08-21 17:22:42]

@asagent: Could you take a look at the following files? You may know about
webstore installer.
- chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.{cc|h}

@satorux: Could you take a look as an owner?
- chrome/browser/chromeos/extensions/file_manager/*

[yoshiki, 2013-08-27 08:17:58]

@asagent: ping

[asargent_no_longer_on_chrome, 2013-08-27 18:22:33]

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
File chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:22: :
extensions::WebstoreStandaloneInstaller(
nit: I think the ":" is supposed to go at the end of the previous line, and then
this line should have 4-space indent (double check the style guide though)

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:38:
return dummy_url_;
Ok, I see now why you need dummy_url_. This is sort of a bummer - maybe we
should just change the signature of the method to return "GURL" instead of
"const GURL&", so you could just "return GURL()" here.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:69:
error->clear();
nit: I'd personally rather just leave |error| alone, but if you really want to
check that the caller isn't doing something silly you could do:

DCHECK(error != NULL);
DCHECK(error->empty());

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:76:
error->clear();
same here

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:81: //
Private implementation.
I'm not sure this comment really helps any with readability

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
File chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h
(right):

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:21:
class WebstoreAppInstaller
This class name is confusingly similar  to WebstoreInstaller - can we pick
something else? Maybe FileManagerInstaller?

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:30:
const Callback& callback);
nit: indentation is off on these 3 lines

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:37: //
Implementations WebstoreStandaloneInstaller Template Method's hooks.
nit: to be pedantic, it's not a template (in the C++ sense), it's an abstract
base class with pure virtual methods. We often just refer to this as an
interface ; eg 

"Implements the WebstoreStandaloneInstaller interface."


I think the "Template Method" comment in webstore_standalone_installer.h is
referring to the "Template Method Design Pattern", but I may go remove or
clarify that comment because I think it's confusing since it has nothing to do
with C++ templates.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:57:
GURL dummy_url_;
nit: this variable could use a comment since it isn't immediately obvious what
it's for

https://codereview.chromium.org/23332012/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/file_browser_private.json:1178: "name":
"installWebstoreApp",
Will this only be allowed to install apps, or will you allow extensions also? If
extensions are allowed too, then you should use a neutral term like "item" or
something, e.g. "installWebstoreItem"

[yoshiki, 2013-08-28 13:19:03]

@asagent, thanks for reviewing. PTAL.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
File chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:22: :
extensions::WebstoreStandaloneInstaller(
On 2013/08/27 18:22:33, Antony Sargent wrote:
> nit: I think the ":" is supposed to go at the end of the previous line, and
then
> this line should have 4-space indent (double check the style guide though)

No. According to the style guide, we hasve to put the colon on new line after
indent 4 spaces.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Constr...

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:38:
return dummy_url_;
On 2013/08/27 18:22:33, Antony Sargent wrote:
> Ok, I see now why you need dummy_url_. This is sort of a bummer - maybe we
> should just change the signature of the method to return "GURL" instead of
> "const GURL&", so you could just "return GURL()" here. 
> 

I agree, this method should return just GURL.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:69:
error->clear();
On 2013/08/27 18:22:33, Antony Sargent wrote:
> nit: I'd personally rather just leave |error| alone, but if you really want to
> check that the caller isn't doing something silly you could do:
> 
> DCHECK(error != NULL);
> DCHECK(error->empty());

Done.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:76:
error->clear();
On 2013/08/27 18:22:33, Antony Sargent wrote:
> same here

Done.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:81: //
Private implementation.
On 2013/08/27 18:22:33, Antony Sargent wrote:
> I'm not sure this comment really helps any with readability

Removed

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
File chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h
(right):

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:21:
class WebstoreAppInstaller
On 2013/08/27 18:22:33, Antony Sargent wrote:
> This class name is confusingly similar  to WebstoreInstaller - can we pick
> something else? Maybe FileManagerInstaller? 
> 

Done.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:30:
const Callback& callback);
On 2013/08/27 18:22:33, Antony Sargent wrote:
> nit: indentation is off on these 3 lines

Done.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:37: //
Implementations WebstoreStandaloneInstaller Template Method's hooks.
On 2013/08/27 18:22:33, Antony Sargent wrote:
> nit: to be pedantic, it's not a template (in the C++ sense), it's an abstract
> base class with pure virtual methods. We often just refer to this as an
> interface ; eg 
> 
> "Implements the WebstoreStandaloneInstaller interface."
> 
> 
> I think the "Template Method" comment in webstore_standalone_installer.h is
> referring to the "Template Method Design Pattern", but I may go remove or
> clarify that comment because I think it's confusing since it has nothing to do
> with C++ templates. 
> 

Thanks, I grep'd and have found "// SomeClassName implementation." seems to be
more common. I use this instead.

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.h:57:
GURL dummy_url_;
On 2013/08/27 18:22:33, Antony Sargent wrote:
> nit: this variable could use a comment since it isn't immediately obvious what
> it's for

Done.

https://codereview.chromium.org/23332012/diff/1/chrome/common/extensions/api/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/1/chrome/common/extensions/api/...
chrome/common/extensions/api/file_browser_private.json:1178: "name":
"installWebstoreApp",
On 2013/08/27 18:22:33, Antony Sargent wrote:
> Will this only be allowed to install apps, or will you allow extensions also?
If
> extensions are allowed too, then you should use a neutral term like "item" or
> something, e.g. "installWebstoreItem" 

Thanks for suggestion. Done.

[asargent_no_longer_on_chrome, 2013-08-29 22:26:03]

lgtm w/ 1 nit

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
File chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/1/chrome/browser/chromeos/exten...
chrome/browser/chromeos/extensions/file_manager/webstore_app_installer.cc:22: :
extensions::WebstoreStandaloneInstaller(
On 2013/08/28 13:19:04, yoshiki wrote:
> On 2013/08/27 18:22:33, Antony Sargent wrote:
> > nit: I think the ":" is supposed to go at the end of the previous line, and
> then
> > this line should have 4-space indent (double check the style guide though)
> 
> No. According to the style guide, we hasve to put the colon on new line after
> indent 4 spaces.
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Constr...

Ok, sorry for my mistake, and thanks for double-checking.

https://codereview.chromium.org/23332012/diff/24001/chrome/common/extensions/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/24001/chrome/common/extensions/...
chrome/common/extensions/api/file_browser_private.json:1203: }
nit: instead of having a callback where you pass a boolean for success failure
and a string for error details, our usual pattern is to use
chrome.runtime.lastError and chrome.runtime.lastError instead. You already get
these set for free by setting error_ to the error string and calling
SendResponse(false) in your InstallWebstoreItemFunction code. 

You can just have a no-parameters callback here, and in the description just say
something like "callers should check chrome.runtime.lastError to determine if
there was any failure, in which case chrome.runtime.lastError.message will
contain a reason for the failure"

[yoshiki, 2013-08-30 06:02:48]

@satorux, could you tale a look as an owner? Thankc.

https://codereview.chromium.org/23332012/diff/24001/chrome/common/extensions/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/24001/chrome/common/extensions/...
chrome/common/extensions/api/file_browser_private.json:1203: }
On 2013/08/29 22:26:04, Antony Sargent wrote:
> nit: instead of having a callback where you pass a boolean for success failure
> and a string for error details, our usual pattern is to use
> chrome.runtime.lastError and chrome.runtime.lastError instead. You already get
> these set for free by setting error_ to the error string and calling
> SendResponse(false) in your InstallWebstoreItemFunction code. 
> 
> You can just have a no-parameters callback here, and in the description just
say
> something like "callers should check chrome.runtime.lastError to determine if
> there was any failure, in which case chrome.runtime.lastError.message will
> contain a reason for the failure"

I didn't know that. Thanks. Done

[yoshiki, 2013-08-30 08:47:57]

@hashimoto, could you take an onwer look? Thanks.

[hashimoto, 2013-08-30 09:09:37]

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:11:
using content::WebContents;
nit: No need to have this 'using' statement?
Most of times you are referring WebContents with "content::".

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:15:
const char kWebContentsDestroyedError[] = "WebContents is destroyed.";
This should be in an unnamed namespace.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
nit: Current style does not have "(c)" in a file header.
http://www.chromium.org/developers/coding-style#TOC-File-headers

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:23:
public content::WebContentsObserver {
Our style guide does not allow inheriting multiple non-pure-interface classses.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Multiple_Inher...

You should do: splitting this class into two parts, modifying this class to own
sub-classes of WebstoreStandaloneInstaller and WebContentsObserver or something
like that.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:53: //
content::WebContentsObserver implementation.
Why this part is private while WebstoreStandaloneInstaller methods are
protected?
I think this should be protected too.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:58:
GURL dummy_url_;
How about using GURL::EmptyGURL instead of having this member?

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:209: if
(args_->GetSize() < 1) {
nit: No need to have '{'?

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:229:
AddRef();  // Balanced in Release() in OnInstallComplete.
How about binding |this| to the callback without base::Unretained instead of
calling AddRef manually?

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:234: const
std::string& error) {
nit: indent.

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
chrome/common/extensions/api/file_browser_private.json:1193: "properties": {
I couldn't find C++ code to set these params.
This part is now irrelevant?

[yoshiki, 2013-08-30 09:51:45]

@hashimoto: PTAL. Thanks.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:11:
using content::WebContents;
On 2013/08/30 09:09:37, hashimoto wrote:
> nit: No need to have this 'using' statement?
> Most of times you are referring WebContents with "content::".

Removed.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:15:
const char kWebContentsDestroyedError[] = "WebContents is destroyed.";
On 2013/08/30 09:09:37, hashimoto wrote:
> This should be in an unnamed namespace.

Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
On 2013/08/30 09:09:37, hashimoto wrote:
> nit: Current style does not have "(c)" in a file header.
> http://www.chromium.org/developers/coding-style#TOC-File-headers

Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:23:
public content::WebContentsObserver {
On 2013/08/30 09:09:37, hashimoto wrote:
> Our style guide does not allow inheriting multiple non-pure-interface
classses.
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Multiple_Inher...
> 
> You should do: splitting this class into two parts, modifying this class to
own
> sub-classes of WebstoreStandaloneInstaller and WebContentsObserver or
something
> like that.

I didn't know it. Thanks. Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:53: //
content::WebContentsObserver implementation.
On 2013/08/30 09:09:37, hashimoto wrote:
> Why this part is private while WebstoreStandaloneInstaller methods are
> protected?
> I think this should be protected too.

Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.h:58:
GURL dummy_url_;
On 2013/08/30 09:09:37, hashimoto wrote:
> How about using GURL::EmptyGURL instead of having this member?

Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
(right):

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:209: if
(args_->GetSize() < 1) {
On 2013/08/30 09:09:37, hashimoto wrote:
> nit: No need to have '{'?

Removed.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:229:
AddRef();  // Balanced in Release() in OnInstallComplete.
On 2013/08/30 09:09:37, hashimoto wrote:
> How about binding |this| to the callback without base::Unretained instead of
> calling AddRef manually?

Nice. Done.

https://codereview.chromium.org/23332012/diff/32001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc:234: const
std::string& error) {
On 2013/08/30 09:09:37, hashimoto wrote:
> nit: indent.

Done.

[hashimoto, 2013-08-30 09:59:20]

looking good

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
chrome/common/extensions/api/file_browser_private.json:1193: "properties": {
On 2013/08/30 09:09:37, hashimoto wrote:
> I couldn't find C++ code to set these params.
> This part is now irrelevant?

ping?

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
nit: Remove (c)

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:15: } 
// anonymous namespace
nit: Just "// namespace".

[yoshiki, 2013-08-30 10:04:25]

@hashimoto: PTAL

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
File chrome/common/extensions/api/file_browser_private.json (right):

https://codereview.chromium.org/23332012/diff/32001/chrome/common/extensions/...
chrome/common/extensions/api/file_browser_private.json:1193: "properties": {
On 2013/08/30 09:59:21, hashimoto wrote:
> On 2013/08/30 09:09:37, hashimoto wrote:
> > I couldn't find C++ code to set these params.
> > This part is now irrelevant?
> 
> ping?

Sorry. They are no longer used. Removed.

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
File chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc
(right):

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:1: //
Copyright (c) 2013 The Chromium Authors. All rights reserved.
On 2013/08/30 09:59:21, hashimoto wrote:
> nit: Remove (c)

Done.

https://codereview.chromium.org/23332012/diff/42001/chrome/browser/chromeos/e...
chrome/browser/chromeos/extensions/file_manager/file_manager_installer.cc:15: } 
// anonymous namespace
On 2013/08/30 09:59:21, hashimoto wrote:
> nit: Just "// namespace".

Done.

[hashimoto, 2013-08-30 10:09:44]

lgtm, thanks

[yoshiki, 2013-08-30 10:10:20]

thanks!

[commit-bot: I haz the power, 2013-08-30 10:11:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23332012/52001

[commit-bot: I haz the power, 2013-08-30 10:11:13]

Failed to apply patch for
chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
  Hunk #1 FAILED at 67.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc.rej

Patch:      
chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
Index:
chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
diff --git
a/chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
b/chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
index
6a39f9e95a3559012e3f0a98f03206b2941fba82..0eef620c833af6b78e87ff3a11353e106a162d76
100644
---
a/chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
+++
b/chrome/browser/chromeos/extensions/file_manager/file_browser_private_api.cc
@@ -67,8 +67,10 @@ FileBrowserPrivateAPI::FileBrowserPrivateAPI(Profile*
profile)
   registry->RegisterFunction<LogoutUserFunction>();
   registry->RegisterFunction<GetPreferencesFunction>();
   registry->RegisterFunction<SetPreferencesFunction>();
+  registry->RegisterFunction<InstallWebstoreItemFunction>();
   registry->RegisterFunction<ZipSelectionFunction>();
   registry->RegisterFunction<ZoomFunction>();
+
   event_router_->ObserveFileSystemEvents();
 }

[commit-bot: I haz the power, 2013-08-30 10:40:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/23332012/30001

[commit-bot: I haz the power, 2013-08-30 14:44:59]

Change committed as 220581