Commit 1063641d authored by Yuki Shiino's avatar Yuki Shiino Committed by Commit Bot

v8binding: Fixes a crash issue at GeoNotifier::TimerFired.

Geolocation stops owning GeoNotifiers in some cases (e.g.
Geolocation::clearWatch), so it's possible that, when
GeoNotifier::TimerFired gets invoked, the owner geolocation
had already stopped owning the notifier.  In this scenario,
no one is performing wrapper-tracing for the notifier, thus
the underlying V8 functions might have already been
collected by V8 GC.  GeoNotifier must not invoke any
callback in such a case.

This patch adds a check whether the geolocation still owns
the notifier or not.

Bug: 792604
Tbr: haraken@chromium.org
Change-Id: I46f2d34f62bac2073b75eb36b935172cb1c10465
Reviewed-on: https://chromium-review.googlesource.com/893240
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: default avatarHitoshi Yoshida <peria@chromium.org>
Reviewed-by: default avatarAdam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532945}
parent b80c1b5a
......@@ -93,6 +93,12 @@ void GeoNotifier::TimerFired(TimerBase*) {
// TODO(yukishiino): Remove this check once we understand the cause.
// https://crbug.com/792604
CHECK(!geolocation_->GetExecutionContext()->IsContextDestroyed());
// As the timer fires asynchronously, it's possible that |geolocation_|
// no longer owns this notifier, i.e. |geolocation_| is no longer performing
// wrapper-tracing. In that case, the underlying V8 function may not be alive.
if (!geolocation_->DoesOwnNotifier(this)) {
return; // Do not invoke anything because of no owner geolocation.
}
// Test for fatal error first. This is required for the case where the
// LocalFrame is disconnected and requests are cancelled.
......
......@@ -296,6 +296,10 @@ void Geolocation::RequestTimedOut(GeoNotifier* notifier) {
StopUpdating();
}
bool Geolocation::DoesOwnNotifier(GeoNotifier* notifier) const {
return one_shots_.Contains(notifier) || watchers_.Contains(notifier);
}
bool Geolocation::HaveSuitableCachedPosition(const PositionOptions& options) {
if (!last_position_)
return false;
......
......@@ -99,6 +99,9 @@ class MODULES_EXPORT Geolocation final
// Discards the notifier if it is a oneshot because it timed it.
void RequestTimedOut(GeoNotifier*);
// Returns true if this geolocation still owns the given notifier.
bool DoesOwnNotifier(GeoNotifier*) const;
// Inherited from PageVisibilityObserver.
void PageVisibilityChanged() override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment