New issue
Advanced search Search tips

Issue 872365 link

Starred by 1 user
Status: Fixed
Owner:
mukai@chromium.org
Closed: Aug 22
Cc:
sadrul@chromium.org
sky@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task

Blocking:
issue 847992



Sign in to add a comment

refactor GestureRecognizer initialization for mash

Project Member Reported by mukai@chromium.org, Aug 8 
So far the ui::GestureRecognizer is singleton, the instance is created for the first call of ui::GestureRecognizer::Get(). That leads to two instances on Mustash (since ash and browser will be in different processes). They'll need to synchronize with each other.

However, when both ash and browser are separated but reside within a single process, it comes back to a singleton instance, which is nice for the short term, but the synchronization problem goes invisible and can't be dealt with.

Maybe we may want to refactor the initialization of ui::GestureRecognizer, so that ash and browser will refer to different instances and let them communicate with each other, even when ash and browser runs in the same process.

Comment 1 by sky@chromium.org, Aug 8

Blocking: 847992

Comment 2 by mukai@chromium.org, Aug 14

Labels: -Proj-Mustash Proj-Mash-SingleProcess
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22 

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

commit 0f3ae52933d7a125432af4218fb1e355045d2620
Author: Jun Mukai <mukai@chromium.org>
Date: Wed Aug 22 16:28:11 2018

move gesture recognizer to aura::Env

As described in the bug, ui::GestureRecognizer shouldn't be
a singleton since Mash and browser process should refer to
different instances. ui::GestureRecognizer is now a property
of aura::Env.

views::Widget now also has GetGestureRecognizer() to support
MacViews since no aura::Env exists in Mac.

chrome/browser/ui/aura/accessibility changes are necessary
since this singleton object owns an aura::Window internally
(AutomationManagerAura owns AxTreeSourceAura which owns
AxRootObjWrapper which owns |alert_window_|) and deletion
of a Window now refers to its Env through
CleanupGestureState(). The change on AutomationManagerAura
makes sure that this instance does not outlive aura::Env
to prevent such references to deleted Env object implicitly.

Bug:  872365 
Test: trybots
Change-Id: Ia617482d5aa6fd5482df4dd8af0039395c5bc3fb
Reviewed-on: https://chromium-review.googlesource.com/1171838
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585047}
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ash/drag_drop/drag_drop_controller.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ash/login/ui/lock_window.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ash/wm/toplevel_window_event_handler.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/chrome/browser/ui/views/tabs/tab_drag_controller.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/chrome/browser/ui/views/touch_events_interactive_uitest_win.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/services/ui/ws2/topmost_window_observer.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/client/default_capture_client.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/env.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/env.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/gestures/gesture_recognizer_unittest.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/test/env_test_helper.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/window.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/window_event_dispatcher.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/aura/window_targeter.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/BUILD.gn
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/gestures/gesture_recognizer.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/gestures/gesture_recognizer_impl.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/gestures/gesture_recognizer_impl.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/gestures/gesture_recognizer_impl_mac.cc
[add] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/events/gestures/gesture_recognizer_impl_mac.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/controls/menu/menu_host.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/desktop_aura/desktop_capture_client.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/desktop_aura/desktop_native_widget_aura.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/native_widget_aura.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/native_widget_aura.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/native_widget_mac.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/native_widget_mac.mm
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/native_widget_private.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/widget.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/views/widget/widget.h
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/wm/core/capture_controller.cc
[modify] https://crrev.com/0f3ae52933d7a125432af4218fb1e355045d2620/ui/wm/core/window_modality_controller.cc

Comment 4 by mukai@chromium.org, Aug 22

Status: Fixed (was: Assigned)
►

Sign in to add a comment