GitLab

We're seeing crashes in GetContextOwner(), which is used to return an
identifier for a v8::Context that is constant across multiple contexts
for that same owner (e.g., an extension id). The crash is happening
at a failed CHECK:

ScriptContext* script_context = GetScriptContextFromV8ContextChecked(context);
const std::string& extension_id = script_context->GetExtensionID();
bool id_is_valid = crx_file::id_util::IdIsValid(extension_id);
CHECK(id_is_valid || script_context->url().is_valid());

Looking at the crash dumps, it appears that this crash happens with a
ScriptContext that has neither an associated extension nor a valid URL
(the URL is empty). The context type is for a Feature::WEB_PAGE_CONTEXT.

In theory, this should never happen, because contexts should only be
classified as a WEB_PAGE_CONTEXT with a valid URL. However, there is a
chance that it could, as described in https://crbug.com/877658. My
suspicion is that this happens during page navigation, when we are
loading/initializing a new context.

Both native and JS-based bindings use the same check for validity, but
it appears this crash only happens in native bindings. The primary
difference is that native bindings check a context owner's identity
immediately on event construction, whereas JS bindings do it lazily on
listener addition/removal.

In order to try and mitigate the crashes, change native bindings to
mimic JS bindings behavior, where the context identity is determined
lazily. This will not fix the underlying context issue, but should
hopefully address the crashes. If it doesn't, we'll have to go back to
the drawing board.

Bug: 877401

Change-Id: I8b29ec872b1fdb33f74b96bfa85e721b5e26b870
Reviewed-on: https://chromium-review.googlesource.com/1188764
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586092}