Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2275113003:
Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. (Closed)
|
Created:
4 years, 3 months ago by Anton Obzhirov Modified:
4 years, 3 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionBuild error in chrome/browser/extensions/api/tabs/tabs_api.cc.
BUG=
Committed: https://crrev.com/e428744d02cbd5a6b479705b0ba7e92dd4d934f0
Cr-Commit-Position: refs/heads/master@{#414793}
Patch Set 1 #Patch Set 2 : Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. #
Total comments: 2
Patch Set 3 : Updated after review #
Total comments: 2
Patch Set 4 : Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. #Messages
Total messages: 16 (6 generated)
Description was changed from ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= ========== to ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= ==========
a.obzhirov@samsung.com changed reviewers: + rdevlin.cronin@chromium.org
a.obzhirov@samsung.com changed reviewers: + reillyg@chromium.org
https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/tabs/tabs_api.cc:264: bool IsHangoutsExtensionId(const std::string& extension_id) { Since this is only used in one location, maybe we should just inline it to avoid having yet-more #if defineds here. https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... chrome/browser/extensions/api/tabs/tabs_api.cc:526: bool saw_focus_key = false; can we put this on line 521 so that it's in the same block as create_ash_panel?
On 2016/08/25 20:51:06, Devlin wrote: > https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... > File chrome/browser/extensions/api/tabs/tabs_api.cc (right): > > https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... > chrome/browser/extensions/api/tabs/tabs_api.cc:264: bool > IsHangoutsExtensionId(const std::string& extension_id) { > Since this is only used in one location, maybe we should just inline it to avoid > having yet-more #if defineds here. > > https://codereview.chromium.org/2275113003/diff/20001/chrome/browser/extensio... > chrome/browser/extensions/api/tabs/tabs_api.cc:526: bool saw_focus_key = false; > can we put this on line 521 so that it's in the same block as create_ash_panel? Done. Thanks for the review, plz have a look.
Nice! One last nit. https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... chrome/browser/extensions/api/tabs/tabs_api.cc:532: // create a panel window. nit: comment wrapping was correct before.
https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... chrome/browser/extensions/api/tabs/tabs_api.cc:532: // create a panel window. On 2016/08/26 15:09:53, Devlin wrote: > nit: comment wrapping was correct before. Ups, I thought I was fixing it :)
On 2016/08/26 16:02:57, Anton Obzhirov wrote: > https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... > File chrome/browser/extensions/api/tabs/tabs_api.cc (right): > > https://codereview.chromium.org/2275113003/diff/40001/chrome/browser/extensio... > chrome/browser/extensions/api/tabs/tabs_api.cc:532: // create a panel window. > On 2016/08/26 15:09:53, Devlin wrote: > > nit: comment wrapping was correct before. > Ups, I thought I was fixing it :) Done.
lgtm, thanks!
The CQ bit was checked by a.obzhirov@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= ========== to ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= ========== to ========== Build error in chrome/browser/extensions/api/tabs/tabs_api.cc. BUG= Committed: https://crrev.com/e428744d02cbd5a6b479705b0ba7e92dd4d934f0 Cr-Commit-Position: refs/heads/master@{#414793} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e428744d02cbd5a6b479705b0ba7e92dd4d934f0 Cr-Commit-Position: refs/heads/master@{#414793} |
