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

Issue 751837 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: 5
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

[Omnibox]: Stop ContextualSuggestionsService's processing when ZeroSuggestProvider::Stop() is called.

Project Member Reported by kenjitoyama@chromium.org, Aug 2 2017

Issue description

Chrome Version: (copy from chrome://version)
OS: All

What steps will reproduce the problem?
(1) Build a non-prod version of Chrome after CL https://chromium-review.googlesource.com/c/576284 lands.
(2) Enable "Experimental contextual omnibox suggestion" in chrome://flags.
(3) Restart Chrome.
(4) Visit an ordinary page like http://www.cnn.com/
(5) Click on the omnibox.
(6) Repeat (3) and (4) lots of times until the first DCHECK() fails on ZeroSuggestProvider::OnURLFetchComplete().

What is the expected result?

No crash.

What happens instead?

A DCHECK(!done_)-fail.

Please use labels and text to provide additional information.

My suspicion after some extensive debugging is that ZeroSuggestProvider::Stop() is called (externally), which sets done_ to true, while a URL fetching is happening. When the URL fetching is finally completed, the OnURLFetchComplete method is triggered, which then causes the failed check in debug mode.

We need to find a way to stop all current processing in ContextualSuggestionsService if ZeroSuggestProvider::Stop() is called.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

 
Status: Assigned (was: Untriaged)
 Issue 756694  has been merged into this issue.
Labels: -Pri-3 -OS-All Hotlist-ConOps-Channel-Canary Hotlist-ConOps Hotlist-ConOps-Source-Feedback M-62 OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Bumping priority because this has been reported by ConOps and adding flags from merged-in bug.

Labels: ReleaseBlock-Stable
Your bug is tagged as Release block Stable. 

M62 is branching soon and we will be taking only CRITICAL merges. Please plan accordingly.
Cc: jdonnelly@chromium.org k...@chromium.org
 Issue 755165  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 28 2017

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

commit 86bbdf613f979271a555fa02d6564e4482a6ff1f
Author: Gheorghe Comanici <gcomanici@chromium.org>
Date: Mon Aug 28 17:20:33 2017

When zs provider is stopped during OAuthToken fetch, stop fetching.

In the current state, if zero suggest provider is stopped while 
waiting for a contextual suggestion fetcher to be produced (i.e. 
OnContextualSuggestionsFetcherAvailable did not yet run), the Stop
action is ignored and the fetch is still performed.

This CL fixes this issue.

Also, maybe on later requests, the fetcher doesn't attach
OAuth2 tokens because the system thinks the fetcher is still
running.  This change makes this dependable.  (If it's no
longer running, make sure to delete the fetcher.)

TBR=rohitrao
for mechanical changes to ios/chrome/browser/autocomplete/*

Bug:  751837 
Change-Id: I1f84ae9658d509d2159964d05f25612e93a67305
Reviewed-on: https://chromium-review.googlesource.com/621226
Commit-Queue: Gheorghe Comanici <gcomanici@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497785}
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/chrome/browser/autocomplete/contextual_suggestions_service_factory.cc
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/chrome/browser/autocomplete/contextual_suggestions_service_factory.h
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/components/omnibox/browser/autocomplete_provider_client.h
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/components/omnibox/browser/contextual_suggestions_service.cc
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/components/omnibox/browser/contextual_suggestions_service.h
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/components/omnibox/browser/mock_autocomplete_provider_client.h
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/components/omnibox/browser/zero_suggest_provider.cc
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.cc
[modify] https://crrev.com/86bbdf613f979271a555fa02d6564e4482a6ff1f/ios/chrome/browser/autocomplete/autocomplete_provider_client_impl.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 29 2017

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

commit 8a50087c8ccc71c32b8544559d0841798b369c0e
Author: Gheorghe Comanici <gcomanici@chromium.org>
Date: Tue Aug 29 20:11:10 2017

AutocompleteClassifierFactory: Add missing dependency.

This CL adds a missing dependency on ContextualSuggestionsServiceFactory. 
This missing dependency was causing issues with the order of destruction
of services. For details see Patchset 14 below

https://chromium-review.googlesource.com/c/chromium/src/+/621226/14

Bug:  751837 
Change-Id: I57e03fd161d9a5a221c3ff7e768bc69d02599a33
Reviewed-on: https://chromium-review.googlesource.com/639452
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Gheorghe Comanici <gcomanici@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498213}
[modify] https://crrev.com/8a50087c8ccc71c32b8544559d0841798b369c0e/chrome/browser/autocomplete/autocomplete_classifier_factory.cc

Status: Fixed (was: Assigned)
when dev channel gets this fix? Bug is very annoying.

Comment 11 by n...@tresorit.com, Aug 31 2017

You can workaround this by disabling the "Experimental contextual omnibox suggestion" flag (chrome://flags) until the fix is released.
 Issue 761262  has been merged into this issue.

Sign in to add a comment