Add option to install an ephemeral app to ChromeOS shelf context menu

chrome/browser/extensions/webstore_install_prompt.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALL_PROMPT_H_
+#define CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALL_PROMPT_H_
+
+#include "base/basictypes.h"
+#include "content/public/browser/page_navigator.h"
+#include "url/gurl.h"
+#include "webstore_standalone_installer.h"

[tapted, 2014/05/28 06:28:46]

this should have the full path

+
+namespace content {
+class WebContents;
+}
+
+namespace extensions {

[tapted, 2014/05/28 06:28:46]

Things under chrome/browser don't typically go in a namespace
(WebstoreStandaloneInstaller is, but I think that's a bug)

+
+// Initiates the install of an extension from the webstore. Downloads and parses
+// metadata from the webstore, shows an install UI and starts the download once
+// the user confirms. No post-install UI is shown.
+//
+// Clients will be notified of success or failure via the |callback| argument
+// passed into the constructor.
+//
+// Clients of this class must be trusted, as verification of the requestor is
+// skipped. This class stubs out many WebstoreStandaloneInstaller abstract
+// methods and can be used as a base class.
+class WebstoreInstallPrompt : public WebstoreStandaloneInstaller,
+                              public content::PageNavigator {
+ public:
+  typedef WebstoreStandaloneInstaller::Callback Callback;

[tapted, 2014/05/28 06:28:46]

I.. think this line is redundant -- inheriting from WebstoreStandaloneInstaller
brings its nested types in. (Although `Callback` is possibly a bad name anyway,
since it looks so much like base::Callback - I'd maybe favour something like
`InstalledCallback`, but that's orthogonal).

If removing this line works, I'd go with that -- duplicating the dec here means
information about what parameters the callback takes is harder to find.

+
+  // Use this constructor when there is no parent window. The install dialog
+  // will be centered on the screen.
+  WebstoreInstallPrompt(const std::string& webstore_item_id,
+                        Profile* profile,
+                        const Callback& callback);
+
+  // If this constructor is used, the parent of the install dialog will be
+  // |parent_window|.
+  WebstoreInstallPrompt(const std::string& webstore_item_id,
+                       Profile* profile,

[tapted, 2014/05/28 06:28:46]

nit: indenting is off

+                       gfx::NativeWindow parent_window,
+                       const Callback& callback);
+
+ protected:
+  friend class base::RefCountedThreadSafe<WebstoreInstallPrompt>;
+  virtual ~WebstoreInstallPrompt();
+
+  // extensions::WebstoreStartupInstaller overrides:

[tapted, 2014/05/28 06:28:46]

Startup -> Standalone?

+  virtual bool CheckRequestorAlive() const OVERRIDE;
+  virtual const GURL& GetRequestorURL() const OVERRIDE;
+  virtual bool ShouldShowPostInstallUI() const OVERRIDE;
+  virtual bool ShouldShowAppInstalledBubble() const OVERRIDE;
+  virtual content::WebContents* GetWebContents() const OVERRIDE;
+  virtual scoped_ptr<ExtensionInstallPrompt::Prompt> CreateInstallPrompt()
+      const OVERRIDE;

[tapted, 2014/05/28 06:28:46]

nit: const/OVERRIDE shouldn't appear on a line by itself. `git cl format` should
fix this properly now. 

[tmdiep, 2014/05/28 08:39:27]

git cl format put the "const OVERRIDE" in a new line. Maybe it prefers the
function name to fit on the first line?

[tapted, 2014/05/28 12:57:32]

probably an accident/clang-format bug - clang-format was changed recently to
ensure 'OVERRIDE' didn't appear on a line by itself to match the "const
shouldn't appear on a line by itself" style rule. But I guess nobody specified
what to do if there are both (of if they did, it's OK)...  I'll defer to
clang-format :)

+  virtual scoped_ptr<ExtensionInstallPrompt> CreateInstallUI() OVERRIDE;
+  virtual bool CheckInlineInstallPermitted(
+      const base::DictionaryValue& webstore_data,
+      std::string* error) const OVERRIDE;
+  virtual bool CheckRequestorPermitted(
+      const base::DictionaryValue& webstore_data,
+      std::string* error) const OVERRIDE;
+
+  // content::PageNavigator overrides:
+  virtual content::WebContents* OpenURL(
+      const content::OpenURLParams& params) OVERRIDE;
+
+ private:
+  GURL dummy_requestor_url_;
+
+  // A non-visible WebContents used to download data from the webstore.
+  scoped_ptr<content::WebContents> dummy_web_contents_;

[tapted, 2014/05/28 06:28:46]

nit: #include scoped_ptr for this

+
+  gfx::NativeWindow parent_window_;

[tapted, 2014/05/28 06:28:46]

nit: #include native_widget_types.h 

+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(WebstoreInstallPrompt);

[tapted, 2014/05/28 06:28:46]

nit: I think this is only if you don't define *any* constructors, otherwise the
compiler hides the implicit default constructor for you. So
DISALLOW_COPY_AND_ASSIGN is probably better

+};
+
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_WEBSTORE_INSTALL_PROMPT_H_