Commit ea403cb7 authored by wutao's avatar wutao Committed by Commit Bot

Add time stamp to ui::Accelerator.

We want to measure the latency between a key event to the time of the
presentation of next UI frame. However, we do not save the time stamp in
ui::Accelerator.

This cl saves the time stamp information when ui::Accelerator is created
from KeyEvent.

Bug: 817116
Test: AcceleratorTest and AcceleratorStructTraitsTest
Change-Id: If4812a491e16371cf32adae8d7074232dc3c05d8
Reviewed-on: https://chromium-review.googlesource.com/956526Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542164}
parent e837a5d2
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/accelerator.h"
#include <stdint.h> #include <stdint.h>
#include <tuple>
#include "base/i18n/rtl.h" #include "base/i18n/rtl.h"
#include "base/logging.h" #include "base/logging.h"
...@@ -39,10 +40,12 @@ Accelerator::Accelerator() : Accelerator(VKEY_UNKNOWN, EF_NONE) {} ...@@ -39,10 +40,12 @@ Accelerator::Accelerator() : Accelerator(VKEY_UNKNOWN, EF_NONE) {}
Accelerator::Accelerator(KeyboardCode key_code, Accelerator::Accelerator(KeyboardCode key_code,
int modifiers, int modifiers,
KeyState key_state) KeyState key_state,
base::TimeTicks time_stamp)
: key_code_(key_code), : key_code_(key_code),
key_state_(key_state), key_state_(key_state),
modifiers_(modifiers & kInterestingFlagsMask), modifiers_(modifiers & kInterestingFlagsMask),
time_stamp_(time_stamp),
interrupted_by_mouse_event_(false) {} interrupted_by_mouse_event_(false) {}
Accelerator::Accelerator(const KeyEvent& key_event) Accelerator::Accelerator(const KeyEvent& key_event)
...@@ -51,12 +54,14 @@ Accelerator::Accelerator(const KeyEvent& key_event) ...@@ -51,12 +54,14 @@ Accelerator::Accelerator(const KeyEvent& key_event)
: KeyState::RELEASED), : KeyState::RELEASED),
// |modifiers_| may include the repeat flag. // |modifiers_| may include the repeat flag.
modifiers_(key_event.flags() & kInterestingFlagsMask), modifiers_(key_event.flags() & kInterestingFlagsMask),
time_stamp_(key_event.time_stamp()),
interrupted_by_mouse_event_(false) {} interrupted_by_mouse_event_(false) {}
Accelerator::Accelerator(const Accelerator& accelerator) { Accelerator::Accelerator(const Accelerator& accelerator) {
key_code_ = accelerator.key_code_; key_code_ = accelerator.key_code_;
key_state_ = accelerator.key_state_; key_state_ = accelerator.key_state_;
modifiers_ = accelerator.modifiers_; modifiers_ = accelerator.modifiers_;
time_stamp_ = accelerator.time_stamp_;
interrupted_by_mouse_event_ = accelerator.interrupted_by_mouse_event_; interrupted_by_mouse_event_ = accelerator.interrupted_by_mouse_event_;
if (accelerator.platform_accelerator_) if (accelerator.platform_accelerator_)
platform_accelerator_ = accelerator.platform_accelerator_->CreateCopy(); platform_accelerator_ = accelerator.platform_accelerator_->CreateCopy();
...@@ -74,7 +79,7 @@ KeyEvent Accelerator::ToKeyEvent() const { ...@@ -74,7 +79,7 @@ KeyEvent Accelerator::ToKeyEvent() const {
return KeyEvent(key_state() == Accelerator::KeyState::PRESSED return KeyEvent(key_state() == Accelerator::KeyState::PRESSED
? ET_KEY_PRESSED ? ET_KEY_PRESSED
: ET_KEY_RELEASED, : ET_KEY_RELEASED,
key_code(), modifiers()); key_code(), modifiers(), time_stamp());
} }
Accelerator& Accelerator::operator=(const Accelerator& accelerator) { Accelerator& Accelerator::operator=(const Accelerator& accelerator) {
...@@ -82,6 +87,7 @@ Accelerator& Accelerator::operator=(const Accelerator& accelerator) { ...@@ -82,6 +87,7 @@ Accelerator& Accelerator::operator=(const Accelerator& accelerator) {
key_code_ = accelerator.key_code_; key_code_ = accelerator.key_code_;
key_state_ = accelerator.key_state_; key_state_ = accelerator.key_state_;
modifiers_ = accelerator.modifiers_; modifiers_ = accelerator.modifiers_;
time_stamp_ = accelerator.time_stamp_;
interrupted_by_mouse_event_ = accelerator.interrupted_by_mouse_event_; interrupted_by_mouse_event_ = accelerator.interrupted_by_mouse_event_;
if (accelerator.platform_accelerator_) if (accelerator.platform_accelerator_)
platform_accelerator_ = accelerator.platform_accelerator_->CreateCopy(); platform_accelerator_ = accelerator.platform_accelerator_->CreateCopy();
...@@ -92,14 +98,10 @@ Accelerator& Accelerator::operator=(const Accelerator& accelerator) { ...@@ -92,14 +98,10 @@ Accelerator& Accelerator::operator=(const Accelerator& accelerator) {
} }
bool Accelerator::operator <(const Accelerator& rhs) const { bool Accelerator::operator <(const Accelerator& rhs) const {
if (key_code_ != rhs.key_code_) const int modifiers_with_mask = MaskOutKeyEventFlags(modifiers_);
return key_code_ < rhs.key_code_; const int rhs_modifiers_with_mask = MaskOutKeyEventFlags(rhs.modifiers_);
if (key_state_ != rhs.key_state_) { return std::tie(key_code_, key_state_, modifiers_with_mask) <
return static_cast<int32_t>(key_state_) < std::tie(rhs.key_code_, rhs.key_state_, rhs_modifiers_with_mask);
static_cast<int32_t>(rhs.key_state_);
}
return MaskOutKeyEventFlags(modifiers_) <
MaskOutKeyEventFlags(rhs.modifiers_);
} }
bool Accelerator::operator ==(const Accelerator& rhs) const { bool Accelerator::operator ==(const Accelerator& rhs) const {
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include <utility> #include <utility>
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/time/time.h"
#include "ui/base/accelerators/platform_accelerator.h" #include "ui/base/accelerators/platform_accelerator.h"
#include "ui/base/ui_base_export.h" #include "ui/base/ui_base_export.h"
#include "ui/events/event_constants.h" #include "ui/events/event_constants.h"
...@@ -44,7 +45,8 @@ class UI_BASE_EXPORT Accelerator { ...@@ -44,7 +45,8 @@ class UI_BASE_EXPORT Accelerator {
// NOTE: this constructor strips out non key related flags. // NOTE: this constructor strips out non key related flags.
Accelerator(KeyboardCode key_code, Accelerator(KeyboardCode key_code,
int modifiers, int modifiers,
KeyState key_state = KeyState::PRESSED); KeyState key_state = KeyState::PRESSED,
base::TimeTicks time_stamp = base::TimeTicks());
explicit Accelerator(const KeyEvent& key_event); explicit Accelerator(const KeyEvent& key_event);
Accelerator(const Accelerator& accelerator); Accelerator(const Accelerator& accelerator);
~Accelerator(); ~Accelerator();
...@@ -73,6 +75,8 @@ class UI_BASE_EXPORT Accelerator { ...@@ -73,6 +75,8 @@ class UI_BASE_EXPORT Accelerator {
int modifiers() const { return modifiers_; } int modifiers() const { return modifiers_; }
base::TimeTicks time_stamp() const { return time_stamp_; }
bool IsShiftDown() const; bool IsShiftDown() const;
bool IsCtrlDown() const; bool IsCtrlDown() const;
bool IsAltDown() const; bool IsAltDown() const;
...@@ -108,6 +112,9 @@ class UI_BASE_EXPORT Accelerator { ...@@ -108,6 +112,9 @@ class UI_BASE_EXPORT Accelerator {
// The state of the Shift/Ctrl/Alt keys. This corresponds to Event::flags(). // The state of the Shift/Ctrl/Alt keys. This corresponds to Event::flags().
int modifiers_; int modifiers_;
// The |time_stamp_| of the KeyEvent.
base::TimeTicks time_stamp_;
// Stores platform specific data. May be NULL. // Stores platform specific data. May be NULL.
// TODO: this is only used in Mac code and should be removed from here. // TODO: this is only used in Mac code and should be removed from here.
// http://crbug.com/702823. // http://crbug.com/702823.
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/accelerator.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/events/event.h"
namespace ui { namespace ui {
...@@ -19,4 +20,16 @@ TEST(AcceleratorTest, Repeat) { ...@@ -19,4 +20,16 @@ TEST(AcceleratorTest, Repeat) {
EXPECT_TRUE(accelerator_b_copy.IsRepeat()); EXPECT_TRUE(accelerator_b_copy.IsRepeat());
} }
TEST(AcceleratorTest, TimeStamp) {
const Accelerator accelerator_a(VKEY_A, EF_NONE);
EXPECT_EQ(base::TimeTicks(), accelerator_a.time_stamp());
const base::TimeTicks event_time =
base::TimeTicks() + base::TimeDelta::FromMilliseconds(1);
KeyEvent keyevent(ET_KEY_PRESSED, VKEY_SPACE, EF_NONE, event_time);
const Accelerator accelerator_b(keyevent);
EXPECT_EQ(event_time, accelerator_b.time_stamp());
}
} // namespace ui } // namespace ui
...@@ -9,6 +9,7 @@ mojom("interfaces") { ...@@ -9,6 +9,7 @@ mojom("interfaces") {
"accelerator.mojom", "accelerator.mojom",
] ]
public_deps = [ public_deps = [
"//mojo/common:common_custom_types",
"//ui/events/mojo:interfaces", "//ui/events/mojo:interfaces",
] ]
} }
......
include_rules = [
"+mojo/common",
]
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
module ui.mojom; module ui.mojom;
import "mojo/common/time.mojom";
import "ui/events/mojo/event_constants.mojom"; import "ui/events/mojo/event_constants.mojom";
import "ui/events/mojo/keyboard_codes.mojom"; import "ui/events/mojo/keyboard_codes.mojom";
...@@ -20,4 +21,5 @@ struct Accelerator { ...@@ -20,4 +21,5 @@ struct Accelerator {
int32 key_code; int32 key_code;
AcceleratorKeyState key_state; AcceleratorKeyState key_state;
int32 modifiers; int32 modifiers;
mojo.common.mojom.TimeTicks time_stamp;
}; };
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef UI_BASE_ACCELERATORS_MOJO_ACCELERATOR_STRUCT_TRAITS_H_ #ifndef UI_BASE_ACCELERATORS_MOJO_ACCELERATOR_STRUCT_TRAITS_H_
#define UI_BASE_ACCELERATORS_MOJO_ACCELERATOR_STRUCT_TRAITS_H_ #define UI_BASE_ACCELERATORS_MOJO_ACCELERATOR_STRUCT_TRAITS_H_
#include "mojo/common/time_struct_traits.h"
#include "ui/base/accelerators/accelerator.h" #include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/mojo/accelerator.mojom.h" #include "ui/base/accelerators/mojo/accelerator.mojom.h"
#include "ui/events/keycodes/keyboard_codes.h" #include "ui/events/keycodes/keyboard_codes.h"
...@@ -49,13 +50,18 @@ struct StructTraits<ui::mojom::AcceleratorDataView, ui::Accelerator> { ...@@ -49,13 +50,18 @@ struct StructTraits<ui::mojom::AcceleratorDataView, ui::Accelerator> {
return p.key_state(); return p.key_state();
} }
static int32_t modifiers(const ui::Accelerator& p) { return p.modifiers(); } static int32_t modifiers(const ui::Accelerator& p) { return p.modifiers(); }
static base::TimeTicks time_stamp(const ui::Accelerator& p) {
return p.time_stamp();
}
static bool Read(ui::mojom::AcceleratorDataView data, ui::Accelerator* out) { static bool Read(ui::mojom::AcceleratorDataView data, ui::Accelerator* out) {
ui::Accelerator::KeyState key_state; ui::Accelerator::KeyState key_state;
if (!data.ReadKeyState(&key_state)) if (!data.ReadKeyState(&key_state))
return false; return false;
base::TimeTicks time_stamp;
if (!data.ReadTimeStamp(&time_stamp))
return false;
*out = ui::Accelerator(static_cast<ui::KeyboardCode>(data.key_code()), *out = ui::Accelerator(static_cast<ui::KeyboardCode>(data.key_code()),
data.modifiers()); data.modifiers(), key_state, time_stamp);
out->set_key_state(key_state);
return true; return true;
} }
}; };
......
...@@ -12,8 +12,10 @@ ...@@ -12,8 +12,10 @@
namespace ui { namespace ui {
TEST(AcceleratorStructTraitsTest, SerializeAndDeserialize1) { TEST(AcceleratorStructTraitsTest, SerializeAndDeserialize1) {
Accelerator accelerator(KeyboardCode::VKEY_TAB, EF_NUM_LOCK_ON); Accelerator accelerator(
accelerator.set_key_state(ui::Accelerator::KeyState::RELEASED); KeyboardCode::VKEY_TAB, EF_NUM_LOCK_ON,
ui::Accelerator::KeyState::RELEASED,
base::TimeTicks() + base::TimeDelta::FromMilliseconds(1));
Accelerator deserialized; Accelerator deserialized;
ASSERT_TRUE(mojom::Accelerator::Deserialize( ASSERT_TRUE(mojom::Accelerator::Deserialize(
mojom::Accelerator::Serialize(&accelerator), &deserialized)); mojom::Accelerator::Serialize(&accelerator), &deserialized));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment