Skip to content

Commit

Permalink
Revert "Listen to changes to touch input devices"
Browse files Browse the repository at this point in the history
This reverts commit 1b60006.

Reason for revert:
Causes a startup crash with chrome --mash, possibly because mash does not have a DeviceDataManager. You might want to check with sadrul@ or rjkroege@ about whether there's a good way to do this under mash.

Failing build (chromeos-side waterfall):
https://luci-milo.appspot.com/buildbot/chromeos.chrome/tricky-tot-chrome-pfq-informational/5205

Stack (from running manually with chrome --mash on Linux):

[60089:60089:0717/114019.863351:FATAL:device_data_manager.cc(84)] Check failed: instance_. DeviceDataManager was not created.
#0 0x7f06a03468bc base::debug::StackTrace::StackTrace()
#1 0x7f06a036a121 logging::LogMessage::~LogMessage()
#2 0x7f069aeb08ff ui::DeviceDataManager::GetInstance()
#3 0x55f183402a41 chromeos::LoginDisplayHostImpl::LoginDisplayHostImpl()
#4 0x55f1834066ad chromeos::ShowLoginWizard()
#5 0x55f1833e4bf7 chromeos::ChromeSessionManager::Initialize()
#6 0x55f1832f9cb4 chromeos::ChromeBrowserMainPartsChromeos::PostProfileInit()
#7 0x55f1837e7aa2 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#8 0x55f1837e709d ChromeBrowserMainParts::PreMainMessageLoopRun()
#9 0x55f1832f8f4a chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#10 0x7f069d9e5081 content::BrowserMainLoop::PreMainMessageLoopRun()
#11 0x7f069dde6407 content::StartupTaskRunner::RunAllTasksNow()
#12 0x7f069d9e353b content::BrowserMainLoop::CreateStartupTasks()
#13 0x7f069d9e7a72 content::BrowserMainRunnerImpl::Initialize()
#14 0x7f069d9e0b57 content::BrowserMain()
#15 0x7f069e1e8462 content::ContentMainRunnerImpl::Run()
#16 0x7f06a0890d39 service_manager::Main()
#17 0x7f069e1e7384 content::ContentMain()
#18 0x55f182e8132f ChromeMain
#19 0x7f069454af45 __libc_start_main
#20 0x55f182e81194 <unknown>


Original change's description:
> Listen to changes to touch input devices
> 
> In https://codereview.chromium.org/2964823002 the OobeDisplayChooser
> started using the DeviceDataManager to look for touchscreen devices when
> searching for a good primary display to use during OOBE.
> 
> On device cold boot the DeviceDataManager has not yet found any
> touchscreen devices at the time OobeUi::ShowOobeUI() is called (likely
> due to lower level systems not being fully initialized).
> 
> This CL make LoginDisplayHostImpl an observer of changes to connected
> touchscreen devices, re-triggering the OobeDisplayChooser when the
> DeviceDataManager is notified of the connected touchscreens. This
> overcomes the timing issues on cold boot.
> 
> Bug: 738885
> Change-Id: Iae488ddc9428b7c5e74d36cf18e35ba3d1235bbd
> Reviewed-on: https://chromium-review.googlesource.com/569958
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Commit-Queue: Felix Ekblom <felixe@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#487007}

TBR=alemate@chromium.org,jdufault@chromium.org,felixe@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 738885
Change-Id: If31322734e679bbb1f4eef0a9aa802d34263cba4
Reviewed-on: https://chromium-review.googlesource.com/574731
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487191}
  • Loading branch information
James Cook authored and Commit Bot committed Jul 17, 2017
1 parent d8f0cfa commit 0cd134a
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 16 deletions.
11 changes: 0 additions & 11 deletions chrome/browser/chromeos/login/ui/login_display_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
#include "ui/compositor/scoped_layer_animation_settings.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/events/devices/device_data_manager.h"
#include "ui/events/event_handler.h"
#include "ui/events/event_utils.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -385,8 +384,6 @@ LoginDisplayHostImpl::LoginDisplayHostImpl(const gfx::Rect& wallpaper_bounds)

display::Screen::GetScreen()->AddObserver(this);

ui::DeviceDataManager::GetInstance()->AddObserver(this);

// We need to listen to CLOSE_ALL_BROWSERS_REQUEST but not APP_TERMINATING
// because/ APP_TERMINATING will never be fired as long as this keeps
// ref-count. CLOSE_ALL_BROWSERS_REQUEST is safe here because there will be no
Expand Down Expand Up @@ -496,7 +493,6 @@ LoginDisplayHostImpl::~LoginDisplayHostImpl() {
DBusThreadManager::Get()->GetSessionManagerClient()->RemoveObserver(this);
CrasAudioHandler::Get()->RemoveAudioObserver(this);
display::Screen::GetScreen()->RemoveObserver(this);
ui::DeviceDataManager::GetInstance()->RemoveObserver(this);

if (login_view_ && login_window_)
login_window_->RemoveRemovalsObserver(this);
Expand Down Expand Up @@ -1021,13 +1017,6 @@ void LoginDisplayHostImpl::OnDisplayMetricsChanged(
}
}

////////////////////////////////////////////////////////////////////////////////
// LoginDisplayHostImpl, ui::InputDeviceEventObserver
void LoginDisplayHostImpl::OnTouchscreenDeviceConfigurationChanged() {
if (GetOobeUI())
GetOobeUI()->OnDisplayConfigurationChanged();
}

////////////////////////////////////////////////////////////////////////////////
// LoginDisplayHostImpl, views::WidgetRemovalsObserver:
void LoginDisplayHostImpl::OnWillRemoveView(views::Widget* widget,
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/chromeos/login/ui/login_display_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/web_contents_observer.h"
#include "ui/display/display_observer.h"
#include "ui/events/devices/input_device_event_observer.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/widget/widget_removals_observer.h"
#include "ui/wm/public/scoped_drag_drop_disabler.h"
Expand All @@ -52,7 +51,6 @@ class LoginDisplayHostImpl : public LoginDisplayHost,
public chromeos::SessionManagerClient::Observer,
public chromeos::CrasAudioHandler::AudioObserver,
public display::DisplayObserver,
public ui::InputDeviceEventObserver,
public views::WidgetRemovalsObserver,
public chrome::MultiUserWindowManager::Observer {
public:
Expand Down Expand Up @@ -124,9 +122,6 @@ class LoginDisplayHostImpl : public LoginDisplayHost,
void OnDisplayMetricsChanged(const display::Display& display,
uint32_t changed_metrics) override;

// Overridden from ui::InputDeviceEventObserver
void OnTouchscreenDeviceConfigurationChanged() override;

// Overriden from views::WidgetRemovalsObserver:
void OnWillRemoveView(views::Widget* widget, views::View* view) override;

Expand Down

0 comments on commit 0cd134a

Please sign in to comment.