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

Issue 803028 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK in RemoteSuggestionsProviderImpl::NotifyFetchWithLoadingIndicatorFailedOrTimeouted()

Project Member Reported by bauerb@chromium.org, Jan 17 2018

Issue description

Chrome revision: r529686 (trunk)

Stack: 
[FATAL:remote_suggestions_provider_impl.cc(686)] Check failed: content.status == CategoryStatus::AVAILABLE_LOADING (5 vs. 2)
    r0 00000000  r1 0000063d  r2 00000006  r3 00000008
    r4 f5be558c  r5 00000006  r6 f5be5534  r7 0000010c
    r8 d679a0b8  r9 d679ade0  sl ff947680  fp ff947684
    ip 0000000b  sp ff9471a8  lr f3f9d5e7  pc f3f9fe44

Stack Trace:
  RELADDR   FUNCTION
  00049e44  <UNKNOWN>
  000475e3  <UNKNOWN>
  0001d8a5  <UNKNOWN>
  000193f1  <UNKNOWN>
  00017034  <UNKNOWN>
  v------>  base::debug::(anonymous namespace)::DebugBreak()
  00096cb1  base::debug::BreakDebugger()
  000a85ab  logging::LogMessage::~LogMessage()
  00b02ddb  ntp_snippets::RemoteSuggestionsProviderImpl::NotifyFetchWithLoadingIndicatorFailedOrTimeouted()
  00b04fd9  void base::internal::Invoker<base::internal::BindState<void (ntp_snippets::RemoteSuggestionsProviderImpl::*)(), base::internal::UnretainedWrapper<ntp_snippets::RemoteSuggestionsProviderImpl> >, void ()>::RunImpl<void (ntp_snippets::RemoteSuggestionsProviderImpl::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<ntp_snippets::RemoteSuggestionsProviderImpl> > const&, 0u>(void (ntp_snippets::RemoteSuggestionsProviderImpl::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<ntp_snippets::RemoteSuggestionsProviderImpl> > const&, std::__ndk1::integer_sequence<unsigned int, 0u>)
  000ebeed  base::Timer::RunScheduledTask()
  000ebf1f  void base::internal::Invoker<base::internal::BindState<void (base::BaseTimerTaskInternal::*)(), base::internal::OwnedWrapper<base::BaseTimerTaskInternal> >, void ()>::RunImpl<void (base::BaseTimerTaskInternal::*)(), std::__ndk1::tuple<base::internal::OwnedWrapper<base::BaseTimerTaskInternal> >, 0u>(void (base::BaseTimerTaskInternal::*&&)(), std::__ndk1::tuple<base::internal::OwnedWrapper<base::BaseTimerTaskInternal> >&&, std::__ndk1::integer_sequence<unsigned int, 0u>)
  0008e8dd  base::OnceCallback<void ()>::Run() &&
  000971ad  base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
  000ad355  base::internal::IncomingTaskQueue::RunTask(base::PendingTask*)
  000af2eb  base::MessageLoop::RunTask(base::PendingTask*)
  000af5d7  base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
  000af7c5  base::MessageLoop::DoDelayedWork(base::TimeTicks*)
  000b03d3  base::MessagePumpForUI::DoRunLoopOnce(_JNIEnv*, base::android::JavaParamRef<_jobject*> const&, unsigned char)
  000b0391  Java_org_chromium_base_SystemMessageHandler_nativeDoRunLoopOnce
  000164f5  <UNKNOWN>

I came across this when testing Chrome with a child account (where IIRC we disable content suggestions). It might be possible (and easier) to reproduce this with content suggestions disabled via enterprise policy.
 
Labels: Needs-Feedback
In which version ahs this first come up?

Comment 2 by bauerb@chromium.org, Jan 18 2018

I saw this yesterday when building from trunk (https://crrev.com/529686).

Comment 3 by jkrcal@chromium.org, Jan 19 2018

Thanks for reporting, Bernhard!

Will look into it on Monday, when I am back in office!

Comment 4 by jkrcal@chromium.org, Jan 25 2018

I see the potential for the crash but still am curious how you reproduce the bug, Bernhard?

To me it looks like a fetch is initialized when suggestions are still enabled and timeouts when they are disabled (by child account / enterprise policy). Do I miss something?

Comment 5 by bauerb@chromium.org, Jan 25 2018

That doesn't seem completely inconceivable. The child account happens asynchronously, and enterprise policy could also change at any time.

Comment 6 by jkrcal@chromium.org, Jan 30 2018

Cc: jkrcal@chromium.org tschumann@chromium.org
Labels: -Pri-1 Pri-2
Owner: zea@chromium.org
Reassigning to SEA.

The crash at remote_suggestions_provider_impl.cc:686 is due to the DCHECK being too restrictive.

When fetching suggestions after longer time (e.g. more then a day), we do not want to show old suggestions in the mean-time. We show a spinner instead. This is communicated to the UI by flipping the status of the articles category to AVAILABLE_LOADING and flipping it back to AVAILABLE when the fetch finishes.

In this case, the suggestions get disabled in the meantime (using settings, enterprise policy kicking in, child account kicking in) which causes the category status flip to CATEGORY_EXPLICITLY_DISABLED.

Comment 7 by zea@chromium.org, Feb 1 2018

Owner: pnoland@chromium.org
Patrick, could you take a look?
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 2 2018

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

commit 21389bc91483c63cc0468a19610c08673a7c3835
Author: Patrick Noland <pnoland@google.com>
Date: Fri Feb 02 18:01:37 2018

Relax DCHECK in remote_suggestions_provider_impl

This DCHECK, as Jan explains in the bug, is too restrictive. It's valid
for the state to flip to CATEGORY_EXPLICITLY_DISABLED before the timeout
triggers.

Bug:  803028 
Change-Id: Ifd206a583b9dbef5650d1ff04f2da65e9a0459e2
Reviewed-on: https://chromium-review.googlesource.com/898068
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Patrick Noland <pnoland@google.com>
Cr-Commit-Position: refs/heads/master@{#534090}
[modify] https://crrev.com/21389bc91483c63cc0468a19610c08673a7c3835/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
[modify] https://crrev.com/21389bc91483c63cc0468a19610c08673a7c3835/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
[modify] https://crrev.com/21389bc91483c63cc0468a19610c08673a7c3835/components/ntp_snippets/remote/remote_suggestions_provider_impl_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment