Skip to content

Commit

Permalink
cros: Fix crash when opening keyboard shortcut overlay from settings
Browse files Browse the repository at this point in the history
Crash was introduced in commit 8f1e606

More than one part of chrome calls mojo methods in NewWindowController,
so there can be more than one binding. I've audited the other changes
in the above CL and they should be OK, so keeping this change small
for easier backport.

Bug: 794581
Test: ash_unittests, chrome unit_tests and browser_tests, manually open keyboard shortcut overlay from webui settings > keyboard section
Change-Id: I6b274ecd7b8e6c0c366c36a398e9094b4c361dd9
Reviewed-on: https://chromium-review.googlesource.com/826026
Reviewed-by: Michael Wasserman <[email protected]>
Commit-Queue: James Cook <[email protected]>
Cr-Commit-Position: refs/heads/master@{#523969}
  • Loading branch information
James Cook authored and Commit Bot committed Dec 14, 2017
1 parent cab63f4 commit 3918d7b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
4 changes: 2 additions & 2 deletions ash/new_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

namespace ash {

NewWindowController::NewWindowController() : binding_(this) {}
NewWindowController::NewWindowController() = default;

NewWindowController::~NewWindowController() = default;

void NewWindowController::BindRequest(
mojom::NewWindowControllerRequest request) {
binding_.Bind(std::move(request));
bindings_.AddBinding(this, std::move(request));
}

void NewWindowController::SetClient(
Expand Down
6 changes: 4 additions & 2 deletions ash/new_window_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "ash/public/interfaces/new_window.mojom.h"
#include "base/macros.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/binding_set.h"

namespace ash {

Expand Down Expand Up @@ -38,7 +38,9 @@ class ASH_EXPORT NewWindowController : public mojom::NewWindowController {
void OpenFeedbackPage();

private:
mojo::Binding<mojom::NewWindowController> binding_;
// More than one part of chrome may connect to call the mojo methods, so use
// BindingSet instead of Binding. http://crbug.com/794581
mojo::BindingSet<mojom::NewWindowController> bindings_;

mojom::NewWindowClientAssociatedPtr client_;

Expand Down

0 comments on commit 3918d7b

Please sign in to comment.