New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 618488 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

mojo::Binding::Close() should do nothing if not bound

Project Member Reported by sky@chromium.org, Jun 8 2016

Issue description

Currently it DCHECKs. My argument for not DCHECKing is that Close() is similar std::unique_ptr::reset(), which does not DCHECK if already null. In fact we should consider renaming Close() to Reset().
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 10 2016

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

commit 3e75809d6084ec6ae9694cc5ba20990be37ec49a
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 10 00:49:23 2016

Mojo C++ bindings: don't DCEHCK in Binding::Close() when the binding is not bound.

BUG= 618488 

Review-Url: https://codereview.chromium.org/2052963002
Cr-Commit-Position: refs/heads/master@{#399059}

[modify] https://crrev.com/3e75809d6084ec6ae9694cc5ba20990be37ec49a/mojo/public/cpp/bindings/lib/binding_state.h

Comment 2 by yzshen@chromium.org, Jun 10 2016

Status: Fixed (was: Untriaged)
WRT Close/Reset, the two seem similar to me. I am fine leaving it as it is. But please let me know if you feel strongly about it.

Thanks!
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 15 2016

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

commit 3e75809d6084ec6ae9694cc5ba20990be37ec49a
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 10 00:49:23 2016

Mojo C++ bindings: don't DCEHCK in Binding::Close() when the binding is not bound.

BUG= 618488 

Review-Url: https://codereview.chromium.org/2052963002
Cr-Commit-Position: refs/heads/master@{#399059}

[modify] https://crrev.com/3e75809d6084ec6ae9694cc5ba20990be37ec49a/mojo/public/cpp/bindings/lib/binding_state.h

Sign in to add a comment