Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2454743002:
Only call X11DesktopHandler for USE_X11 builds (Closed)
|
Created:
4 years, 1 month ago by tonikitoo Modified:
4 years, 1 month ago Reviewers:
rjkroege, not to use - tonikitoo, sadrul, fwang, Tom (Use chromium acct), Lei Zhang, Tom Anderson CC:
chromium-reviews, tfarina, fwang Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionOnly call X11DesktopHandler for USE_X11 builds
... and the same applies to calling ui::GetCustomFramePrefDefault,
implemented in ui/base/x/x11_util.cc
BUG=295089
Committed: https://crrev.com/f2f307b06528f971698a6e729a89bd74d6a06b23
Cr-Commit-Position: refs/heads/master@{#427830}
Patch Set 1 #
Total comments: 4
Patch Set 2 : addressed sadrul's suggestion #
Messages
Total messages: 28 (16 generated)
Description was changed from ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 ========== to ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 ==========
tonikitoo@igalia.com changed reviewers: + rjkroege@chromium.org, sadrul@chromium.org, thestig@chromium.org
PTAL
The CQ bit was checked by tonikitoo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
This seems to be the right thing to do to avoid compiling X11 stuff. informal l g t m
tonikitoo@igalia.com changed reviewers: + thomasanderson@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/browser_v... File chrome/browser/ui/browser_view_prefs.cc (right): https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/browser_v... chrome/browser/ui/browser_view_prefs.cc:35: #if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_X11) I would remove OS_LINUX. USE_X11 && !OS_CHROMEOS should be sufficient. https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/views/fra... File chrome/browser/ui/views/frame/browser_frame.cc (right): https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/views/fra... chrome/browser/ui/views/frame/browser_frame.cc:241: #if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_X11) ditto
https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/browser_v... File chrome/browser/ui/browser_view_prefs.cc (right): https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/browser_v... chrome/browser/ui/browser_view_prefs.cc:35: #if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_X11) On 2016/10/26 16:39:47, sadrul wrote: > I would remove OS_LINUX. USE_X11 && !OS_CHROMEOS should be sufficient. Done. https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/views/fra... File chrome/browser/ui/views/frame/browser_frame.cc (right): https://codereview.chromium.org/2454743002/diff/1/chrome/browser/ui/views/fra... chrome/browser/ui/views/frame/browser_frame.cc:241: #if defined(OS_LINUX) && !defined(OS_CHROMEOS) && defined(USE_X11) On 2016/10/26 16:39:47, sadrul wrote: > ditto Done.
lgtm with sadrul's approval.
thomasanderson@google.com changed reviewers: + thomasanderson@google.com
lgtm
lgtm
The CQ bit was checked by thomasanderson@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm fwiw.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tonikitoo@igalia.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 ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 ========== to ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 ========== to ========== Only call X11DesktopHandler for USE_X11 builds ... and the same applies to calling ui::GetCustomFramePrefDefault, implemented in ui/base/x/x11_util.cc BUG=295089 Committed: https://crrev.com/f2f307b06528f971698a6e729a89bd74d6a06b23 Cr-Commit-Position: refs/heads/master@{#427830} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f2f307b06528f971698a6e729a89bd74d6a06b23 Cr-Commit-Position: refs/heads/master@{#427830}
Message was sent while issue was closed.
fwang@igalia.com changed reviewers: + fwang@igalia.com
Message was sent while issue was closed.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
