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

Issue 15612002: Make sure the google wallet in-app payment support app is always installed. (Closed)

Created:
7 years, 7 months ago by Marijn Kruisselbrink
Modified:
7 years, 3 months ago
CC:
chromium-reviews, Aaron Boodman, chromium-apps-reviews_chromium.org
Visibility:
Public.

Description

Make sure the google wallet in-app payment support app is always installed. BUG=168290 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201773

Patch Set 1 #

Patch Set 2 : Change logic to decide which apps to show in chrome://extensions #

Total comments: 4

Patch Set 3 : fix comments #

Patch Set 4 : fix clang compile error #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -2 lines) Patch
M chrome/browser/extensions/api/developer_private/developer_private_api.cc View 1 2 3 chunks +17 lines, -2 lines 0 comments Download
A chrome/browser/extensions/external_component_loader.h View 1 2 3 1 chunk +37 lines, -0 lines 0 comments Download
A chrome/browser/extensions/external_component_loader.cc View 1 2 3 1 chunk +24 lines, -0 lines 0 comments Download
M chrome/browser/extensions/external_provider_impl.cc View 2 chunks +10 lines, -0 lines 3 comments Download
M chrome/chrome_browser_extensions.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.h View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Marijn Kruisselbrink
This I think is the most straight forward way of having the wallet app installed ...
7 years, 7 months ago (2013-05-21 21:46:25 UTC) #1
asargent_no_longer_on_chrome
Mostly fine with a couple of nits. I'm slightly wary of using the term "component" ...
7 years, 7 months ago (2013-05-22 00:07:13 UTC) #2
Marijn Kruisselbrink
On 2013/05/22 00:07:13, Antony Sargent wrote: > I'm slightly wary of using the term "component" ...
7 years, 7 months ago (2013-05-22 21:29:37 UTC) #3
asargent_no_longer_on_chrome
Ok, you've convinced me that "external component" is the least bad. =) lgtm
7 years, 7 months ago (2013-05-22 23:42:33 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/13001
7 years, 7 months ago (2013-05-22 23:44:40 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/21001
7 years, 7 months ago (2013-05-23 00:08:11 UTC) #6
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=152589
7 years, 7 months ago (2013-05-23 04:47:08 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/21001
7 years, 7 months ago (2013-05-23 04:51:49 UTC) #8
commit-bot: I haz the power
Change committed as 201773
7 years, 7 months ago (2013-05-23 12:57:52 UTC) #9
Dmitry Polukhin
https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc File chrome/browser/extensions/external_provider_impl.cc (right): https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc#newcode463 chrome/browser/extensions/external_provider_impl.cc:463: provider_list->push_back( FYI, I'm going to move this provider under ...
7 years, 4 months ago (2013-08-14 19:39:12 UTC) #10
Marijn Kruisselbrink
On 2013/08/14 19:39:12, Dmitry Polukhin wrote: > https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc > File chrome/browser/extensions/external_provider_impl.cc (right): > > https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc#newcode463 ...
7 years, 4 months ago (2013-08-14 19:44:58 UTC) #11
Mattias Nissler (ping if slow)
https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc File chrome/browser/extensions/external_provider_impl.cc (right): https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/extensions/external_provider_impl.cc#newcode469 chrome/browser/extensions/external_provider_impl.cc:469: Manifest::EXTERNAL_POLICY_DOWNLOAD, Is the EXTERNAL_POLICY_DOWNLOAD location intentional here? That may ...
7 years, 3 months ago (2013-09-05 08:19:06 UTC) #12
asargent_no_longer_on_chrome
7 years, 3 months ago (2013-09-05 21:19:15 UTC) #13
Message was sent while issue was closed.
https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/ext...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://chromiumcodereview.appspot.com/15612002/diff/21001/chrome/browser/ext...
chrome/browser/extensions/external_provider_impl.cc:469:
Manifest::EXTERNAL_POLICY_DOWNLOAD,
On 2013/09/05 08:19:06, Mattias Nissler wrote:
> Is the EXTERNAL_POLICY_DOWNLOAD location intentional here? That may trigger
> various policy-specific extension restrictions and messaging, which may not be
> appropriate for the payment app.

Whether it was intentional or not, it is incorrect as you point out (also it
messes up our UMA metrics). I've opened crbug.com/285983 to fix it.

Powered by Google App Engine
This is Rietveld 408576698