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

Issue 648361 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Don't invoke callbacks on PLMOs that are no longer tracking

Project Member Reported by bmcquade@chromium.org, Sep 19 2016

Issue description

The current StopTracking implementation sets a did_stop_tracking_ boolean which is used in the destructor to avoid invoking the final callback. However, partial callbacks, such as timing callbacks, continue to get invoked. This is arguably a bug, especially now that we've added 'immediate' timing callbacks. We should fix this by discarding all observers at the time we stop tracking.
 
Cc: csharrison@chromium.org
The initial description is incorrect. Timing callbacks won't get invoked due to policy that prevents the IPCs from getting sent on the render process side. That said, browser-initiated callbacks like WasShown and WasHidden do get invoked which seems incorrect.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20 2016

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

commit 16e566dd1cd74c5a5def9a1d60077f6973e9f000
Author: bmcquade <bmcquade@chromium.org>
Date: Tue Sep 20 12:33:26 2016

Destroy observers eagerly when StopTracking is called.

Currently, if we decide to StopTracking a navigation at commit time,
some callbacks may still get invoked. In particular, browser-initiated
callbacks like WasShown and WasHidden may be called; timing and other
callbacks should be prevented by RenderPageTrackDecider policy in the
render process. This change makes us destroy observers eagerly at the
time StopTracking() gets called, which prevents us from invoking any
subsequent callbacks on observers after this point.

BUG= 648361 

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

[modify] https://crrev.com/16e566dd1cd74c5a5def9a1d60077f6973e9f000/chrome/browser/page_load_metrics/metrics_web_contents_observer.cc

Status: Fixed (was: Assigned)

Sign in to add a comment