Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
15927033:
Add host-side rate-limiting to desktop resize events. (Closed)
|
Created:
7 years, 6 months ago by Jamie Modified:
7 years, 6 months ago Reviewers:
alexeypa (please no reviews) CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, rmsousa+watch_chromium.org, weitaosu+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdd host-side rate-limiting to desktop resize events.
Also, make the client-side rate-limiting more granular. This means that the
desktop resizes sooner after the user stops resizing the window.
BUG=187272
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205093
Patch Set 1 #
Total comments: 14
Patch Set 2 : Reviewer feedback. #
Total comments: 9
Patch Set 3 : Reviewer comments. #Patch Set 4 : Rebase #Patch Set 5 : Fixed ResizingHostObserver unit-tests. #
Total comments: 8
Patch Set 6 : Use QuitClosure instead of Bind. #Patch Set 7 : Made unit-test less sensitive to timing. #
Total comments: 10
Patch Set 8 : Reviewer feedback. #Patch Set 9 : Rebase #
Messages
Total messages: 24 (0 generated)
ptal https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:17: const int kMinimumResizeIntervalMs = 1000; This value is taken directly from the previous implementation. The bug report suggests making this configurable per-platform. I don't think that's worth doing unless we have some data on what suitable resize times might be on each platform. https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... File remoting/webapp/client_session.js (right): https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... remoting/webapp/client_session.js:888: 250); 250ms seems to be a good compromise between sending the resize command as soon as possible after the user stops dragging the window border and not sending erroneous resize commands while s/he is still doing so (sending resize commands too early is bad because it causes the host-side rate-limiting to kick in, delaying resizing the desktop to the intended size).
https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:17: const int kMinimumResizeIntervalMs = 1000; nit #1: Add a comment explaining what this timeout is for. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:126: base::Time::Now() - previous_resize_time_; nit: This looks like potential integer overflow. if (previous_resize_time_ + base::TimeDelta::FromMilliseconds(kMinimumResizeIntervalMs) <= base::Time::Now()) { ... } https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:134: base::MessageLoop::current()->PostDelayedTask( nit: Actually base::OneShotTimer expresses the intent much better. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:167: resize_pending_ = false; nit: This flag and the whole function is not needed when base::OneShotTimer is used. SetScreenResolution() will simply rearm the timer every time a resize message arrives too early. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... File remoting/host/resizing_host_observer.h (right): https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.h:41: ScreenResolution pending_resolution_; nit: Add comments explaining what these members are for. https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... File remoting/webapp/client_session.js (right): https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... remoting/webapp/client_session.js:888: 250); Consider using different timeout depending on the kind of the host we are connected. The timeout should be shorter for the host implementing rate-limiting. A new capability needs to be added to the host and webapp to be able to do that. Look for 'sendInitialResolution' to see how it is done. The only caveat is that this new capability should be returned by all implementation of DesktopEnvironment::GetCapabilities().
ptal https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:17: const int kMinimumResizeIntervalMs = 1000; On 2013/06/04 00:05:26, alexeypa wrote: > nit #1: Add a comment explaining what this timeout is for. Done. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:126: base::Time::Now() - previous_resize_time_; On 2013/06/04 00:05:26, alexeypa wrote: > nit: This looks like potential integer overflow. > > if (previous_resize_time_ + > base::TimeDelta::FromMilliseconds(kMinimumResizeIntervalMs) <= > base::Time::Now()) { > ... > } I've rewritten it to something I think is a bit clearer than both my original and your suggested replacement. LMKWYT. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:134: base::MessageLoop::current()->PostDelayedTask( On 2013/06/04 00:05:26, alexeypa wrote: > nit: Actually base::OneShotTimer expresses the intent much better. Nice, I was wasn't familiar with that. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.cc:167: resize_pending_ = false; On 2013/06/04 00:05:26, alexeypa wrote: > nit: This flag and the whole function is not needed when base::OneShotTimer is > used. SetScreenResolution() will simply rearm the timer every time a resize > message arrives too early. Done. https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... File remoting/host/resizing_host_observer.h (right): https://codereview.chromium.org/15927033/diff/1/remoting/host/resizing_host_o... remoting/host/resizing_host_observer.h:41: ScreenResolution pending_resolution_; On 2013/06/04 00:05:26, alexeypa wrote: > nit: Add comments explaining what these members are for. Done. https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... File remoting/webapp/client_session.js (right): https://codereview.chromium.org/15927033/diff/1/remoting/webapp/client_sessio... remoting/webapp/client_session.js:888: 250); On 2013/06/04 00:05:26, alexeypa wrote: > Consider using different timeout depending on the kind of the host we are > connected. The timeout should be shorter for the host implementing > rate-limiting. > > A new capability needs to be added to the host and webapp to be able to do that. > Look for 'sendInitialResolution' to see how it is done. The only caveat is that > this new capability should be returned by all implementation of > DesktopEnvironment::GetCapabilities(). Done. https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... File remoting/host/client_session.cc (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... remoting/host/client_session.cc:218: host_capabilities_ = host_capabilities_ + " " + kRateLimitResizeRequests; Is this an acceptable way of getting the capability registered (as opposed to doing so in every DesktopEnvironment)?
https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... File remoting/host/client_session.cc (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... remoting/host/client_session.cc:218: host_capabilities_ = host_capabilities_ + " " + kRateLimitResizeRequests; On 2013/06/04 01:27:49, Jamie wrote: > Is this an acceptable way of getting the capability registered (as opposed to > doing so in every DesktopEnvironment)? No, I don't think we should do it this way. I think each desktop environment should explicitly specify this capability. I realize that it means more code has to be written but there are two compelling reasons to do it: 1. It is very easy to miss that all DE instances should implement rate-limiting unless rate limiting is implemented in ClientSession. (which is probably not a good idea too). 2. There are at least two kinds DE that don't implement rate limiting even with this CL applied: It2MeDesktopEnvironment and IpcDesktopEnvironment. The latter also invokes different code for curtained (RdpDesktopSession) and not curtained (Me2meDesktopEnvironment) sessions. https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.cc:128: if (base::Time::Now() < next_allowed_resize) { nit: I think |previous_resize_time_| is not really needed too. Instead you can arm the timer after changing the desktop size. I.e.: { if (resolution.IsEmpty() || resolution.dimensions() == desktop_resizer_->GetCurrentSize()) { return; } ... if (deferred_resize_timer_.IsPending()) { deferred_resize_timer_.Start(...); return; } ... desktop_resizer_->SetSize(best_size.size()); deferred_resize_timer_.Start(current_size); } https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... File remoting/host/resizing_host_observer.h (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.h:37: void SetPendingScreenResolution(); Remove this line. https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.h:44: base::OneShotTimer<ResizingHostObserver> deferred_resize_timer_; nit: move |deferred_resize_timer_| in front of |previous_resize_time_|.
ptal https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... File remoting/host/client_session.cc (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/client_sessi... remoting/host/client_session.cc:218: host_capabilities_ = host_capabilities_ + " " + kRateLimitResizeRequests; On 2013/06/04 17:07:37, alexeypa wrote: > On 2013/06/04 01:27:49, Jamie wrote: > > Is this an acceptable way of getting the capability registered (as opposed to > > doing so in every DesktopEnvironment)? > > No, I don't think we should do it this way. I think each desktop environment > should explicitly specify this capability. I realize that it means more code has > to be written but there are two compelling reasons to do it: Done. However, see my comments below. > 1. It is very easy to miss that all DE instances should implement rate-limiting > unless rate limiting is implemented in ClientSession. (which is probably not a > good idea too). Rate-limiting is implemented by a platform independent class, so any desktop environment that supports resize gets it automatically. If a desktop environment doesn't support resize, then we end up reporting that it rate-limits resize requests, which is misleading, but not harmful afaict. > 2. There are at least two kinds DE that don't implement rate limiting even with > this CL applied: It2MeDesktopEnvironment and IpcDesktopEnvironment. The latter > also invokes different code for curtained (RdpDesktopSession) and not curtained > (Me2meDesktopEnvironment) sessions. I believe that IpcDesktopEnvironment does implement rate-limiting, as long as the underlying DE does. In any case, with the change as I've implemented it now, it still reports that it implements rate-limiting without consulting the underlying DE, as that would require extending the IPC, which is overkill imo. https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.cc:128: if (base::Time::Now() < next_allowed_resize) { On 2013/06/04 17:07:37, alexeypa wrote: > nit: I think |previous_resize_time_| is not really needed too. Instead you can > arm the timer after changing the desktop size. I.e.: > > { > if (resolution.IsEmpty() || > resolution.dimensions() == desktop_resizer_->GetCurrentSize()) { > return; > } > > ... > if (deferred_resize_timer_.IsPending()) { > deferred_resize_timer_.Start(...); > return; > } > > ... > desktop_resizer_->SetSize(best_size.size()); > > deferred_resize_timer_.Start(current_size); > } That saves a bit of state, but I think it's less clear, and it relies on consulting the underlying DesktopResizer in order to detect that a resize has already happened. In particular, if the desktop resizer hasn't finished resizing, or if the size has changed again since we set up the timer, it will do the wrong thing. https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... File remoting/host/resizing_host_observer.h (right): https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.h:37: void SetPendingScreenResolution(); On 2013/06/04 17:07:37, alexeypa wrote: > Remove this line. Done. https://codereview.chromium.org/15927033/diff/6001/remoting/host/resizing_hos... remoting/host/resizing_host_observer.h:44: base::OneShotTimer<ResizingHostObserver> deferred_resize_timer_; On 2013/06/04 17:07:37, alexeypa wrote: > nit: move |deferred_resize_timer_| in front of |previous_resize_time_|. Done.
Alex: ping.
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/15927033/16001
Step "update" is always a major failure. Look at the try server FAQ for more details. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/15927033/50001
Retried try job too often on linux_rel for step(s) remoting_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Forgot to update the unit-tests. PTAL.
https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer_unittest.cc (right): https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:201: int interval_ms = 10; 1ms should be good enough. All we need is to cause a task to be posted to the message loop. Any non zero interval will work. Also, there is no need to make it a variable. Remove |interval_ms|. https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:211: EXPECT_EQ(GetBestSize(SkISize::Make(100, 100)), SkISize::Make(100, 100)); If I read this correctly, it does not check whether the size was eventually set. Can you add an expectation that size will be changed twice? You will be able to bind posting of QuitClosure() to this expectation. https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:217: base::Bind(&base::RunLoop::Quit, base::Unretained(&run_loop)), Use |run_loop.QuitClosure()| instead of base::Bind.
ptal https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer_unittest.cc (right): https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:201: int interval_ms = 10; On 2013/06/06 21:45:11, alexeypa wrote: > 1ms should be good enough. All we need is to cause a task to be posted to the > message loop. Any non zero interval will work. There is a small chance that it might take > 1ms between the first two calls to GetBestSize, which would make the test flaky. It's very unlikely, but I think an extra 9ms is worth it in this case. LMK if you still disagree and I'll change it as you suggest. > Also, there is no need to make it a variable. Remove |interval_ms|. It's also used as a parameter to PostDelayedTask, so I'd prefer to keep it. https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:211: EXPECT_EQ(GetBestSize(SkISize::Make(100, 100)), SkISize::Make(100, 100)); On 2013/06/06 21:45:11, alexeypa wrote: > If I read this correctly, it does not check whether the size was eventually set. > Can you add an expectation that size will be changed twice? You will be able to > bind posting of QuitClosure() to this expectation. That expectation is at the end of the test, after the RunLoop exits. https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:217: base::Bind(&base::RunLoop::Quit, base::Unretained(&run_loop)), On 2013/06/06 21:45:11, alexeypa wrote: > Use |run_loop.QuitClosure()| instead of base::Bind. Done.
https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer_unittest.cc (right): https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:201: int interval_ms = 10; On 2013/06/06 22:08:04, Jamie wrote: > On 2013/06/06 21:45:11, alexeypa wrote: > > 1ms should be good enough. All we need is to cause a task to be posted to the > > message loop. Any non zero interval will work. > > There is a small chance that it might take > 1ms between the first two calls to > GetBestSize, which would make the test flaky. It's very unlikely, but I think an > extra 9ms is worth it in this case. LMK if you still disagree and I'll change it > as you suggest. Oh, I see. So the first SetScreenResoultion() records base::Time::Now() but by the time of the seconds call base::Time::Now() can already return an updated value. In this case 10ms is not enough. The precision of base::Time::Now() is 18ms or so on Windows. Also if the machine running the test is under heavy load the thread could easily be suspended for longer time. I.e. this is flaky. Another approach we can try is instead of setting the timeout via SetMinimumResizeIntervalForTesting() the test should control whether the next resize attempt should be allowed or result in a PostTask (Arming the timer for 0ms seems to be doing PostTask). https://codereview.chromium.org/15927033/diff/65001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:221: EXPECT_EQ(desktop_resizer_->GetCurrentSize(), SkISize::Make(300, 300)); This expectation is flaky. Posting QuitClosure() before the timeout elapses will exit the loop before the delayed tasks have a chance to be executed.
As discussed, I've changed the unit-test seam to allow Time::Now to be mocked out, which I think eliminates the flakiness.
lgtm once my comments are addressed. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer.cc:127: base::TimeDelta minimum_resize_interval = base::TimeDelta::FromSeconds(1); I think it was better with kDefaultMinimumResizeIntervalMs constant. It makes it easy to find and tweak the timeout value if needed. Also mention next to the constant declaration that it must be kept in sync with ResizingHostObserverTest.RateLimited. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer_unittest.cc (right): https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:105: base::Bind(&AutoIncrementNow::Now, now_function_)); AutoIncrementNow::Now does not need to be in a separate ref-counted class. It can be method of ResizingHostObserverTest. ResizingHostObserverTest controls the lifetime of |resizing_host_observer_|. It also controls when SetScreenResolution() is called. So it is safe to use base::Unretained here. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:255: now_function->Increment(base::TimeDelta::FromMilliseconds(1)); nit: remove this line. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:258: // be processed within 1ms. Mention that |kDefaultMinimumResizeIntervalMs| must be kept in sync with this test. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:262: base::TimeDelta::FromMilliseconds(1)); nit: Make it 2ms
fyi https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer.cc (right): https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer.cc:127: base::TimeDelta minimum_resize_interval = base::TimeDelta::FromSeconds(1); On 2013/06/07 19:50:25, alexeypa wrote: > I think it was better with kDefaultMinimumResizeIntervalMs constant. It makes it > easy to find and tweak the timeout value if needed. Done (without the Default, since it can't be overridden any more). > Also mention next to the constant declaration that it must be kept in sync with > ResizingHostObserverTest.RateLimited. Done (although the consequences of failing to do so would be a unit test failure, so I'd hope it would be pretty obvious). https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... File remoting/host/resizing_host_observer_unittest.cc (right): https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:105: base::Bind(&AutoIncrementNow::Now, now_function_)); On 2013/06/07 19:50:25, alexeypa wrote: > AutoIncrementNow::Now does not need to be in a separate ref-counted class. It > can be method of ResizingHostObserverTest. ResizingHostObserverTest controls > the lifetime of |resizing_host_observer_|. It also controls when > SetScreenResolution() is called. So it is safe to use base::Unretained here. Done. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:255: now_function->Increment(base::TimeDelta::FromMilliseconds(1)); On 2013/06/07 19:50:25, alexeypa wrote: > nit: remove this line. It's needed, otherwise when the delayed resize happens, it will get the wrong time and think it needs to be delayed again. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:258: // be processed within 1ms. On 2013/06/07 19:50:25, alexeypa wrote: > Mention that |kDefaultMinimumResizeIntervalMs| must be kept in sync with this > test. Done. https://codereview.chromium.org/15927033/diff/75001/remoting/host/resizing_ho... remoting/host/resizing_host_observer_unittest.cc:262: base::TimeDelta::FromMilliseconds(1)); On 2013/06/07 19:50:25, alexeypa wrote: > nit: Make it 2ms Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/15927033/94001
The commit queue went berserk retrying too often for a seemingly flaky test on builder android_dbg: http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/15927033/94001
Failed to apply patch for remoting/host/me2me_desktop_environment.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file remoting/host/me2me_desktop_environment.h
Hunk #1 FAILED at 22.
1 out of 1 hunk FAILED -- saving rejects to file
remoting/host/me2me_desktop_environment.h.rej
Patch: remoting/host/me2me_desktop_environment.h
Index: remoting/host/me2me_desktop_environment.h
diff --git a/remoting/host/me2me_desktop_environment.h
b/remoting/host/me2me_desktop_environment.h
index
316d5c3fcfab5910740edc555e8e2847ba6846fc..3f3a2765d7b52ba83e8115a0a1e763b48e7b8451
100644
--- a/remoting/host/me2me_desktop_environment.h
+++ b/remoting/host/me2me_desktop_environment.h
@@ -22,6 +22,7 @@ class Me2MeDesktopEnvironment : public BasicDesktopEnvironment
{
// DesktopEnvironment interface.
virtual scoped_ptr<ScreenControls> CreateScreenControls() OVERRIDE;
virtual scoped_ptr<media::ScreenCapturer> CreateVideoCapturer() OVERRIDE;
+ virtual std::string GetCapabilities() const OVERRIDE;
protected:
friend class Me2MeDesktopEnvironmentFactory;
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/15927033/32002
Message was sent while issue was closed.
Change committed as 205093 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
