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

Issue 2700523002: arc: Fix crash on accessing app info for secondary user. (Closed)

Created:
3 years, 10 months ago by khmel
Modified:
3 years, 10 months ago
Reviewers:
msw
CC:
chromium-reviews, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

arc: Fix crash on accessing app info for secondary user. This fix the crash whem user activates Chrome app info page for secondary user profile. This alse adds unit test that cover this case and generic functionality. TEST=Manually, unit tests extended BUG=689263 Review-Url: https://codereview.chromium.org/2700523002 Cr-Commit-Position: refs/heads/master@{#451118} Committed: https://chromium.googlesource.com/chromium/src/+/b756297e51407c3b602c6d65746a8f18d835d71b

Patch Set 1 #

Patch Set 2 : clean up #

Total comments: 25

Patch Set 3 : rebase + comments addressed #

Total comments: 2

Patch Set 4 : minor comment update #

Unified diffs Side-by-side diffs Delta from patch set Stats (+134 lines, -59 lines) Patch
M chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.h View 1 2 2 chunks +7 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc View 1 2 3 chunks +14 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views_unittest.cc View 1 2 3 10 chunks +113 lines, -44 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
khmel
Hi Mike, PTAL https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc (right): https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc#newcode147 chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:147: // "Manage supported links" link for ...
3 years, 10 months ago (2017-02-15 16:38:45 UTC) #2
msw
https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc (right): https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc#newcode146 chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:146: // When ARC is enabled and Settings app is ...
3 years, 10 months ago (2017-02-15 21:48:29 UTC) #3
khmel
Thank you for taking a look. Please find updated version. Thanks! https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc (right): ...
3 years, 10 months ago (2017-02-16 15:47:52 UTC) #4
msw
lgtm with a nit and a minor comment. https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc (right): https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc#newcode147 chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:147: // ...
3 years, 10 months ago (2017-02-16 18:55:15 UTC) #5
khmel
Thank you for review! https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc File chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc (right): https://codereview.chromium.org/2700523002/diff/20001/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc#newcode147 chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.cc:147: // "Manage supported links" link ...
3 years, 10 months ago (2017-02-16 20:33:30 UTC) #6
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/2700523002/60001
3 years, 10 months ago (2017-02-16 20:36:34 UTC) #9
commit-bot: I haz the power
3 years, 10 months ago (2017-02-16 22:25:11 UTC) #12
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b756297e51407c3b602c6d65746a...

Powered by Google App Engine
This is Rietveld 408576698