New issue
Advanced search Search tips

Issue 817112 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 817118

Blocking:
issue 731255



Sign in to add a comment

interactive_ui_tests fail with --mus

Project Member Reported by sky@chromium.org, Feb 27 2018

Issue description

They end up injecting events, which doesn't quite work the same with --mus 
 

Comment 1 by sky@chromium.org, Feb 27 2018

Components: Internals>Services>WindowService

Comment 2 by sky@chromium.org, Feb 27 2018

Blockedon: 817118
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca28ade037d73f061558b6ed0167d08a587c0115

commit ca28ade037d73f061558b6ed0167d08a587c0115
Author: Scott Violet <sky@chromium.org>
Date: Fri Mar 02 17:18:00 2018

Nukes RunClosureAfterAllPendingUIEvents()

The reason I would like to nuke it is that it is mostly
unnecessary. In particular the code can supply a closure to the last
event generating routine instead, which should have the same effect.

This function was added as a work around. In particular the windows event
matching code was looking for l/r/mbuttondown (or up), but if the events
are close enough a double click is generated. So, added logic for that case.

BUG= 817112 
TEST=none

Cq-Include-Trybots: master.tryserver.chromium.mac:mac_chromium_10.10_macviews
Change-Id: I6f2937501eff6b9b85176d45ce14712bea4a2023
Reviewed-on: https://chromium-review.googlesource.com/940424
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540542}
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ash/test/ui_controls_factory_ash.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/chrome/browser/ui/views/toolbar/toolbar_view_interactive_uitest.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/aura/test/ui_controls_factory_aurawin.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/aura/test/ui_controls_factory_aurax11.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/aura/test/ui_controls_factory_ozone.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls.h
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls_aura.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls_aura.h
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls_internal_win.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls_mac.mm
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/base/test/ui_controls_win.cc
[modify] https://crrev.com/ca28ade037d73f061558b6ed0167d08a587c0115/ui/views/test/ui_controls_factory_desktop_aurax11.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8aa27d12119f5fb594cf469dbdf9235506992daf

commit 8aa27d12119f5fb594cf469dbdf9235506992daf
Author: Scott Violet <sky@chromium.org>
Date: Thu Mar 08 06:12:31 2018

aura: disable input throttling in tab dragging tests

Without this code thinks an event has been processed when it may not
have been, which leads to assertions failing. This is only an issue
with the Window Service, which triggers throttling more often.

BUG= 817112 
TEST=test only change

Change-Id: Ie290dcc827e4be67263f0430c540af271b16ab02
Reviewed-on: https://chromium-review.googlesource.com/954702
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541732}
[modify] https://crrev.com/8aa27d12119f5fb594cf469dbdf9235506992daf/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
[modify] https://crrev.com/8aa27d12119f5fb594cf469dbdf9235506992daf/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.h
[modify] https://crrev.com/8aa27d12119f5fb594cf469dbdf9235506992daf/ui/aura/env.cc
[modify] https://crrev.com/8aa27d12119f5fb594cf469dbdf9235506992daf/ui/aura/env.h

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a0319e8188786647bef32190980ae34210354d6

commit 9a0319e8188786647bef32190980ae34210354d6
Author: Scott Violet <sky@chromium.org>
Date: Thu Mar 08 19:04:25 2018

chromeos: adjust event location for capture

In interactive ui tests we generate events using
RemoteEventDispatcher. When there is capture we need to ensure the
location matches what happens on device, otherwise we won't find the
right target.

BUG= 817112 
TEST=covered by tests

Change-Id: Ic47b316058282db85d83b348f3b346f2132cd9a1
Reviewed-on: https://chromium-review.googlesource.com/955925
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541851}
[modify] https://crrev.com/9a0319e8188786647bef32190980ae34210354d6/services/ui/ws/remote_event_dispatcher.cc
[modify] https://crrev.com/9a0319e8188786647bef32190980ae34210354d6/services/ui/ws/remote_event_dispatcher.h
[modify] https://crrev.com/9a0319e8188786647bef32190980ae34210354d6/services/ui/ws/window_manager_state.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8c4c6869896e150b382a7b7d8020474e08921a9

commit c8c4c6869896e150b382a7b7d8020474e08921a9
Author: Scott Violet <sky@chromium.org>
Date: Mon Mar 12 23:25:34 2018

