Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(57)

Issue 2892843002: Remove RuntimeAPIDelegate::GetPreviousVersion. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 1 week ago by lazyboy
Modified:
4 months ago
Reviewers:
Devlin
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove RuntimeAPIDelegate::GetPreviousVersion. We currently go to chrome/ to get the extension version, but we don't need to. ExtensionRegistry in extensions/ has the info already, move the function to ExtensionRegistry. Also, the name GetPreviousVersion is misleading. It only gives us the current version, so rename it to be simply GetVersion. BUG=723754 Test=None, internal only change. Review-Url: https://codereview.chromium.org/2892843002 Cr-Commit-Position: refs/heads/master@{#473332} Committed: https://chromium.googlesource.com/chromium/src/+/640dc5161179cd9058dd3c97788880a8ac4c7f4d

Patch Set 1 #

Total comments: 4

Patch Set 2 : . #

Total comments: 8

Patch Set 3 : address comments #

Total comments: 2

Patch Set 4 : address missing comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -33 lines) Patch
M chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.cc View 1 chunk +0 lines, -11 lines 0 comments Download
M extensions/browser/api/runtime/runtime_api.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M extensions/browser/api/runtime/runtime_api_delegate.h View 1 chunk +0 lines, -4 lines 0 comments Download
M extensions/browser/extension_registry.h View 1 2 3 2 chunks +11 lines, -0 lines 0 comments Download
M extensions/browser/extension_registry.cc View 1 2 3 1 chunk +9 lines, -0 lines 0 comments Download
M extensions/browser/test_runtime_api_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M extensions/browser/test_runtime_api_delegate.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M extensions/shell/browser/shell_runtime_api_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M extensions/shell/browser/shell_runtime_api_delegate.cc View 1 chunk +0 lines, -5 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 28 (18 generated)
lazyboy
Hi Devlin, The chrome/ dependency is getting in my way to https://codereview.chromium.org/2893693002/, so in order ...
4 months, 1 week ago (2017-05-18 23:27:27 UTC) #2
Devlin
Nice, thanks for the cleanup! One general thought: Do you think it's worth having the ...
4 months, 1 week ago (2017-05-19 01:07:10 UTC) #5
lazyboy
> One general thought: Do you think it's worth having the separate GetVersion() > function ...
4 months, 1 week ago (2017-05-19 01:38:48 UTC) #6
Devlin
On 2017/05/19 01:38:48, lazyboy wrote: > > One general thought: Do you think it's worth ...
4 months, 1 week ago (2017-05-19 02:04:01 UTC) #9
lazyboy
https://codereview.chromium.org/2892843002/diff/20001/extensions/browser/extension_registry.cc File extensions/browser/extension_registry.cc (right): https://codereview.chromium.org/2892843002/diff/20001/extensions/browser/extension_registry.cc#newcode8 extensions/browser/extension_registry.cc:8: #include "base/version.h" On 2017/05/19 02:04:00, Devlin (catching up) wrote: ...
4 months ago (2017-05-19 18:24:55 UTC) #13
Devlin
https://codereview.chromium.org/2892843002/diff/40001/extensions/browser/extension_registry.h File extensions/browser/extension_registry.h (right): https://codereview.chromium.org/2892843002/diff/40001/extensions/browser/extension_registry.h#newcode94 extensions/browser/extension_registry.h:94: base::Version GetStoredVersion(const Extension* extension) const; s/const Extension* extension/const ExtensionId& ...
4 months ago (2017-05-19 18:28:24 UTC) #15
lazyboy
https://codereview.chromium.org/2892843002/diff/40001/extensions/browser/extension_registry.h File extensions/browser/extension_registry.h (right): https://codereview.chromium.org/2892843002/diff/40001/extensions/browser/extension_registry.h#newcode94 extensions/browser/extension_registry.h:94: base::Version GetStoredVersion(const Extension* extension) const; On 2017/05/19 18:28:24, Devlin ...
4 months ago (2017-05-19 20:20:47 UTC) #18
Devlin
(s lgtm)
4 months ago (2017-05-19 20:22:44 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2892843002/60001
4 months ago (2017-05-19 21:30:07 UTC) #25
commit-bot: I haz the power
4 months ago (2017-05-19 21:37:28 UTC) #28
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/640dc5161179cd9058dd3c977888...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b