Remove deprecated ExtensionService::GetInstalledExtension() usage

[Extensions] Add GetInstalledExtension() method to ExtensionRegistry

This CL adds GetInstalledExtension() method to ExtensionRegistry and
uses it instead of deprecated ExtensionService::GetInstalledExtension()
in chrome/browser/ui/app_list/.

Part of removing the deprecated GetInstalledExtension() call
from the ExtensionService.

BUG=489687
Committed: https://crrev.com/db93178bcaaf7e99ebb18bd51fa99b2feaf47e1f
Cr-Commit-Position: refs/heads/master@{#333036}

[babu, 2015-05-19 18:04:54]

sudarsana.nagineni@intel.com changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2015-05-19 18:19:00]

Please pick someone from chrome/browser/ui/app_list/OWNERS. Thanks.

[babu, 2015-05-19 19:01:13]

sudarsana.nagineni@intel.com changed reviewers:
+ stevenjb@chromium.org

[Matt Giuca, 2015-05-20 00:01:02]

mgiuca@chromium.org changed reviewers:
+ kalman@chromium.org, mgiuca@chromium.org

[Matt Giuca, 2015-05-20 00:01:02]

Hmm. I don't have any context on this (I can see that in extension_service.h
there is a big comment that says DEPRECATED), but from my point of view, this
seems backwards. We have a nice function that does a non-trivial operation
(small abstraction), and we're replacing it with a dozen copies of that
operation. To make things worse, we already have an ExtensionService* in most of
the call sites, and we're replacing that new redundant code to get an
ExtensionRegistry*.

+kalman to comment on why this method is deprecated
(https://codereview.chromium.org/426553010) and whether this change makes sense.

[stevenjb, 2015-05-26 21:26:25]

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/app_list_syncable_service.cc (right):

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/app_list_syncable_service.cc:130: return;
We should either move this check to service->UninstallExtension(), or add a
service->IsExtensionInstalled() rather than adding this extra dependency here.

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/extension_uninstaller.cc (right):

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/extension_uninstaller.cc:49:
extensions::ExtensionSystem::Get(profile_)->extension_service();
Same comment; we shouldn't have to check ExtensionRegistry before calling into
ExtensionService.

[Matt Giuca, 2015-05-27 00:21:47]

Ping +kalman.

[Matt Giuca, 2015-05-27 00:22:02]

(see my first comment)

[not at google - send to devlin, 2015-05-27 00:27:45]

It's deprecated because it encourages ExtensionService as a dependency rather
than ExtensionRegistry. However, a GetInstalledExtension method on
ExtensionRegistry would make sense.

[Matt Giuca, 2015-05-27 00:38:17]

OK, I'd be happy with that.

(I just don't want to introduce lots of small-scale code duplication by removing
this abstraction entirely.)

[not at google - send to devlin, 2015-05-27 15:42:19]

Cool. I've wanted a GetInstalledExtension function on previous occasions. Let's
add that as part of this CL.

[babu, 2015-06-04 14:48:11]

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/app_list_syncable_service.cc (right):

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/app_list_syncable_service.cc:130: return;
On 2015/05/26 21:26:25, stevenjb wrote:
> We should either move this check to service->UninstallExtension(), or add a
> service->IsExtensionInstalled() rather than adding this extra dependency here.

Thanks for reviewing! 

UninstallExtension() has the CHECK() failure already if the extension isn't
there.  Also, instead of adding a new method IsExtensionInstalled(), I found
that UninstallExtensionHelpr() does the same check before calling
UninstallExtension(). So, changed to UninstallExtensionHelpr() here. Hope it is
okay.

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
File chrome/browser/ui/app_list/extension_uninstaller.cc (right):

https://codereview.chromium.org/1130353010/diff/1/chrome/browser/ui/app_list/...
chrome/browser/ui/app_list/extension_uninstaller.cc:49:
extensions::ExtensionSystem::Get(profile_)->extension_service();
On 2015/05/26 21:26:25, stevenjb wrote:
> Same comment; we shouldn't have to check ExtensionRegistry before calling into
> ExtensionService.

Code has changed and this change is no longer needed.

[babu, 2015-06-04 14:50:56]

On 2015/05/27 15:42:19, kalman wrote:
> Cool. I've wanted a GetInstalledExtension function on previous occasions.
Let's
> add that as part of this CL.

Sorry for the late response. 

Now I added GetInstalledExtension() method to ExtensionRegistry and changed the
code to use it instead of deprecated ExtensionService::GetInstalledExtension().

[stevenjb, 2015-06-04 16:13:27]

ui/app_list lgtm

[not at google - send to devlin, 2015-06-04 17:41:33]

lgtm

https://codereview.chromium.org/1130353010/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_service.h (right):

https://codereview.chromium.org/1130353010/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_service.h:183: // true if the target
extension exists.
"iff" is correct, it means "if and only if".

[babu, 2015-06-05 11:16:26]

https://codereview.chromium.org/1130353010/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_service.h (right):

https://codereview.chromium.org/1130353010/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_service.h:183: // true if the target
extension exists.
On 2015/06/04 17:41:32, kalman wrote:
> "iff" is correct, it means "if and only if".

Acknowledged. Thanks!

[babu, 2015-06-05 11:16:31]

The CQ bit was checked by sudarsana.nagineni@intel.com

[babu, 2015-06-05 11:16:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1130353010/#ps40001
(title: "Changed if to iff")

[commit-bot: I haz the power, 2015-06-05 11:16:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1130353010/40001

[commit-bot: I haz the power, 2015-06-05 12:01:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-05 12:03:54]

Patchset 3 (id:??) landed as
https://crrev.com/db93178bcaaf7e99ebb18bd51fa99b2feaf47e1f
Cr-Commit-Position: refs/heads/master@{#333036}