chromeos: provide ability to supply event for rewriting

EventRewriters don't honor the target, and so generally expect the
root_location and location to be the same. This patch changes EventSource::SendEventToSink()
to create a new Event if the existing has differing locations (and a target).

BUG= 817112 
TEST=covered by tests

Change-Id: Iaabe019fed4350610306576a054b7d395def92ff
Reviewed-on: https://chromium-review.googlesource.com/956925
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542653}
[modify] https://crrev.com/c8c4c6869896e150b382a7b7d8020474e08921a9/ui/events/event_source.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1286c724e3a2b07c93ebe7cc1c1b679229a699af

commit 1286c724e3a2b07c93ebe7cc1c1b679229a699af
Author: Trent Apted <tapted@chromium.org>
Date: Tue Mar 13 02:59:26 2018

Revert "chromeos: provide ability to supply event for rewriting"

This reverts commit c8c4c6869896e150b382a7b7d8020474e08921a9.

Reason for revert: Causes EventRewriterTest.EventRewriting to fail on Android/Linux CFI starting

https://uberchromegw.corp.google.com/i/chromium.memory/builders/Android%20CFI/builds/241

error like

../../ui/events/event.cc:306:10: runtime error: control flow integrity check for type 'ui::LocatedEvent' failed during base-to-derived cast (vtable address 0x0000002689b0)
0x0000002689b0: note: vtable is of type 'ui::(anonymous namespace)::TestEvent'
    #0 0x62afb5 in ui::Event::AsLocatedEvent() const ./../../ui/events/event.cc:306:10
    #1 0x63063a in ui::(anonymous namespace)::IsLocatedEventWithDifferentLocations(ui::Event const&) ./../../ui/events/event_source.cc:19:45
    #2 0x630083 in ui::EventSource::SendEventToSink(ui::Event*) ./../../ui/events/event_source.cc:46:34
    #3 0x4a23e2 in ui::(anonymous namespace)::TestEventRewriteSource::Send(ui::EventType) ./../../ui/events/event_rewriter_unittest.cc:69:5
    #4 0x4a1fe4 in ui::EventRewriterTest_EventRewriting_Test::TestBody() ./../../ui/events/event_rewriter_unittest.cc:188:5


Original change's description:
> chromeos: provide ability to supply event for rewriting
> 
> EventRewriters don't honor the target, and so generally expect the
> root_location and location to be the same. This patch changes EventSource::SendEventToSink()
> to create a new Event if the existing has differing locations (and a target).
> 
> BUG= 817112 
> TEST=covered by tests
> 
> Change-Id: Iaabe019fed4350610306576a054b7d395def92ff
> Reviewed-on: https://chromium-review.googlesource.com/956925
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#542653}

TBR=sadrul@chromium.org,sky@chromium.org

Change-Id: I5ee070c74eba97a446c1a8e71e12f64b90b27401
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  817112 
Reviewed-on: https://chromium-review.googlesource.com/958806
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542705}
[modify] https://crrev.com/1286c724e3a2b07c93ebe7cc1c1b679229a699af/ui/events/event_source.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a5ea6086a54429ad2770cf28a3898413866e1bdd

commit a5ea6086a54429ad2770cf28a3898413866e1bdd
Author: Scott Violet <sky@chromium.org>
Date: Tue Mar 13 15:32:14 2018

reland: chromeos: provide ability to supply event for rewriting

EventRewriters don't honor the target, and so generally expect the
root_location and location to be the same. This patch changes EventSource::SendEventToSink()
to create a new Event if the existing has differing locations (and a target).

BUG= 817112 
TEST=covered by tests

Change-Id: I29f9cafe962194938c8e457aabc3a5ce6b379649
Reviewed-on: https://chromium-review.googlesource.com/959803
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542809}
[modify] https://crrev.com/a5ea6086a54429ad2770cf28a3898413866e1bdd/ui/events/event_rewriter_unittest.cc
[modify] https://crrev.com/a5ea6086a54429ad2770cf28a3898413866e1bdd/ui/events/event_source.cc

Comment 9 by sky@chromium.org, Mar 13 2018

Status: Fixed (was: Started)

Comment 10 by sky@chromium.org, Mar 13 2018

Issue 814065 has been merged into this issue.

Sign in to add a comment