Chromium Code Reviews
Help | Chromium Project | Sign in
(100)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months ago by Marijn Kruisselbrink
Modified:
7 months, 2 weeks ago
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M chrome/browser/extensions/api/developer_private/developer_private_api.cc View 1 2 3 chunks +17 lines, -2 lines 0 comments ? errors Download
A chrome/browser/extensions/external_component_loader.h View 1 2 3 1 chunk +37 lines, -0 lines 0 comments 1 errors Download
A chrome/browser/extensions/external_component_loader.cc View 1 2 3 1 chunk +24 lines, -0 lines 0 comments 1 errors Download
M chrome/browser/extensions/external_provider_impl.cc View 2 chunks +10 lines, -0 lines 3 comments ? errors Download
M chrome/chrome_browser_extensions.gypi View 1 chunk +2 lines, -0 lines 0 comments ? errors Download
M chrome/common/extensions/extension.cc View 1 2 1 chunk +4 lines, -0 lines 0 comments ? errors Download
M chrome/common/extensions/extension_constants.h View 1 2 1 chunk +3 lines, -0 lines 0 comments ? errors Download
M chrome/common/extensions/extension_constants.cc View 1 2 1 chunk +1 line, -0 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 13
Marijn Kruisselbrink
This I think is the most straight forward way of having the wallet app installed ...
11 months ago #1
Antony Sargent
Mostly fine with a couple of nits. I'm slightly wary of using the term "component" ...
11 months ago #2
Marijn Kruisselbrink
On 2013/05/22 00:07:13, Antony Sargent wrote: > I'm slightly wary of using the term "component" ...
11 months ago #3
Antony Sargent
Ok, you've convinced me that "external component" is the least bad. =) lgtm
11 months ago #4
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/13001
11 months ago #5
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/21001
11 months ago #6
I haz the power (commit-bot)
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
11 months ago #7
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mek@chromium.org/15612002/21001
11 months ago #8
I haz the power (commit-bot)
Change committed as 201773
11 months ago #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 ...
8 months, 1 week ago #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 ...
8 months, 1 week ago #11
Mattias Nissler
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 months, 2 weeks ago #12
Antony Sargent
7 months, 2 weeks ago #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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6