Show post-install UI when ephemeral apps are promoted to installed apps

Show post-install UI when ephemeral apps are promoted to installed apps

Installing an ephemeral app bypasses download of the CRX from the
webstore and thus also bypasses post-install UI. This patch ensures
that post-install UI is shown, such as showing the app launcher with
the app highlighted.

BUG=394997
TEST=See bug

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

[tmdiep, 2014-07-18 06:37:31]

This patch just ensures we show post-install UI when ephemeral apps are
promoted, because the post-install UI is usually shown by the CrxInstaller.
asargent: PTAL, thanks.

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/st...
File chrome/browser/extensions/startup_helper.cc (right):

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/st...
chrome/browser/extensions/startup_helper.cc:41: // On Windows, the jumplist
action for installing an ephemeral app has to use
Note: Windows jumplists only allow you to execute shell paths. That's why we had
to use --install-from-webstore to implement the "Install App" action.
http://msdn.microsoft.com/en-us/library/windows/desktop/dd378402(v=vs.85).aspx

[asargent_no_longer_on_chrome, 2014-07-21 23:23:16]

lgtm w/ one question

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
File chrome/browser/extensions/webstore_standalone_installer.cc (right):

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
chrome/browser/extensions/webstore_standalone_installer.cc:366:
extension_service->PromoteEphemeralApp(installed_extension, false);
I notice that both of the places we call PromoteEphemeralApp (here, and in
webstore_private_api.cc) you needed to add the call to
ShowPostInstallUIForApproval. Would it make sense to push the call to
ShowPostInstallUIForApproval inside PromoteEphemeralApp?

[tmdiep, 2014-07-21 23:32:25]

Thanks for the review!

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
File chrome/browser/extensions/webstore_standalone_installer.cc (right):

https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
chrome/browser/extensions/webstore_standalone_installer.cc:366:
extension_service->PromoteEphemeralApp(installed_extension, false);
On 2014/07/21 23:23:16, Antony Sargent wrote:
> I notice that both of the places we call PromoteEphemeralApp (here, and in
> webstore_private_api.cc) you needed to add the call to
> ShowPostInstallUIForApproval. Would it make sense to push the call to
> ShowPostInstallUIForApproval inside PromoteEphemeralApp?

Hmm, I'm inclined to keep the post-install UI out of ExtensionService. The
WebstorePrivateAPI and WebstoreStandaloneInstaller both are user-initiated, so
it makes sense to show the post-install UI. But we also call
PromoteEphemeralApp() from ExtensionSyncService - in this case the process
should be silent.

We'll have to duplicate the call to ShowPostInstallUIForApproval(), but I
created the util function so that most of the code doesn't have to be
duplicated.

[asargent_no_longer_on_chrome, 2014-07-21 23:35:18]

On 2014/07/21 23:32:25, tmdiep wrote:
> Thanks for the review!
> 
>
https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
> File chrome/browser/extensions/webstore_standalone_installer.cc (right):
> 
>
https://codereview.chromium.org/407483002/diff/1/chrome/browser/extensions/we...
> chrome/browser/extensions/webstore_standalone_installer.cc:366:
> extension_service->PromoteEphemeralApp(installed_extension, false);
> On 2014/07/21 23:23:16, Antony Sargent wrote:
> > I notice that both of the places we call PromoteEphemeralApp (here, and in
> > webstore_private_api.cc) you needed to add the call to
> > ShowPostInstallUIForApproval. Would it make sense to push the call to
> > ShowPostInstallUIForApproval inside PromoteEphemeralApp?
> 
> Hmm, I'm inclined to keep the post-install UI out of ExtensionService. The
> WebstorePrivateAPI and WebstoreStandaloneInstaller both are user-initiated, so
> it makes sense to show the post-install UI. But we also call
> PromoteEphemeralApp() from ExtensionSyncService - in this case the process
> should be silent.
> 
> We'll have to duplicate the call to ShowPostInstallUIForApproval(), but I
> created the util function so that most of the code doesn't have to be
> duplicated.

Ok, that makes sense

[tmdiep, 2014-07-21 23:36:41]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-07-21 23:37:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tmdiep@chromium.org/407483002/1

[tmdiep, 2014-07-21 23:44:31]

The CQ bit was unchecked by tmdiep@chromium.org

[tmdiep, 2014-07-21 23:55:25]

+benwells for stamp on c/b/ui/app_list and c/b/ui/extensions. They are mainly
interface changes.

[benwells, 2014-07-22 04:06:02]

lgtm

[tmdiep, 2014-07-22 06:02:27]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-07-22 06:03:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tmdiep@chromium.org/407483002/1

[commit-bot: I haz the power, 2014-07-22 06:23:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[commit-bot: I haz the power, 2014-07-22 06:25:58]

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

[commit-bot: I haz the power, 2014-07-22 06:25:59]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[tmdiep, 2014-07-22 12:04:53]

+miguelg for minor change in c/b/ui/android. Thanks in advance.

[Miguel Garcia, 2014-07-24 11:28:20]

LGTM  c/b/ui/android

[tmdiep, 2014-07-25 03:33:11]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-07-25 03:36:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tmdiep@chromium.org/407483002/20001

[commit-bot: I haz the power, 2014-07-25 06:41:51]

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

[commit-bot: I haz the power, 2014-07-25 10:24:42]

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

[commit-bot: I haz the power, 2014-07-25 10:24:43]

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

[tmdiep, 2014-07-28 00:51:50]

The CQ bit was checked by tmdiep@chromium.org

[commit-bot: I haz the power, 2014-07-28 00:52:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tmdiep@chromium.org/407483002/40001

[commit-bot: I haz the power, 2014-07-28 03:32:24]

Change committed as 285859