Browse Source

chore: cherry-pick fe20b05a0e5e from chromium (#28780)

* chore: cherry-pick fe20b05a0e5e from chromium

* update patches

Co-authored-by: Electron Bot <[email protected]>
Co-authored-by: Cheng Zhao <[email protected]>
Pedro Pontes 4 years ago
parent
commit
95d9d7ddfe
2 changed files with 141 additions and 0 deletions
  1. 1 0
      patches/chromium/.patches
  2. 140 0
      patches/chromium/cherry-pick-fe20b05a0e5e.patch

+ 1 - 0
patches/chromium/.patches

@@ -166,4 +166,5 @@ cherry-pick-6a6361c9f31c.patch
 cherry-pick-012e9baf46c9.patch
 cherry-pick-8c3eb9d1c409.patch
 use_idtype_for_permission_change_subscriptions.patch
+cherry-pick-fe20b05a0e5e.patch
 cherry-pick-6b84dc72351b.patch

+ 140 - 0
patches/chromium/cherry-pick-fe20b05a0e5e.patch

@@ -0,0 +1,140 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jana Grill <[email protected]>
+Date: Tue, 20 Apr 2021 18:23:33 +0000
+Subject: M86-LTS: DevTools: expect PageHandler may be destroyed during
+ Page.navigate
+
+(cherry picked from commit ff5e70191ec701cce4f84aaa25cd676376253a8a)
+
+Bug: 1188889
+Change-Id: I5c2fcca84834d66c46d77a70683212c2330177a5
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2787756
+Commit-Queue: Andrey Kosyakov <[email protected]>
+Reviewed-by: Dmitry Gozman <[email protected]>
+Reviewed-by: Karan Bhatia <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#867507}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2821536
+Commit-Queue: Achuith Bhandarkar <[email protected]>
+Reviewed-by: Achuith Bhandarkar <[email protected]>
+Reviewed-by: Victor-Gabriel Savu <[email protected]>
+Owners-Override: Achuith Bhandarkar <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4240@{#1618}
+Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
+
+diff --git a/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+index 71ce5a3399db29451e990d530736460aa28eeec0..b35accc8ce46f3465624898fe18d463529498d07 100644
+--- a/chrome/browser/extensions/api/debugger/debugger_apitest.cc
++++ b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+@@ -24,6 +24,7 @@
+ #include "components/sessions/content/session_tab_helper.h"
+ #include "content/public/test/browser_test.h"
+ #include "content/public/test/browser_test_utils.h"
++#include "content/public/test/no_renderer_crashes_assertion.h"
+ #include "extensions/browser/extension_function.h"
+ #include "extensions/common/extension.h"
+ #include "extensions/common/extension_builder.h"
+@@ -353,6 +354,19 @@ IN_PROC_BROWSER_TEST_F(DebuggerExtensionApiTest,
+       << message_;
+ }
+ 
++// Tests that navigation to a forbidden URL is properly denied and
++// does not cause a crash.
++// This is a regression test for https://crbug.com/1188889.
++IN_PROC_BROWSER_TEST_F(DebuggerExtensionApiTest, DISABLED_NavigateToForbiddenUrl) {
++  content::ScopedAllowRendererCrashes scoped_allow_renderer_crashes;
++  // We don't send a DevTools command callback before disconnecting the session,
++  // so the extension does not receive a callback either.
++  base::AutoReset<bool> ignore_did_respond(
++      &ExtensionFunction::ignore_all_did_respond_for_testing_do_not_use, true);
++  ASSERT_TRUE(RunExtensionTest("debugger_navigate_to_forbidden_url"))
++      << message_;
++}
++
+ class SitePerProcessDebuggerExtensionApiTest : public DebuggerExtensionApiTest {
+  public:
+   void SetUpCommandLine(base::CommandLine* command_line) override {
+diff --git a/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/background.js b/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/background.js
+new file mode 100644
+index 0000000000000000000000000000000000000000..e2ef32fffd3e5d49e7dc10d53f8c891ddb0f3872
+--- /dev/null
++++ b/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/background.js
+@@ -0,0 +1,28 @@
++// Copyright 2021 The Chromium Authors. All rights reserved.
++// Use of this source code is governed by a BSD-style license that can be
++// found in the LICENSE file.
++
++const protocolVersion = '1.3';
++const DETACHED_WHILE_HANDLING = 'Detached while handling command.';
++
++chrome.test.runTests([
++  async function testNavigateToForbiddenUrl() {
++    const {openTab} = await import('/_test_resources/test_util/tabs_util.js');
++    const tab = await openTab('about:blank');
++    const debuggee = {tabId: tab.id};
++    await new Promise(resolve =>
++        chrome.debugger.attach(debuggee, protocolVersion, resolve));
++    chrome.debugger.sendCommand(debuggee, 'Page.crash');
++    await new Promise(resolve =>
++        chrome.debugger.onEvent.addListener((source, method, params) => {
++          if (method === 'Inspector.targetCrashed')
++            resolve();
++        }));
++    const result = await new Promise(resolve =>
++      chrome.debugger.sendCommand(debuggee, 'Page.navigate', {
++          url: 'chrome://version'
++      }, resolve));
++    chrome.test.assertLastError(DETACHED_WHILE_HANDLING);
++    chrome.test.succeed();
++  }
++]);
+diff --git a/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/manifest.json b/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/manifest.json
+new file mode 100644
+index 0000000000000000000000000000000000000000..05db294ed7f49893431b0039a5f338d20e08f27d
+--- /dev/null
++++ b/chrome/test/data/extensions/api_test/debugger_navigate_to_forbidden_url/manifest.json
+@@ -0,0 +1,11 @@
++{
++  "name": "Debugger API test for CDP-initiated navigation to forbidden URLs",
++  "version": "1.0",
++  "manifest_version": 2,
++  "background": {
++    "scripts": ["background.js"]
++  },
++  "permissions": [
++    "debugger"
++  ]
++}
+diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
+index 630de0dd016fd3d054bcd40b22d75a242eeaa23e..a340d3e4519ada9edba279090ea11b57521ef0f4 100644
+--- a/content/browser/devtools/protocol/page_handler.cc
++++ b/content/browser/devtools/protocol/page_handler.cc
+@@ -496,7 +496,12 @@ void PageHandler::Navigate(const std::string& url,
+   params.referrer = Referrer(GURL(referrer.fromMaybe("")), policy);
+   params.transition_type = type;
+   params.frame_tree_node_id = frame_tree_node->frame_tree_node_id();
++  // Handler may be destroyed while navigating if the session
++  // gets disconnected as a result of access checks.
++  base::WeakPtr<PageHandler> weak_self = weak_factory_.GetWeakPtr();
+   frame_tree_node->navigator().GetController()->LoadURLWithParams(params);
++  if (!weak_self)
++    return;
+ 
+   base::UnguessableToken frame_token = frame_tree_node->devtools_frame_token();
+   auto navigate_callback = navigate_callbacks_.find(frame_token);
+diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc
+index 52fdd0f1066699cc019c33de2517c23f12b4a616..8795c547717b206f4e459f655f6e62a7ba9229e0 100644
+--- a/content/browser/devtools/render_frame_devtools_agent_host.cc
++++ b/content/browser/devtools/render_frame_devtools_agent_host.cc
+@@ -472,8 +472,11 @@ void RenderFrameDevToolsAgentHost::UpdateFrameHost(
+     if (!ShouldAllowSession(session))
+       restricted_sessions.push_back(session);
+   }
+-  if (!restricted_sessions.empty())
++  scoped_refptr<RenderFrameDevToolsAgentHost> protect;
++  if (!restricted_sessions.empty()) {
++    protect = this;
+     ForceDetachRestrictedSessions(restricted_sessions);
++  }
+ 
+   UpdateFrameAlive();
+ }