Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1896993003:
Not use ProfileCompare when removing profile. (Closed)
|
Created:
4 years, 8 months ago by Azure Wei Modified:
4 years, 8 months ago Reviewers:
Shu Chen CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionInstead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly.
BUG=602746
TEST=Verfied not repo on local build.
Committed: https://crrev.com/13814091e327ebc1792dfb329b88d2289568a78a
Cr-Commit-Position: refs/heads/master@{#388431}
Patch Set 1 #Patch Set 2 : Remove ProfileCompare. #
Total comments: 4
Patch Set 3 : #Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #Messages
Total messages: 17 (7 generated)
Description was changed from ========== Not remove profile for ChromeOS. BUG=602746 TEST=Verfied not repo on local build. ========== to ========== Not remove profile for ChromeOS. BUG=602746 TEST=Verfied not repo on local build. ==========
azurewei@chromium.org changed reviewers: + shuchen@chromium.org
Hello Shu, please review this cl. Thank you!
Description was changed from ========== Not remove profile for ChromeOS. BUG=602746 TEST=Verfied not repo on local build. ========== to ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ==========
On 2016/04/19 05:59:35, Azure Wei wrote: > Hello Shu, please review this cl. Thank you! This cl was updated, please review the newest patch set.
On 2016/04/19 05:59:35, Azure Wei wrote: > Hello Shu, please review this cl. Thank you! This cl was updated, please review the newest patch set.
https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/api/input_ime/input_ime_api.cc (left): https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/input_ime/input_ime_api.cc:415: return nullptr; the null check here seems unnecessary. https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/api/input_ime/input_ime_api.h (right): https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/input_ime/input_ime_api.h:120: std::unordered_map<Profile*, InputImeEventRouter*> router_map_; why using unordered_map?
https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/api/input_ime/input_ime_api.cc (left): https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/input_ime/input_ime_api.cc:415: return nullptr; On 2016/04/19 07:27:17, Shu Chen wrote: > the null check here seems unnecessary. Removed. https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/api/input_ime/input_ime_api.h (right): https://codereview.chromium.org/1896993003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/input_ime/input_ime_api.h:120: std::unordered_map<Profile*, InputImeEventRouter*> router_map_; On 2016/04/19 07:27:17, Shu Chen wrote: > why using unordered_map? We don't need to sort the profile, so std::unordered_map seems more efficient and convenient here.
Description was changed from ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ========== to ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ==========
Please review the updated patch set. Thanks!
The CQ bit was checked by shuchen@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1896993003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1896993003/100001
Message was sent while issue was closed.
Description was changed from ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ========== to ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. ========== to ========== Instead of using ProfileCompare in InputImeEventRouterFactory::router_map_ when trying to remove profile, just compares the profile directly. BUG=602746 TEST=Verfied not repo on local build. Committed: https://crrev.com/13814091e327ebc1792dfb329b88d2289568a78a Cr-Commit-Position: refs/heads/master@{#388431} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/13814091e327ebc1792dfb329b88d2289568a78a Cr-Commit-Position: refs/heads/master@{#388431} |
