New issue
Advanced search Search tips

Issue 605794 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Replace safe-bool idiom with explicit operator bool() in base::WeakPtr.

Project Member Reported by w...@chromium.org, Apr 22 2016

Issue description

Now that explicit operator bool() is available, base::WeakPtr should use it rather than the old safe-bool idiom
 

Comment 1 by w...@chromium.org, May 4 2016

Blockedon: 607208
FYI scoped_refptr is  issue 610048 .
Project Member

Comment 3 by bugdroid1@chromium.org, May 26 2016

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

commit ca1070937cf760f039eae9392280b9e7c1601174
Author: wez <wez@chromium.org>
Date: Thu May 26 20:30:52 2016

Fix pointer checks to test-as-bool rather than null compares.

Various test fixtures have acquired assertions and expectations on pointers which try to compare explicitly with NULL or nullptr, rather than testing pointers as bools. These cases will break when WeakPtr is fixed to provide explicit operator bool().

Some bool-checks of other pointer containers (e.g. foo.get() == NULL) are also cleaned up.

BUG= 605794 

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

[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/spdy/spdy_session_pool_unittest.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/spdy/spdy_session_unittest.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/spdy/spdy_stream_unittest.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/net/spdy/spdy_test_util_common.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/remoting/protocol/fake_datagram_socket.cc
[modify] https://crrev.com/ca1070937cf760f039eae9392280b9e7c1601174/sync/internal_api/attachments/attachment_downloader_impl_unittest.cc

Comment 4 by w...@chromium.org, Jun 2 2016

Blockedon: -607208
Cc: thakis@chromium.org jbroman@chromium.org
Labels: OS-All
Removing blocked-on  issue 607208 , based on the current analysis in comment #11 on that bug.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 22 2016

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

commit b9820d493c42b7e3a4aebcd7956b016232600a1d
Author: wez <wez@chromium.org>
Date: Wed Jun 22 23:41:04 2016

Replace safe-bool idiom with explicit operator bool() in base::WeakPtr.

Also fixes various call-sites which were using WeakPtr as a bool
return-value, or comparing to NULL to test as a bool.

BUG= 605794 

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

[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/base/memory/weak_ptr.h
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/base/memory/weak_ptr_unittest.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/chrome/browser/ui/sync/one_click_signin_sync_starter.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/content/browser/renderer_host/media/video_capture_manager_unittest.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/content/renderer/media/pepper_to_video_track_adapter.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/device/bluetooth/bluetooth_adapter_factory.cc
[modify] https://crrev.com/b9820d493c42b7e3a4aebcd7956b016232600a1d/net/dns/host_resolver_impl.cc

Comment 6 by w...@chromium.org, Jun 24 2016

Status: Fixed (was: Started)

Sign in to add a comment