Streamlined sideload UI for extensions from the webstore.

Streamlined sideload UI for extensions from the webstore.

http://i.imgur.com/OOjPmDJ.png

Sideloaded extensions that have a webstore update URL have a global error bubble replacing the normal extension install dialog.

Still TODO:
- Making this not appear for new profiles.
- Unit tests.

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

[Yoyo Zhou, 2013-04-19 01:19:33]

Matt,

 I wanted to get your thoughts on this UI and (the possible abuse of install
prompt delegates) before I continue further. The goal is for "Enable extension"
and "Remove from Chrome" to do those things without popping up another modal
dialog.

[Matt Perry, 2013-04-19 02:07:55]

I'm lost as to which classes are which. Correct me if I'm wrong:
1. ExtensionInstallPrompt is the class that shows the modal dialog when
extensions are installed.
2. ExternalInstallGlobalError shows the little badge in the Chrome vent menu.
And also the popup bubble?
3. ExternalInstallDialogDelegate handles the result from (1).

If that's true, then I don't see why you need to involve the
ExtensionInstallPrompt. You're not actually displaying a dialog. What does it
give you?

[Yoyo Zhou, 2013-04-19 02:59:31]

On 2013/04/19 02:07:55, Matt Perry wrote:
> I'm lost as to which classes are which. Correct me if I'm wrong:

Yes, that's right, more or less. But...

> 1. ExtensionInstallPrompt is the class that shows the modal dialog when
> extensions are installed.

You can pass ExtensionInstallPrompt a callback that tells it how to show the
dialog. In some sense, it's what prepares the UI to show to the user - strings,
icons, etc.

> 2. ExternalInstallGlobalError shows the little badge in the Chrome vent menu.
> And also the popup bubble?

Yes, global errors can be both a hot dog menu item and a bubble.

> 3. ExternalInstallDialogDelegate handles the result from (1).
> 
> If that's true, then I don't see why you need to involve the
> ExtensionInstallPrompt. You're not actually displaying a dialog. What does it
> give you?

True, I don't actually need the prompt, but having one means I don't have to
duplicate all the code to set the title, accept button, cancel button, etc.

[Matt Perry, 2013-04-19 18:25:06]

OK, I see. It definitely feels a bit dirty to:
(a) re-use the ExtensionInstallPrompt for a bubble-view. But I guess nothing
says it has to be a modal dialog.
(b) bifurcate the ExternalInstallGlobalError into either an inline install or a
warning message. As different UIs it would probably be cleaner to have different
classes represent each case. But that would end up duplicating a lot of
boilerplate.

So, I'm torn. It's not a huge deal, so LGTM.

[Yoyo Zhou, 2013-04-19 18:28:48]

On 2013/04/19 18:25:06, Matt Perry wrote:
> OK, I see. It definitely feels a bit dirty to:
> (a) re-use the ExtensionInstallPrompt for a bubble-view. But I guess nothing
> says it has to be a modal dialog.
> (b) bifurcate the ExternalInstallGlobalError into either an inline install or
a
> warning message. As different UIs it would probably be cleaner to have
different
> classes represent each case. But that would end up duplicating a lot of
> boilerplate.
> 
> So, I'm torn. It's not a huge deal, so LGTM.

(b) wouldn't be too bad to fix, I think, if you had the bubble global error
class inherit from the non-bubble global error.

[Yoyo Zhou, 2013-04-19 21:56:40]

Some boilerplate was duplicated - mostly restoring the original boilerplate,
anyway - but they're now 2 classes.

I'd like to try to land this in its current state to make M28.

[Matt Perry, 2013-04-19 21:59:40]

Thanks, it does look a bit cleaner now. LGTM

[commit-bot: I haz the power, 2013-04-19 22:18:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/14328029/7001

[commit-bot: I haz the power, 2013-04-20 01:50:50]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-04-20 08:38:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/14328029/7001

[commit-bot: I haz the power, 2013-04-20 12:36:28]

Change committed as 195391