Skip to content

Commit

Permalink
[Extensions Bindings] Gracefully handle invalidated contexts in app h…
Browse files Browse the repository at this point in the history
…ooks

Handle the case of a context being invalidated before accessing
chrome.app.isInstalled. This can happen in cases where the owning frame
is removed.

Add a regression test for the same.

Bug: 855853
Change-Id: Ic9e5f5a45fc2a833c5c1a45d3b7c1e8c04c71a18
Reviewed-on: https://chromium-review.googlesource.com/1119066
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571355}
  • Loading branch information
rdcronin authored and Commit Bot committed Jun 29, 2018
1 parent 9aeb746 commit e97d4ed
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
28 changes: 28 additions & 0 deletions chrome/browser/extensions/chrome_app_api_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,34 @@ IN_PROC_BROWSER_TEST_F(ChromeAppAPITest, IsInstalled) {
EXPECT_EQ("true", result);
}

// Test accessing app.isInstalled when the context has been invalidated (e.g.
// by removing the frame). Regression test for https://crbug.com/855853.
IN_PROC_BROWSER_TEST_F(ChromeAppAPITest, IsInstalledFromRemovedFrame) {
GURL app_url =
embedded_test_server()->GetURL("app.com", "/extensions/test_file.html");
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("app_dot_com_app"));
ASSERT_TRUE(extension);
ui_test_utils::NavigateToURL(browser(), app_url);

constexpr char kScript[] =
R"(var i = document.createElement('iframe');
i.onload = function() {
var frameApp = i.contentWindow.chrome.app;
document.body.removeChild(i);
var isInstalled = frameApp.isInstalled;
window.domAutomationController.send(
isInstalled === undefined);
};
i.src = '%s';
document.body.appendChild(i);)";
bool result = false;
ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
browser()->tab_strip_model()->GetActiveWebContents(),
base::StringPrintf(kScript, app_url.spec().c_str()), &result));
EXPECT_TRUE(result);
}

IN_PROC_BROWSER_TEST_F(ChromeAppAPITest, InstallAndRunningState) {
GURL app_url = embedded_test_server()->GetURL(
"app.com", "/extensions/get_app_details_for_frame.html");
Expand Down
7 changes: 6 additions & 1 deletion chrome/renderer/extensions/app_hooks_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ void IsInstalledGetterCallback(
v8::Local<v8::Context> context = info.Holder()->CreationContext();
ScriptContext* script_context =
ScriptContextSet::GetContextByV8Context(context);
DCHECK(script_context);

// The ScriptContext may have been invalidated if e.g. the frame was removed.
// Return undefined in this case.
if (!script_context)
return;

auto* core =
static_cast<AppBindingsCore*>(info.Data().As<v8::External>()->Value());
// Since this is more-or-less an API, log it as an API call.
Expand Down

0 comments on commit e97d4ed

Please sign in to comment.