New issue
Advanced search Search tips

Issue 900280 link

Starred by 1 user
Status: Fixed
Owner:
qin...@chromium.org
Closed: Nov 1
Cc:
qin...@chromium.org
hnakashima@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 809322



Sign in to add a comment

Resume download pending after relaunching Chrome.

Project Member Reported by hanxi@chromium.org, Oct 30 
I build a debug build of Chrome, and hit this crash repeatedly
.
Steps to reproduce:
1. Open Clank.
2. Navigate to https://www.thinkbroadband.com/download and start a download.
3. Pause the notification.
4. Swipe Chrome from recents. Clicks Chrome's icon and swipe it again, and make sure Chrome is killed.
5. Resume the download notification.
6. Pause the notification before it finishes.
7. Launch Chrome from app icon.
8. Resume the download again.

The crash is:
Find ABI:arm
pid: 19454, tid: 19454, name: oid.apps.chrome  >>> com.google.android.apps.chrome <<<
signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
[FATAL:observer_list.h(271)] Check failed: false. Observers can only be added once!'

Stack Trace:                                                                                                     /system/lib/libc.so
  v------>  base::debug::(anonymous namespace)::DebugBreak()                                                                                                             /usr/local/google/home/hanxi/Projects/clankium/src/base/debug/debugger_posix.cc:229:5
  0013c415  base::debug::BreakDebugger()                                                                                                                                 /usr/local/google/home/hanxi/Projects/clankium/src/base/debug/debugger_posix.cc:263:0
  000dce03  logging::LogMessage::~LogMessage()                                                                                                                           /usr/local/google/home/hanxi/Projects/clankium/src/base/logging.cc:876:7
  0007e9cd  base::ObserverList<download::DownloadItem::Observer, false, true, base::internal::UncheckedObserverAdapter>::AddObserver(download::DownloadItem::Observer*)  /usr/local/google/home/hanxi/Projects/clankium/src/base/observer_list.h:271:7
  0007e95f  download::DownloadItemImpl::AddObserver(download::DownloadItem::Observer*)                                                                                   /usr/local/google/home/hanxi/Projects/clankium/src/components/download/internal/common/download_item_impl.cc:442:14
  00ade4b7  DownloadController::AboutToResumeDownload(download::DownloadItem*)                                                                                           /usr/local/google/home/hanxi/Projects/clankium/src/chrome/browser/android/download/download_controller.cc:305:18
  00ae094f  DownloadManagerService::ResumeDownloadInternal(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&, bool)  /usr/local/google/home/hanxi/Projects/clankium/src/chrome/browser/android/download/download_manager_service.cc:450:34
  00adff71  DownloadManagerService::ResumeDownload(_JNIEnv*, _jobject*, base::android::JavaParamRef<_jstring*> const&, bool)                                             /usr/local/google/home/hanxi/Projects/clankium/src/chrome/browser/android/download/download_manager_service.cc:276:5
  00adff37  Java_org_chromium_chrome_browser_download_DownloadManagerService_nativeResumeDownload                                                                        /usr/local/google/home/hanxi/Projects/clankium/src/out-gn/Debug/gen/chrome/browser/jni_headers/chrome/jni/DownloadManagerService_jni.h:105:18
  019c72ed  <UNKNOWN>                                                                                                                                                    /data/app/com.google.android.apps.chrome-1/oat/arm/base.odex

Comment 1 by qin...@chromium.org, Oct 30

Owner: qin...@chromium.org
We need to call RemoveObserver() and AddObserver() in DownloadController
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 1 

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

commit b5f31e6c1a6365cd270db58fa4f7ce2fd814f407
Author: Min Qin <qinmin@chromium.org>
Date: Thu Nov 01 09:28:53 2018

Small refactoring to remove observer from download item during pause

Currently when pausing a download, we remove the download controller
from observing the download item in DownloadManagerService. This CL moves
the logic to DownloadController itself when onDownloadUpdated()
is called and the download is paused.
This should have no impact on the existing flow since calling
download->Pause() will trigger a onDownloadUpdated() call. However, this
helps the servicification path as when downloadManager is created,
it calls DownloadUIController::OnDownloadCreated() for all the downloads.
And the DownloadUIController will call DownloadController::OnDownloadStarted()
to add the DownloadController as an observer to the already paused download.

BUG= 900280 

Change-Id: I58dd004b74e738cbe49db9b103357df2d0f52d3c
Reviewed-on: https://chromium-review.googlesource.com/c/1311795
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604531}
[modify] https://crrev.com/b5f31e6c1a6365cd270db58fa4f7ce2fd814f407/chrome/browser/android/download/download_controller.cc
[modify] https://crrev.com/b5f31e6c1a6365cd270db58fa4f7ce2fd814f407/chrome/browser/android/download/download_manager_service.cc

Comment 3 by qin...@chromium.org, Nov 1

Status: Fixed (was: Available)
►

Sign in to add a comment