New issue
Advanced search Search tips

Issue 800404 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

DownloadProtectionServiceTest.VerifyReferrerChainWithEmptyNavigationHistory leaks |web_contents|

Project Member Reported by eladalon@chromium.org, Jan 9 2018

Issue description

|web_contents| is allocated using the new-operator, but is never delete-d.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 11 2018

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

commit 0823211ed4a4abbd26ecb7df1408fe010ecb3412
Author: Elad Alon <eladalon@chromium.org>
Date: Thu Jan 11 11:48:29 2018

Move ownership of WebRTCInternals to BrowserMainLoop

WebRTCInternals was a lazily-constructed singleton. This CL changes
this, to allow a similar change in WebRtcEventLogManager, where this
lazy construction (and subsequent leaking) created problems for
unit tests due to WebRtcEventLogManager's task runner. (The change
to WebRtcEventLogManager will be done in a separate, subsequent CL.)

Piggy-backed on this - plug the leak in DownloadProtectionServiceTest.VerifyReferrerChainWithEmptyNavigationHistory

Bug: 775415,  800404 ,  792847 
Change-Id: Iddc2569933b83bbe3a4e16c161ac531748448477
Reviewed-on: https://chromium-review.googlesource.com/857470
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528597}
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/browser_main_loop.cc
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/browser_main_loop.h
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/0823211ed4a4abbd26ecb7df1408fe010ecb3412/content/browser/webrtc/webrtc_internals_message_handler.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 11 2018

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

commit ce383b564cd0ea6510551c77c81e13422d78f069
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Jan 11 12:47:26 2018

Revert "Move ownership of WebRTCInternals to BrowserMainLoop"

This reverts commit 0823211ed4a4abbd26ecb7df1408fe010ecb3412.

Reason for revert: Compile on Cast Audio Linux broke
https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/7958

Log message:
error: undefined symbol: content::WebRTCInternals::CreateSingletonInstance()
>>> referenced by browser_main_loop.cc:1572 (../../content/browser/browser_main_loop.cc:1572)

Original change's description:
> Move ownership of WebRTCInternals to BrowserMainLoop
> 
> WebRTCInternals was a lazily-constructed singleton. This CL changes
> this, to allow a similar change in WebRtcEventLogManager, where this
> lazy construction (and subsequent leaking) created problems for
> unit tests due to WebRtcEventLogManager's task runner. (The change
> to WebRtcEventLogManager will be done in a separate, subsequent CL.)
> 
> Piggy-backed on this - plug the leak in DownloadProtectionServiceTest.VerifyReferrerChainWithEmptyNavigationHistory
> 
> Bug: 775415,  800404 ,  792847 
> Change-Id: Iddc2569933b83bbe3a4e16c161ac531748448477
> Reviewed-on: https://chromium-review.googlesource.com/857470
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Elad Alon <eladalon@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528597}

TBR=jam@chromium.org,terelius@chromium.org,eladalon@chromium.org

Change-Id: Ifdeac7829d2d0e940ed938555793f38410d30272
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 775415,  800404 ,  792847 
Reviewed-on: https://chromium-review.googlesource.com/860062
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528608}
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/browser_main_loop.cc
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/browser_main_loop.h
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/ce383b564cd0ea6510551c77c81e13422d78f069/content/browser/webrtc/webrtc_internals_message_handler.cc

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 13 2018

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

commit a619d5567e6e32af78321ea591cbfbd9a7ab2bc8
Author: Elad Alon <eladalon@chromium.org>
Date: Sat Jan 13 01:30:15 2018

Revert "Revert "Move ownership of WebRTCInternals to BrowserMainLoop""

This reverts commit ce383b564cd0ea6510551c77c81e13422d78f069.

Reason for revert: Fix the build-break introduced by the original CL and reland.

Original change's description:
> Revert "Move ownership of WebRTCInternals to BrowserMainLoop"
>
> This reverts commit 0823211ed4a4abbd26ecb7df1408fe010ecb3412.
>
> Reason for revert: Compile on Cast Audio Linux broke
> https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/7958
>
> Log message:
> error: undefined symbol: content::WebRTCInternals::CreateSingletonInstance()
> >>> referenced by browser_main_loop.cc:1572 (../../content/browser/browser_main_loop.cc:1572)
>
> Original change's description:
> > Move ownership of WebRTCInternals to BrowserMainLoop
> >
> > WebRTCInternals was a lazily-constructed singleton. This CL changes
> > this, to allow a similar change in WebRtcEventLogManager, where this
> > lazy construction (and subsequent leaking) created problems for
> > unit tests due to WebRtcEventLogManager's task runner. (The change
> > to WebRtcEventLogManager will be done in a separate, subsequent CL.)
> >
> > Piggy-backed on this - plug the leak in DownloadProtectionServiceTest.VerifyReferrerChainWithEmptyNavigationHistory
> >
> > Bug: 775415,  800404 ,  792847 
> > Change-Id: Iddc2569933b83bbe3a4e16c161ac531748448477
> > Reviewed-on: https://chromium-review.googlesource.com/857470
> > Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> > Commit-Queue: Elad Alon <eladalon@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#528597}
>
> TBR=jam@chromium.org,terelius@chromium.org,eladalon@chromium.org
>
> Change-Id: Ifdeac7829d2d0e940ed938555793f38410d30272
> Bug: 775415,  800404 ,  792847 
> Reviewed-on: https://chromium-review.googlesource.com/860062
> Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
> Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528608}

TBR=jam@chromium.org,terelius@chromium.org,fhorschig@chromium.org,eladalon@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:cast_shell_audio_linux,fuchsia_arm64_cast_audio,fuchsia_x64_cast_audio
No-Try: true

Bug: 775415,  800404 ,  792847 
Change-Id: I28f34f417cfde490710a82a3e29711a0f1e9b15c
Reviewed-on: https://chromium-review.googlesource.com/860404
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529135}
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/browser_main_loop.cc
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/browser_main_loop.h
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/renderer_host/media/peer_connection_tracker_host.cc
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/webrtc/webrtc_internals.cc
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/webrtc/webrtc_internals.h
[modify] https://crrev.com/a619d5567e6e32af78321ea591cbfbd9a7ab2bc8/content/browser/webrtc/webrtc_internals_message_handler.cc

Sign in to add a comment