diff options
author | rharrison@chromium.org <rharrison@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-09-11 13:23:31 +0000 |
---|---|---|
committer | rharrison@chromium.org <rharrison@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-09-11 13:23:31 +0000 |
commit | 960058f702addf77db38368967747d26ab9dc075 (patch) | |
tree | a3d3727ceb86144c7b43a21a02d4abdb9696f966 /chromeos | |
parent | 597fdd82ea8c6c2ccb1ba5c2d24838cf4e3eb9de (diff) | |
download | chromium_src-960058f702addf77db38368967747d26ab9dc075.zip chromium_src-960058f702addf77db38368967747d26ab9dc075.tar.gz chromium_src-960058f702addf77db38368967747d26ab9dc075.tar.bz2 |
Add is_fullscreen to video updates
This is a re-attempt at http://codereview.chromium.org/10916123/, which also ended up being reverted.
This was reverted due to a failing unit test on Win_Aura. Through an unfortunate set of mistakes this builder was not in the default try set or the CQ set, but could close the tree leading to the revert. Through testing I have been able to reproduce the failure on linux_chromeos, so it might be a flakey failure, since it passed sometimes on that bot. This CL is basically the same as the CL list above, except that there is now a window->Focus() call in the unit test, since the reason the test was failing was when it was being made full screen it ceased to visible. This fixes the issue within this test. I will added a comment about this on the bug tracking the underlying issue.
This is a re-attempt at http://codereview.chromium.org/10905026/, which ended up being reverted.
The crash that caused the revert was actually due to powerd not being able to parse the protobuf correctly. https://gerrit.chromium.org/gerrit/32243/ corrects this issue, so once that CL lands this CL is good to go. I confirmed the combination of CLs works correctly.
This is the original CL description:
This adds a boolean to the video update message sent to powerd that indicates whether or not the video playing window is fullscreen'd. This is information used in powerd for controlling when to enable/disable keyboard backlights. Specifically if video is playing and fullscreen we assume that the user is doing something like watching a movie so will disable backlight.
An additional change that I have made is to convert the message being sent from an int64 to a protobuffer that contains a int64 and a boolean. Currently, since the receiver of this message does not live in the same repo it means that every time one wants to change the message format there is a small window where bad builds can be generated or one has to land two changes to the other repo, one to add handling of the new message format and then a second one to remove the old format. Converting to a protobuffer means that for common cases this issues are removed.
This CL depends on CLs for power_manager (https://gerrit.chromium.org/gerrit/32092/), one for system_api (https://gerrit.chromium.org/gerrit/31916), and a DEPS roll(http://codereview.chromium.org/10915032/). There will be future CLs for power_manager that will use this information and more thourghly tests this code.
BUG=chrome-os-partner:9203
TEST=Run updated unittests.
Confirm there is no messages in the cros logs about this method being mishandled.
Make sure that powerd is not crashing due to protocol buffer processing failing.
Review URL: https://chromiumcodereview.appspot.com/10913134
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@156008 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chromeos')
-rw-r--r-- | chromeos/chromeos.gyp | 15 | ||||
-rw-r--r-- | chromeos/dbus/mock_power_manager_client.h | 2 | ||||
-rw-r--r-- | chromeos/dbus/power_manager_client.cc | 14 | ||||
-rw-r--r-- | chromeos/dbus/power_manager_client.h | 6 |
4 files changed, 31 insertions, 6 deletions
diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp index fdf1958..3a1b5f6 100644 --- a/chromeos/chromeos.gyp +++ b/chromeos/chromeos.gyp @@ -18,6 +18,7 @@ '../third_party/libxml/libxml.gyp:libxml', 'power_state_control_proto', 'power_supply_properties_proto', + 'video_activity_update_proto', ], 'defines': [ 'CHROMEOS_IMPLEMENTATION', @@ -296,5 +297,19 @@ }, 'includes': ['../build/protoc.gypi'], }, + { + # Protobuf compiler / generator for the VideoActivityUpdate protocol + # buffer. + 'target_name': 'video_activity_update_proto', + 'type': 'static_library', + 'sources': [ + '../third_party/cros_system_api/dbus/video_activity_update.proto', + ], + 'variables': { + 'proto_in_dir': '../third_party/cros_system_api/dbus/', + 'proto_out_dir': 'chromeos/dbus', + }, + 'includes': ['../build/protoc.gypi'], + }, ], } diff --git a/chromeos/dbus/mock_power_manager_client.h b/chromeos/dbus/mock_power_manager_client.h index ae828af..6d5ee18 100644 --- a/chromeos/dbus/mock_power_manager_client.h +++ b/chromeos/dbus/mock_power_manager_client.h @@ -34,7 +34,7 @@ class MockPowerManagerClient : public PowerManagerClient { MOCK_METHOD1(RequestIdleNotification, void(int64)); MOCK_METHOD0(RequestActiveNotification, void(void)); MOCK_METHOD1(NotifyUserActivity, void(const base::TimeTicks&)); - MOCK_METHOD1(NotifyVideoActivity, void(const base::TimeTicks&)); + MOCK_METHOD2(NotifyVideoActivity, void(const base::TimeTicks&, bool)); MOCK_METHOD4(RequestPowerStateOverrides, void(uint32, uint32, diff --git a/chromeos/dbus/power_manager_client.cc b/chromeos/dbus/power_manager_client.cc index 6b8c4af..fe1fd36 100644 --- a/chromeos/dbus/power_manager_client.cc +++ b/chromeos/dbus/power_manager_client.cc @@ -16,6 +16,7 @@ #include "base/timer.h" #include "chromeos/dbus/power_state_control.pb.h" #include "chromeos/dbus/power_supply_properties.pb.h" +#include "chromeos/dbus/video_activity_update.pb.h" #include "dbus/bus.h" #include "dbus/message.h" #include "dbus/object_path.h" @@ -225,12 +226,18 @@ class PowerManagerClientImpl : public PowerManagerClient { } virtual void NotifyVideoActivity( - const base::TimeTicks& last_activity_time) OVERRIDE { + const base::TimeTicks& last_activity_time, + bool is_fullscreen) OVERRIDE { dbus::MethodCall method_call( power_manager::kPowerManagerInterface, power_manager::kHandleVideoActivityMethod); dbus::MessageWriter writer(&method_call); - writer.AppendInt64(last_activity_time.ToInternalValue()); + + VideoActivityUpdate protobuf; + protobuf.set_last_activity_time(last_activity_time.ToInternalValue()); + protobuf.set_is_fullscreen(is_fullscreen); + + writer.AppendProtoAsArrayOfBytes(protobuf); power_manager_proxy_->CallMethod( &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, @@ -593,7 +600,8 @@ class PowerManagerClientStubImpl : public PowerManagerClient { virtual void NotifyUserActivity( const base::TimeTicks& last_activity_time) OVERRIDE {} virtual void NotifyVideoActivity( - const base::TimeTicks& last_activity_time) OVERRIDE {} + const base::TimeTicks& last_activity_time, + bool is_fullscreen) OVERRIDE {} virtual void RequestPowerStateOverrides( uint32 request_id, uint32 duration, diff --git a/chromeos/dbus/power_manager_client.h b/chromeos/dbus/power_manager_client.h index e910232..5be8298 100644 --- a/chromeos/dbus/power_manager_client.h +++ b/chromeos/dbus/power_manager_client.h @@ -156,9 +156,11 @@ class CHROMEOS_EXPORT PowerManagerClient { virtual void NotifyUserActivity( const base::TimeTicks& last_activity_time) = 0; - // Notifies the power manager that a video is currently playing. + // Notifies the power manager that a video is currently playing. It also + // includes whether or not the containing window for the video is fullscreen. virtual void NotifyVideoActivity( - const base::TimeTicks& last_activity_time) = 0; + const base::TimeTicks& last_activity_time, + bool is_fullscreen) = 0; // Override the current power state on the machine. The overrides will be // applied to the request ID specified. To specify a new request; use 0 as |