Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(35)

Issue 10010038: Do not show the install prompt for themes. (Closed)

Created:
8 years, 8 months ago by jstritar
Modified:
8 years, 8 months ago
CC:
chromium-reviews, mihaip+watch_chromium.org
Visibility:
Public.

Description

Do not show the install prompt for themes. We do not show the install prompt for themes because you can revert from the post-install infobar. This patch updates web store installs to use ExtensionInstallUI. BUG=122351 TEST=inline, bundle and standard web store installs should still work Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133288

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 1

Patch Set 4 : #

Total comments: 13

Patch Set 5 : feedback #

Total comments: 4

Patch Set 6 : #

Patch Set 7 : #

Total comments: 2

Patch Set 8 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -145 lines) Patch
M chrome/browser/extensions/bundle_installer.h View 1 2 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/extensions/bundle_installer.cc View 1 2 1 chunk +2 lines, -6 lines 0 comments Download
M chrome/browser/extensions/extension_install_dialog.h View 1 2 1 chunk +0 lines, -21 lines 0 comments Download
M chrome/browser/extensions/extension_install_dialog.cc View 1 2 3 4 1 chunk +0 lines, -57 lines 0 comments Download
M chrome/browser/extensions/extension_install_ui.h View 1 2 3 4 5 6 7 chunks +49 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_install_ui.cc View 1 2 3 4 5 6 7 9 chunks +109 lines, -28 lines 0 comments Download
M chrome/browser/extensions/extension_webstore_private_api.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_webstore_private_api.cc View 1 2 3 4 2 chunks +8 lines, -13 lines 0 comments Download
M chrome/browser/extensions/extension_webstore_private_apitest.cc View 1 2 3 4 3 chunks +22 lines, -3 lines 0 comments Download
M chrome/browser/extensions/webstore_inline_installer.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/webstore_inline_installer.cc View 1 2 3 4 1 chunk +8 lines, -11 lines 0 comments Download
M chrome/common/chrome_notification_types.h View 1 2 3 4 1 chunk +0 lines, -4 lines 0 comments Download
A chrome/test/data/extensions/api_test/webstore_private/theme.html View 1 chunk +27 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
jstritar
8 years, 8 months ago (2012-04-06 17:56:38 UTC) #1
jstritar
actually not quite ready
8 years, 8 months ago (2012-04-06 18:58:42 UTC) #2
jstritar
Okay, ready to look at now.
8 years, 8 months ago (2012-04-06 20:08:11 UTC) #3
Aaron Boodman
Is the issue that we aren't using ExtensionInstallUI for installation from the store?
8 years, 8 months ago (2012-04-07 01:05:44 UTC) #4
jstritar
On 2012/04/07 01:05:44, Aaron Boodman wrote: > Is the issue that we aren't using ExtensionInstallUI ...
8 years, 8 months ago (2012-04-09 14:24:02 UTC) #5
jstritar
Can you take another look? http://codereview.chromium.org/10010038/diff/9001/chrome/common/chrome_notification_types.h File chrome/common/chrome_notification_types.h (left): http://codereview.chromium.org/10010038/diff/9001/chrome/common/chrome_notification_types.h#oldcode153 chrome/common/chrome_notification_types.h:153: NOTIFICATION_EXTENSION_WILL_SHOW_CONFIRM_DIALOG, This wasn't being ...
8 years, 8 months ago (2012-04-16 20:53:37 UTC) #6
jstritar
hey Aaron, can you take a look when you get a chance?
8 years, 8 months ago (2012-04-19 12:21:05 UTC) #7
jstritar
Hi Yoyo, can you take a look?
8 years, 8 months ago (2012-04-20 13:56:27 UTC) #8
Yoyo Zhou
http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.cc File chrome/browser/extensions/extension_install_ui.cc (left): http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.cc#oldcode354 chrome/browser/extensions/extension_install_ui.cc:354: service->Notify(chrome::NOTIFICATION_EXTENSION_WILL_SHOW_CONFIRM_DIALOG, What happened to this? Nobody cared to observe ...
8 years, 8 months ago (2012-04-20 15:38:14 UTC) #9
jstritar
http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.cc File chrome/browser/extensions/extension_install_ui.cc (left): http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.cc#oldcode354 chrome/browser/extensions/extension_install_ui.cc:354: service->Notify(chrome::NOTIFICATION_EXTENSION_WILL_SHOW_CONFIRM_DIALOG, On 2012/04/20 15:38:14, Yoyo Zhou wrote: > What ...
8 years, 8 months ago (2012-04-20 16:18:27 UTC) #10
Yoyo Zhou
LGTM. A little more cleanup is possible. http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.h File chrome/browser/extensions/extension_install_ui.h (right): http://codereview.chromium.org/10010038/diff/10014/chrome/browser/extensions/extension_install_ui.h#newcode282 chrome/browser/extensions/extension_install_ui.h:282: Prompt prompt_; ...
8 years, 8 months ago (2012-04-20 17:19:12 UTC) #11
jstritar
http://codereview.chromium.org/10010038/diff/19001/chrome/browser/extensions/extension_install_ui.cc File chrome/browser/extensions/extension_install_ui.cc (right): http://codereview.chromium.org/10010038/diff/19001/chrome/browser/extensions/extension_install_ui.cc#newcode251 chrome/browser/extensions/extension_install_ui.cc:251: prompt_type_(NUM_PROMPT_TYPES), On 2012/04/20 17:19:12, Yoyo Zhou wrote: > nit: ...
8 years, 8 months ago (2012-04-20 18:03:29 UTC) #12
Yoyo Zhou
LGTM http://codereview.chromium.org/10010038/diff/30002/chrome/browser/extensions/extension_install_ui.cc File chrome/browser/extensions/extension_install_ui.cc (right): http://codereview.chromium.org/10010038/diff/30002/chrome/browser/extensions/extension_install_ui.cc#newcode500 chrome/browser/extensions/extension_install_ui.cc:500: if (!icon_.empty() || prompt_type_ == BUNDLE_INSTALL_PROMPT) { ...and ...
8 years, 8 months ago (2012-04-20 18:06:11 UTC) #13
jstritar
https://chromiumcodereview.appspot.com/10010038/diff/30002/chrome/browser/extensions/extension_install_ui.cc File chrome/browser/extensions/extension_install_ui.cc (right): https://chromiumcodereview.appspot.com/10010038/diff/30002/chrome/browser/extensions/extension_install_ui.cc#newcode500 chrome/browser/extensions/extension_install_ui.cc:500: if (!icon_.empty() || prompt_type_ == BUNDLE_INSTALL_PROMPT) { oh yeah, ...
8 years, 8 months ago (2012-04-20 18:08:39 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jstritar@chromium.org/10010038/18004
8 years, 8 months ago (2012-04-20 18:09:13 UTC) #15
commit-bot: I haz the power
8 years, 8 months ago (2012-04-20 22:56:38 UTC) #16
Change committed as 133288

Powered by Google App Engine
This is Rietveld 408576698