Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(553)

Issue 11066125: Check that the user is signed in and the app has oauth2 section before recording an aouth2 grant. (Closed)

Created:
8 years, 2 months ago by Munjal (Google)
Modified:
8 years, 2 months ago
CC:
chromium-reviews, Aaron Boodman, mihaip-chromium-reviews_chromium.org
Visibility:
Public.

Description

Check that the user is signed in and the app has oauth2 section before recording an aouth2 grant. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161643

Patch Set 1 #

Patch Set 2 : #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -7 lines) Patch
M chrome/browser/extensions/extension_service.cc View 1 1 chunk +2 lines, -5 lines 0 comments Download
M chrome/browser/extensions/permissions_updater.cc View 1 chunk +13 lines, -2 lines 5 comments Download

Messages

Total messages: 8 (0 generated)
Munjal (Google)
8 years, 2 months ago (2012-10-11 19:10:24 UTC) #1
Mihai Parparita -not on Chrome
LGTM Can you also then put back the boolean that I changed in r160978 (and ...
8 years, 2 months ago (2012-10-11 21:27:47 UTC) #2
Munjal (Google)
Added back boolean and removed TODO.
8 years, 2 months ago (2012-10-11 21:39:51 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/[email protected]/11066125/4001
8 years, 2 months ago (2012-10-12 17:31:51 UTC) #4
commit-bot: I haz the power
Change committed as 161643
8 years, 2 months ago (2012-10-12 19:42:40 UTC) #5
Evan Stade
lgtm http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/permissions_updater.cc File chrome/browser/extensions/permissions_updater.cc (right): http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/permissions_updater.cc#newcode149 chrome/browser/extensions/permissions_updater.cc:149: if (token_service && token_service->HasOAuthLoginToken()) { no curlies :(
8 years, 2 months ago (2012-10-16 01:21:26 UTC) #6
Mihai Parparita -not on Chrome
On Monday, October 15, 2012, wrote: > > chrome/browser/extensions/**permissions_updater.cc:149: if (token_service > && token_service->**HasOAuthLoginToken()) { ...
8 years, 2 months ago (2012-10-16 04:08:44 UTC) #7
Evan Stade
8 years, 2 months ago (2012-10-16 04:33:21 UTC) #8
> The style guide says they're OK (
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Condit...)
> and the Chromium style additions don't mention them.
> 
> Mihai

"Either is fine, but be consistent."

http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/p...
File chrome/browser/extensions/permissions_updater.cc (right):

http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/p...
chrome/browser/extensions/permissions_updater.cc:138: extension->location() !=
Extension::INTERNAL)
^

http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/p...
chrome/browser/extensions/permissions_updater.cc:169: if (!profile_ ||
!profile_->GetExtensionEventRouter())
^

http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/p...
chrome/browser/extensions/permissions_updater.cc:184: if (!changed ||
changed->IsEmpty())
^

http://codereview.chromium.org/11066125/diff/4001/chrome/browser/extensions/p...
chrome/browser/extensions/permissions_updater.cc:212: if
(profile_->IsSameProfile(profile))
^

Powered by Google App Engine
This is Rietveld 408576698