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

Issue 638395 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

OfflinePageDownloadBridge memory leak

Project Member Reported by dfalcant...@chromium.org, Aug 16 2016

Issue description

There's a memory leak when Offline Pages is told to download the current page via the menu icon because the bridge doesn't have destroy() called on it.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 20 2016

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

commit a6e1def19ac161b674482e96e7d346dcfca2a92d
Author: dimich <dimich@chromium.org>
Date: Sat Aug 20 03:54:40 2016

Add 'title' field to DownloadUIItem and thread through bridges to OfflinePageDownloadItem.
Also call destroy() on the bridge to fix mem leak

BUG= 638395 , 630817 

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

[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridge.java
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadItem.java
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageDownloadBridgeTest.java
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/components/offline_pages/downloads/download_ui_adapter_unittest.cc
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/components/offline_pages/downloads/download_ui_item.cc
[modify] https://crrev.com/a6e1def19ac161b674482e96e7d346dcfca2a92d/components/offline_pages/downloads/download_ui_item.h

Comment 2 by dim...@chromium.org, Aug 22 2016

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 23 2016

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

commit a286301f5467365276284917019525a663cc0da8
Author: fgorski <fgorski@chromium.org>
Date: Tue Aug 23 23:24:23 2016

[Offline pages] Making sure that destroying the bridge does not leave notification hanging

* Preventing memory leak might invalidate the pointer to the bridge on
  C++ side, which is used to complete save page operation and to update
  the notification to failed or succeeded.
* This patch makes sure that callback is invoked as a static function
  and is independent of weak ptr.

BUG= 638395 
R=dimich

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

[modify] https://crrev.com/a286301f5467365276284917019525a663cc0da8/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/a286301f5467365276284917019525a663cc0da8/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.h

Sign in to add a comment