Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2004613002:
Add support for inexact resize on X11 (Closed)
DescriptionAdd support for inexact resize on X11
Support switching between existing modes when XRandR is available but
exact resize is not.
BUG=596125
Committed: https://crrev.com/211658866f3462be8edc3dd80865d6b68266ca81
Cr-Commit-Position: refs/heads/master@{#398709}
Patch Set 1 #
Total comments: 31
Patch Set 2 : Address first round of comments #
Total comments: 8
Patch Set 3 : Address further comments #Messages
Total messages: 19 (5 generated)
rkjnsn@chromium.org changed reviewers: + jamiewalch@chromium.org, lambroslambrou@chromium.org
Here's an initial approach for supporting resize outside of our custom Xvfb-randr server. It should work both with Xorg+dummy and in non-curtain-mode me2me contexts. You can test it with a command such as the following: env CHROME_REMOTE_DESKTOP_USE_XORG=1 CHROME_REMOTE_DESKTOP_DEFAULT_DESKTOP_SIZES=1600x1200,1600x900,1440x900,1366x768,1360x768,1280x1024,1280x800,1280x768,1280x720,1152x864,1024x768,1024x600,800x600,1680x1050,1920x1080,1920x1200,2560x1440,2560x1600,3840x2160 ./chrome-remote-desktop --start If you connect and resize the window, it should automatically pick the best resolution for that window size. It should also work on an uncurtained session, though the original resolution won't currently be restored on disconnect. Instead of looking for the exact-resize flag, it should be possible to determine based on the RandR version and whether we are in curtain mode. (We don't want to add extra modes to a session connected to a real display.) Additionally we'll want to restore the initial resolution on disconnect if and only if the session is not curtained. Removing the exact-resize flag has the advantage of not requiring the Python script to detect whether the installed dummy driver supports exact resize or not. If the above sounds like they way we want to go, I can either update this CL or do a second one to remove the flag after this is merged. Either way, we'll probably want to ensure that uncurtained sessions are restored to their original resolution when the client disconnects before this gets merged. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:144: void SetResolutionRandr(const ScreenResolution& resolution); Are these decent names for these functions? https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:165: int rr_error_base_; We don't currently use either of these, so it might make sense to discard the values returned by XRRQueryExtension until we need to do something that actually makes use of them. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:178: // open? It doesn't look like we do, elsewhere. It appears we generally assume that XOpenDisplay succeeds, and thus display_, screen_, and root_ are all always valid, so I do the same here, for now. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:191: // (Obviously we'd want to skip the XRRUpdateConfiguration loop.) I think we could return something meaningful here even when XRandR isn't supported, as I believe DisplayWidth and DisplayHeight are base Xlib features, and not RandR specific. Of course, the XRRUpdateConfiguration loop in RandR specific, but that should be fine since if RandR isn't supported, there won't be RandR events that need to be processed to update DisplayWidth and DisplayHeight. Would returning something here when we don't support resize confuse logic elsewhere? https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:201: // don't have one, hence this horrible hack. I wasn't sure what the actual TODO item is, here, and at what point it would make sense to do it. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:217: // TODO(rkjnsn): Do we want to calculate this from the screen dimensions? We seem to assume 96 DPI everywhere. Would it make sense to calculate the actually DPI when we're not in curtain mode?
https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:144: void SetResolutionRandr(const ScreenResolution& resolution); On 2016/05/20 22:08:40, rkjnsn wrote: > Are these decent names for these functions? Since they both use RANDR, maybe SetResolutionNewMode and SetResolutionExistingMode might be better choices. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:165: int rr_error_base_; On 2016/05/20 22:08:39, rkjnsn wrote: > We don't currently use either of these, so it might make sense to discard the > values returned by XRRQueryExtension until we need to do something that actually > makes use of them. SGTM. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:178: // open? It doesn't look like we do, elsewhere. On 2016/05/20 22:08:40, rkjnsn wrote: > It appears we generally assume that XOpenDisplay succeeds, and thus display_, > screen_, and root_ are all always valid, so I do the same here, for now. If this is standard elsewhere, then it's okay to do it here (and you don't need a TODO). https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:191: // (Obviously we'd want to skip the XRRUpdateConfiguration loop.) On 2016/05/20 22:08:39, rkjnsn wrote: > I think we could return something meaningful here even when XRandR isn't > supported, as I believe DisplayWidth and DisplayHeight are base Xlib features, > and not RandR specific. Of course, the XRRUpdateConfiguration loop in RandR > specific, but that should be fine since if RandR isn't supported, there won't be > RandR events that need to be processed to update DisplayWidth and DisplayHeight. > > Would returning something here when we don't support resize confuse logic > elsewhere? I'll defer to Lambros here, given his TODO https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:217: // TODO(rkjnsn): Do we want to calculate this from the screen dimensions? On 2016/05/20 22:08:39, rkjnsn wrote: > We seem to assume 96 DPI everywhere. Would it make sense to calculate the > actually DPI when we're not in curtain mode? I wouldn't worry about it for now, unless it's a really trivial change. AFAIK, no-one is using this for non-virtual sessions, since we don't make it easy to do so and it's not supported. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:256: XRRFreeScreenConfigInfo(config); Use the ScreenResources class to manage the lifetime of these. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:293: // if and only if RandR is supported and we are not in curtain mode. Agreed; that makes sense.
https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:18: // now, we only support resize-to-client for Xvfb-based servers that can match Update this comment. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:191: // (Obviously we'd want to skip the XRRUpdateConfiguration loop.) On 2016/05/20 22:08:39, rkjnsn wrote: > I think we could return something meaningful here even when XRandR isn't > supported, as I believe DisplayWidth and DisplayHeight are base Xlib features, > and not RandR specific. Of course, the XRRUpdateConfiguration loop in RandR > specific, but that should be fine since if RandR isn't supported, there won't be > RandR events that need to be processed to update DisplayWidth and DisplayHeight. > > Would returning something here when we don't support resize confuse logic > elsewhere? I can't remember why we return an invalid resolution. Makes sense to fix this to return the actual resolution in all cases. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:201: // don't have one, hence this horrible hack. On 2016/05/20 22:08:40, rkjnsn wrote: > I wasn't sure what the actual TODO item is, here, and at what point it would > make sense to do it. I don't know what the TODO is, either :) The problem seems to be that we're trying to use multiple X connections on the same thread, which is why we have to manually pump the message queue. The proper fixes are to either: * Pass in an existing Display*, instead of opening a new connection. * Open a new X connection on a new thread, and make it a UI thread that pumps X messages. I'm not exactly sure what blocks us from doing either of these. In any case, we'd have to hook into the message-loop to spy on X messages and pass the "relevant" ones into XRRUpdateConfiguration(). https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:235: // Additionally impose a minimum size of 640x480, since anything smaller Maybe add a blank line before this comment, though it's not part of this CL? https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:251: // TODO(rkjnsn): Do we want to calculate this from the screen nit: Blank line before comment. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:256: XRRFreeScreenConfigInfo(config); On 2016/05/20 23:56:19, Jamie wrote: > Use the ScreenResources class to manage the lifetime of these. I guess you'd need a separate scoper, with a dtor that calls this specific XRRFree...() function? I think Chromium supports some way of writing custom scopers like this, though I'm not sure it's worth the effort in this case? https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:346: XRRFreeScreenConfigInfo(config); If you do a custom scoper for XRRFreeScreenConfigInfo() above, you might as well do another one here?
sergeyu@chromium.org changed reviewers: + sergeyu@chromium.org
https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:201: // don't have one, hence this horrible hack. On 2016/05/21 00:57:00, Lambros wrote: > On 2016/05/20 22:08:40, rkjnsn wrote: > > I wasn't sure what the actual TODO item is, here, and at what point it would > > make sense to do it. > > I don't know what the TODO is, either :) > The problem seems to be that we're trying to use multiple X connections on the > same thread, which is why we have to manually pump the message queue. I don't think there are any other X connections used on the main thread. Input injector and screen capturer both use their own X display on their own threads (input injector actually uses two x connections: one for input injection, and one for clipboard) > The proper fixes are to either: > * Pass in an existing Display*, instead of opening a new connection. > * Open a new X connection on a new thread, and make it a UI thread that pumps X > messages. > > I'm not exactly sure what blocks us from doing either of these. In any case, > we'd have to hook into the message-loop to spy on X messages and pass the > "relevant" ones into XRRUpdateConfiguration(). Potentially we could share this X connection with the input injector, but that won't do anything about this TODO as we will still need to pump the events somewhere. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:217: // TODO(rkjnsn): Do we want to calculate this from the screen dimensions? On 2016/05/20 22:08:39, rkjnsn wrote: > We seem to assume 96 DPI everywhere. Would it make sense to calculate the > actually DPI when we're not in curtain mode? This code is used only with Me2Me where we only support standard DPI at the moment. The DPI that's reported to the client comes from ScreenCapturer. ScreenCapturerLinux currently doesn't return DPI for captured frames, so the client gets 96x96. Ideally we should fix it (in ScreenCapturerLinux and here). But it would be wrong to use physical screen dimensions to calculate DPI. Applications don't scale their content according to the screen dimensions and it's the only thing that matters here. I think we will want something like this: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
On 2016/05/22 20:00:17, Sergey Ulanov (OOO) wrote: > https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... > File remoting/host/desktop_resizer_x11.cc (right): > > https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... > remoting/host/desktop_resizer_x11.cc:201: // don't have one, hence this horrible > hack. > On 2016/05/21 00:57:00, Lambros wrote: > > On 2016/05/20 22:08:40, rkjnsn wrote: > > > I wasn't sure what the actual TODO item is, here, and at what point it would > > > make sense to do it. > > > > I don't know what the TODO is, either :) > > The problem seems to be that we're trying to use multiple X connections on the > > same thread, which is why we have to manually pump the message queue. > > I don't think there are any other X connections used on the main thread. Input > injector and screen capturer both use their own X display on their own threads > (input injector actually uses two x connections: one for input injection, and > one for clipboard) > > > The proper fixes are to either: > > * Pass in an existing Display*, instead of opening a new connection. > > * Open a new X connection on a new thread, and make it a UI thread that pumps > X > > messages. > > > > I'm not exactly sure what blocks us from doing either of these. In any case, > > we'd have to hook into the message-loop to spy on X messages and pass the > > "relevant" ones into XRRUpdateConfiguration(). > > Potentially we could share this X connection with the input injector, but that > won't do anything about this TODO as we will still need to pump the events > somewhere. Ideally you'd wrap up a Display* into a ref-counted object so that while it remains live the message-loop can pump it, and components that use the Display shouldn't need to pump it themselves except in very special situations. IIRC there is one case where we need a separate X display, because we use XRecord for local-input monitoring?
I uploaded a new version of the patch. It addresses most of the comments and removes my TODOs. It also enables RestoreResolution, since we now change the screen resolution on uncurtained Linux sessions as well, and we want to change those back. This is currently a behavior chance on curtained Linux sessions using Xvfb-randr. I will prepare another CL that will disable restore behavior when in curtain mode on all platforms, and we might want to wait for that before committing this one to avoid the behavior change. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:18: // now, we only support resize-to-client for Xvfb-based servers that can match On 2016/05/21 00:57:00, Lambros wrote: > Update this comment. Done. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:144: void SetResolutionRandr(const ScreenResolution& resolution); On 2016/05/20 23:56:19, Jamie wrote: > On 2016/05/20 22:08:40, rkjnsn wrote: > > Are these decent names for these functions? > > Since they both use RANDR, maybe SetResolutionNewMode and > SetResolutionExistingMode might be better choices. Done. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:165: int rr_error_base_; On 2016/05/20 23:56:19, Jamie wrote: > On 2016/05/20 22:08:39, rkjnsn wrote: > > We don't currently use either of these, so it might make sense to discard the > > values returned by XRRQueryExtension until we need to do something that > actually > > makes use of them. > > SGTM. Done. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:178: // open? It doesn't look like we do, elsewhere. On 2016/05/20 23:56:19, Jamie wrote: > On 2016/05/20 22:08:40, rkjnsn wrote: > > It appears we generally assume that XOpenDisplay succeeds, and thus display_, > > screen_, and root_ are all always valid, so I do the same here, for now. > > If this is standard elsewhere, then it's okay to do it here (and you don't need > a TODO). Done. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:191: // (Obviously we'd want to skip the XRRUpdateConfiguration loop.) On 2016/05/21 00:57:00, Lambros wrote: > On 2016/05/20 22:08:39, rkjnsn wrote: > > I think we could return something meaningful here even when XRandR isn't > > supported, as I believe DisplayWidth and DisplayHeight are base Xlib features, > > and not RandR specific. Of course, the XRRUpdateConfiguration loop in RandR > > specific, but that should be fine since if RandR isn't supported, there won't > be > > RandR events that need to be processed to update DisplayWidth and > DisplayHeight. > > > > Would returning something here when we don't support resize confuse logic > > elsewhere? > > I can't remember why we return an invalid resolution. Makes sense to fix this to > return the actual resolution in all cases. > Done. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:201: // don't have one, hence this horrible hack. On 2016/05/22 20:00:17, Sergey Ulanov (OOO) wrote: > On 2016/05/21 00:57:00, Lambros wrote: > > On 2016/05/20 22:08:40, rkjnsn wrote: > > > I wasn't sure what the actual TODO item is, here, and at what point it would > > > make sense to do it. > > > > I don't know what the TODO is, either :) > > The problem seems to be that we're trying to use multiple X connections on the > > same thread, which is why we have to manually pump the message queue. > > I don't think there are any other X connections used on the main thread. Input > injector and screen capturer both use their own X display on their own threads > (input injector actually uses two x connections: one for input injection, and > one for clipboard) > > > The proper fixes are to either: > > * Pass in an existing Display*, instead of opening a new connection. > > * Open a new X connection on a new thread, and make it a UI thread that pumps > X > > messages. > > > > I'm not exactly sure what blocks us from doing either of these. In any case, > > we'd have to hook into the message-loop to spy on X messages and pass the > > "relevant" ones into XRRUpdateConfiguration(). > > Potentially we could share this X connection with the input injector, but that > won't do anything about this TODO as we will still need to pump the events > somewhere. Are there any specific action items that I should notate in the code for this? Or would it make sense to remove "TODO" and leave the rest as a normal comment? https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:217: // TODO(rkjnsn): Do we want to calculate this from the screen dimensions? On 2016/05/22 20:00:17, Sergey Ulanov (OOO) wrote: > On 2016/05/20 22:08:39, rkjnsn wrote: > > We seem to assume 96 DPI everywhere. Would it make sense to calculate the > > actually DPI when we're not in curtain mode? > > This code is used only with Me2Me where we only support standard DPI at the > moment. The DPI that's reported to the client comes from ScreenCapturer. > ScreenCapturerLinux currently doesn't return DPI for captured frames, so the > client gets 96x96. Ideally we should fix it (in ScreenCapturerLinux and here). > But it would be wrong to use physical screen dimensions to calculate DPI. > Applications don't scale their content according to the screen dimensions and > it's the only thing that matters here. I think we will want something like this: > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/... Acknowledged. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:256: XRRFreeScreenConfigInfo(config); On 2016/05/20 23:56:19, Jamie wrote: > Use the ScreenResources class to manage the lifetime of these. I wouldn't be able to reuse ScreenResources for this. I'd have to make my own ScreenConfig class. How much of an anathema is manually freeing a resource like this in Chrome? I'll add a class for it if you think it's worth the effort. https://codereview.chromium.org/2004613002/diff/1/remoting/host/desktop_resiz... remoting/host/desktop_resizer_x11.cc:293: // if and only if RandR is supported and we are not in curtain mode. On 2016/05/20 23:56:19, Jamie wrote: > Agreed; that makes sense. Per our discussion, this now unconditionally calls SetResolution, and it will be up to the cross-platform logic to avoid calling RestoreResolution when we are in curtain mode. I'll probably put that in another CL, but we may want to wait until it's ready before committing this one, as otherwise it would be a behavior change on curtained Linux.
lgtm
Okay, now that restoring the resolution is conditional on not being in curtain mode, this should be pretty much ready to land. Please see my last couple of questions below. Thanks. https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:197: // file descriptor is never seen as readable. Given the previous discussion, are there any specific action items that I should notate in the code for this? Or would it make sense to remove "TODO" and leave the rest as a normal comment? https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:246: XRRFreeScreenConfigInfo(config); As I noted before, I wouldn't be able to reuse ScreenResources for this. I'd have to make my own ScreenConfig class. How much of an anathema is manually freeing a resource like this in Chrome? I'll add a class for it if you think it's worth the effort.
LGTM, but please wait for Lambros to sign off as well. https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:197: // file descriptor is never seen as readable. On 2016/06/07 23:02:10, rkjnsn wrote: > Given the previous discussion, are there any specific action items that I should > notate in the code for this? Or would it make sense to remove "TODO" and leave > the rest as a normal comment? I think just removing the TODO part of the comment is fine. https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:246: XRRFreeScreenConfigInfo(config); On 2016/06/07 23:02:10, rkjnsn wrote: > As I noted before, I wouldn't be able to reuse ScreenResources for this. I'd > have to make my own ScreenConfig class. How much of an anathema is manually > freeing a resource like this in Chrome? I'll add a class for it if you think > it's worth the effort. No, I think this is fine. I didn't spot that it was a different cleanup function.
lgtm https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:18: // curtain mode, we do exact resize where supported (currently only using our "our" is a bit weird. It's not obvious (or desirable?) that Google is speaking here? Maybe use more neutral language, such as "... if the version of Xvfb is patched to support it". https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:176: int rr_event_base; Don't need these, since you already have rr_event_base_ ... members.
https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... File remoting/host/desktop_resizer_x11.cc (right): https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:18: // curtain mode, we do exact resize where supported (currently only using our On 2016/06/08 00:12:39, Lambros wrote: > "our" is a bit weird. It's not obvious (or desirable?) that Google is speaking > here? > Maybe use more neutral language, such as "... if the version of Xvfb is patched > to support it". I'll change it to read "(currently only using a patched Xvfb server)" https://codereview.chromium.org/2004613002/diff/20001/remoting/host/desktop_r... remoting/host/desktop_resizer_x11.cc:176: int rr_event_base; On 2016/06/08 00:12:39, Lambros wrote: > Don't need these, since you already have rr_event_base_ ... members. Oops. The goal was to avoid adding the rr_event_base_ and rr_error_base_ members, since we don't use them anywhere. Nice catch. (I'm actually surprised this didn't throw an "unused private member" warning.)
The CQ bit was checked by rkjnsn@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lambroslambrou@chromium.org, jamiewalch@chromium.org Link to the patchset: https://codereview.chromium.org/2004613002/#ps40001 (title: "Address further comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2004613002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Add support for inexact resize on X11 Support switching between existing modes when XRandR is available but exact resize is not. BUG=596125 ========== to ========== Add support for inexact resize on X11 Support switching between existing modes when XRandR is available but exact resize is not. BUG=596125 Committed: https://crrev.com/211658866f3462be8edc3dd80865d6b68266ca81 Cr-Commit-Position: refs/heads/master@{#398709} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/211658866f3462be8edc3dd80865d6b68266ca81 Cr-Commit-Position: refs/heads/master@{#398709} |
