github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick 57bdd90d8c from chromium (#33886)

Browse Source
This commit is contained in:
Pedro Pontes
2022-04-28 04:52:08 +02:00
committed by GitHub
parent d10adccf3f
commit af7d3b7171
2 changed files with 267 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -128,3 +128,4 @@ cherry-pick-4d26949260aa.patch
skia_renderer_use_rectf_intersect_in_applyscissor.patch
cherry-pick-1a31e2110440.patch
cherry-pick-5cb934a23ddf.patch
use_iserrordocument_to_prevent_bfcacheing_of_interstitials_and.patch

266
patches/chromium/use_iserrordocument_to_prevent_bfcacheing_of_interstitials_and.patch Normal file
View File

@@ -0,0 +1,266 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Fergal Daly <fergal@chromium.org>
Date: Thu, 7 Apr 2022 02:16:11 +0000
Subject: Use IsErrorDocument() to prevent BFCacheing of interstitials and
errors.
In the bug, a crash occurs because we try to cache an interstitial. We
catch some error documents via status codes etc but interstitials do
not consistently set those. Checking IsErrorDocument() is more reliable.
(cherry picked from commit 7a05b426c6c51254a08de9a8dee8db9c1911b9c9)
Bug: 1274308,1287996,1283050
Change-Id: Ifec662c169c77e33ca5dc4d56b0e42c8d71f1d97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3319862
Commit-Queue: Fergal Daly <fergal@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#981026}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3559271
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Owners-Override: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/branch-heads/4896@{#1065}
Cr-Branched-From: 1f63ff4bc27570761b35ffbc7f938f6586f7bee8-refs/heads/main@{#972766}
diff --git a/base/tracing/protos/chrome_track_event.proto b/base/tracing/protos/chrome_track_event.proto
index d85626908f9c9e8ddb661a34cb89be4523b07ee4..47df2f78c611f0ac47b4de85078e07b01f603306 100644
--- a/base/tracing/protos/chrome_track_event.proto
+++ b/base/tracing/protos/chrome_track_event.proto
@@ -514,6 +514,7 @@ message BackForwardCacheCanStoreDocumentResult {
CACHE_CONTROL_NO_STORE_HTTP_ONLY_COOKIE_MODIFIED = 51;
NO_RESPONSE_HEAD = 52;
ACTIVATION_NAVIGATION_DISALLOWED_FOR_BUG_1234857 = 53;
+ ERROR_DOCUMENT = 54;
}
optional BackForwardCacheNotRestoredReason
diff --git a/content/browser/back_forward_cache_basics_browsertest.cc b/content/browser/back_forward_cache_basics_browsertest.cc
index 450e27a2e7a3477a08c911923bd6d055dba5a77d..0511b468f25d3430e3e725ae262ca45c6777a6f8 100644
--- a/content/browser/back_forward_cache_basics_browsertest.cc
+++ b/content/browser/back_forward_cache_basics_browsertest.cc
@@ -744,7 +744,8 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
EXPECT_FALSE(WaitForLoadStop(shell()->web_contents()));
ExpectNotRestored(
{BackForwardCacheMetrics::NotRestoredReason::kHTTPStatusNotOK,
- BackForwardCacheMetrics::NotRestoredReason::kNoResponseHead},
+ BackForwardCacheMetrics::NotRestoredReason::kNoResponseHead,
+ NotRestoredReason::kErrorDocument},
{}, {}, {}, {}, FROM_HERE);
}
diff --git a/content/browser/back_forward_cache_browsertest.cc b/content/browser/back_forward_cache_browsertest.cc
index 72d9f9b0c056a384d446f576b725e53e78d5474f..51d3829bee15be4bb8e315908772b6ab02a8ee56 100644
--- a/content/browser/back_forward_cache_browsertest.cc
+++ b/content/browser/back_forward_cache_browsertest.cc
@@ -52,6 +52,7 @@
#include "content/public/test/test_navigation_throttle_inserter.h"
#include "content/public/test/test_utils.h"
#include "content/public/test/text_input_test_utils.h"
+#include "content/public/test/url_loader_interceptor.h"
#include "content/shell/browser/shell.h"
#include "content/shell/browser/shell_javascript_dialog_manager.h"
#include "content/test/web_contents_observer_test_utils.h"
@@ -665,6 +666,23 @@ void BackForwardCacheBrowserTest::ExpectBrowsingInstanceNotSwappedReasons(
<< location.ToString();
}
+void BackForwardCacheBrowserTest::NavigateAndBlock(GURL url,
+ int history_offset) {
+ // Block the navigation with an error.
+ std::unique_ptr<URLLoaderInterceptor> url_interceptor =
+ URLLoaderInterceptor::SetupRequestFailForURL(url,
+ net::ERR_BLOCKED_BY_CLIENT);
+ TestNavigationManager manager(web_contents(), url);
+ if (history_offset) {
+ shell()->GoBackOrForward(history_offset);
+ } else {
+ shell()->LoadURL(url);
+ }
+ manager.WaitForNavigationFinished();
+ ASSERT_EQ(current_frame_host()->GetLastCommittedURL(), url);
+ ASSERT_TRUE(current_frame_host()->IsErrorDocument());
+}
+
std::initializer_list<RenderFrameHostImpl*> Elements(
std::initializer_list<RenderFrameHostImpl*> t) {
return t;
diff --git a/content/browser/back_forward_cache_browsertest.h b/content/browser/back_forward_cache_browsertest.h
index fa1c8d0223ba43366ff34acd2a150901ca04b239..e5c915382bbffd8ded673b9c997532eded4184f4 100644
--- a/content/browser/back_forward_cache_browsertest.h
+++ b/content/browser/back_forward_cache_browsertest.h
@@ -167,6 +167,11 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest,
void ReleaseKeyboardLock(RenderFrameHostImpl* rfh);
+ // Start a navigation to |url| but block it on an error. If |history_offset|
+ // is not 0, then the navigation will be a history navigation and this will
+ // assert that the URL after navigation is |url|.
+ void NavigateAndBlock(GURL url, int history_offset);
+
base::HistogramTester histogram_tester_;
bool same_site_back_forward_cache_enabled_ = true;
diff --git a/content/browser/back_forward_cache_internal_browsertest.cc b/content/browser/back_forward_cache_internal_browsertest.cc
index aad18a04a8775c937fa2ccb57cc2f102da729dbe..41e637bb394d4f5df3cd86d467336e7457e4aa39 100644
--- a/content/browser/back_forward_cache_internal_browsertest.cc
+++ b/content/browser/back_forward_cache_internal_browsertest.cc
@@ -3659,4 +3659,69 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
tester.IsDisabledForFrameWithReason(process_id, routing_id, reason));
}
+// Test that when we navigate away from an error page and back with no error
+// that we don't serve the error page from BFCache.
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+ ErrorDocumentNotCachedWithSecondError) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+
+ GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+ GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+ // Navigate to a.com.
+ ASSERT_TRUE(NavigateToURL(web_contents(), url_a));
+
+ // Navigate to b.com and block due to an error.
+ NavigateAndBlock(url_b, /*history_offset=*/0);
+ RenderFrameHostImplWrapper rfh_b(current_frame_host());
+
+ // Navigate back to a.com.
+ ASSERT_TRUE(HistoryGoBack(web_contents()));
+ ExpectRestored(FROM_HERE);
+ ASSERT_TRUE(rfh_b.WaitUntilRenderFrameDeleted());
+
+ // Navigate forward to b.com again and block with an error again.
+ NavigateAndBlock(url_b, /*history_offset=*/1);
+ ExpectNotRestored(
+ {NotRestoredReason::kHTTPStatusNotOK, NotRestoredReason::kNoResponseHead,
+ NotRestoredReason::kErrorDocument},
+ {}, {}, {}, {}, FROM_HERE);
+}
+
+// Test that when we navigate away from an error page and back with no error
+// that we don't serve the error page from BFCache.
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
+ ErrorDocumentNotCachedWithoutSecondError) {
+ ASSERT_TRUE(embedded_test_server()->Start());
+
+ GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+ GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+ // Navigate to a.com.
+ ASSERT_TRUE(NavigateToURL(web_contents(), url_a));
+
+ // Navigate to b.com and block due to an error.
+ NavigateAndBlock(url_b, /*history_offset=*/0);
+ RenderFrameHostImplWrapper rfh_b(current_frame_host());
+
+ int history_entry_id =
+ web_contents()->GetController().GetLastCommittedEntry()->GetUniqueID();
+
+ // Navigate back to a.com.
+ ASSERT_TRUE(HistoryGoBack(web_contents()));
+ ASSERT_TRUE(rfh_b.WaitUntilRenderFrameDeleted());
+
+ // Navigate forward to b.com again with no error.
+ ASSERT_TRUE(HistoryGoForward(web_contents()));
+
+ // We would normally confirm that the blocking reasons are correct, however,
+ // when performing a history navigations back to an error document, a new
+ // entry is created and the reasons in the old entry are not recorded.
+ //
+ // Check that we indeed got a new history entry.
+ ASSERT_NE(
+ history_entry_id,
+ web_contents()->GetController().GetLastCommittedEntry()->GetUniqueID());
+}
+
} // namespace content
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index d48b48718e5eb3ca09b439af4766d8749cb0be98..842fdf70c1d826d5fb26a2e8c2a3f228a18b24ea 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -1407,6 +1407,8 @@ Page::BackForwardCacheNotRestoredReason NotRestoredReasonToProtocol(
case Reason::kActivationNavigationsDisallowedForBug1234857:
return Page::BackForwardCacheNotRestoredReasonEnum::
ActivationNavigationsDisallowedForBug1234857;
+ case Reason::kErrorDocument:
+ return Page::BackForwardCacheNotRestoredReasonEnum::ErrorDocument;
case Reason::kBlocklistedFeatures:
// Blocklisted features should be handled separately and be broken down
// into sub reasons.
@@ -1686,6 +1688,7 @@ Page::BackForwardCacheNotRestoredReasonType MapNotRestoredReasonToType(
case Reason::kCacheControlNoStoreCookieModified:
case Reason::kCacheControlNoStoreHTTPOnlyCookieModified:
case Reason::kNoResponseHead:
+ case Reason::kErrorDocument:
return Page::BackForwardCacheNotRestoredReasonTypeEnum::Circumstantial;
case Reason::kOptInUnloadHeaderNotPresent:
case Reason::kUnloadHandlerExistsInMainFrame:
diff --git a/content/browser/renderer_host/back_forward_cache_can_store_document_result.cc b/content/browser/renderer_host/back_forward_cache_can_store_document_result.cc
index 9077a176fff2af3ec4a84f918e81601b618902df..d2b7608067198a4bc623854feb8c1495a6662a87 100644
--- a/content/browser/renderer_host/back_forward_cache_can_store_document_result.cc
+++ b/content/browser/renderer_host/back_forward_cache_can_store_document_result.cc
@@ -183,6 +183,8 @@ ProtoEnum::BackForwardCacheNotRestoredReason NotRestoredReasonToTraceEnum(
return ProtoEnum::NO_RESPONSE_HEAD;
case Reason::kActivationNavigationsDisallowedForBug1234857:
return ProtoEnum::ACTIVATION_NAVIGATION_DISALLOWED_FOR_BUG_1234857;
+ case Reason::kErrorDocument:
+ return ProtoEnum::ERROR_DOCUMENT;
case Reason::kBlocklistedFeatures:
return ProtoEnum::BLOCKLISTED_FEATURES;
case Reason::kUnknown:
@@ -396,6 +398,8 @@ std::string BackForwardCacheCanStoreDocumentResult::NotRestoredReasonToString(
return "Activation navigations are disallowed to avoid bypassing "
"PasswordProtectionService as a workaround for "
"https://crbug.com/1234857.";
+ case Reason::kErrorDocument:
+ return "Error documents cannot be stored in bfcache";
}
}
diff --git a/content/browser/renderer_host/back_forward_cache_impl.cc b/content/browser/renderer_host/back_forward_cache_impl.cc
index ac7cb84e8059be774f48da429e21fd40a7bc53b8..5db2853cdb207bc3ceeccd9bf5ec0092db0aed25 100644
--- a/content/browser/renderer_host/back_forward_cache_impl.cc
+++ b/content/browser/renderer_host/back_forward_cache_impl.cc
@@ -727,6 +727,13 @@ BackForwardCacheImpl::CanPotentiallyStorePageLater(RenderFrameHostImpl* rfh) {
if (rfh->last_http_status_code() != net::HTTP_OK)
result.No(BackForwardCacheMetrics::NotRestoredReason::kHTTPStatusNotOK);
+ // Interstitials and other internal error pages should set an error status
+ // code but there's no guarantee, e.g. https://crbug/1274308,
+ // https://crbug/1287996. This catches those cases. It might also make the
+ // kHTTPStatusNotOK check redundant.
+ if (rfh->IsErrorDocument())
+ result.No(BackForwardCacheMetrics::NotRestoredReason::kErrorDocument);
+
// Only store documents that were fetched via HTTP GET method.
if (rfh->last_http_method() != net::HttpRequestHeaders::kGetMethod)
result.No(BackForwardCacheMetrics::NotRestoredReason::kHTTPMethodNotGET);
diff --git a/content/browser/renderer_host/back_forward_cache_metrics.h b/content/browser/renderer_host/back_forward_cache_metrics.h
index 05289d0ab65faf2939f74d76530091b8a9efd123..c2338608889ce4936c3a8b00833a5205c74a78af 100644
--- a/content/browser/renderer_host/back_forward_cache_metrics.h
+++ b/content/browser/renderer_host/back_forward_cache_metrics.h
@@ -110,7 +110,8 @@ class BackForwardCacheMetrics
kCacheControlNoStoreHTTPOnlyCookieModified = 55,
kNoResponseHead = 56,
kActivationNavigationsDisallowedForBug1234857 = 57,
- kMaxValue = kActivationNavigationsDisallowedForBug1234857,
+ kErrorDocument = 58,
+ kMaxValue = kErrorDocument,
};
using NotRestoredReasons =
diff --git a/third_party/blink/public/devtools_protocol/browser_protocol.pdl b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
index 0ae53ce429dbc1bf254ac773e0499dfe07273d46..d7231d7cdf9c230b24cb661a86bd64da5d6e7eb8 100644
--- a/third_party/blink/public/devtools_protocol/browser_protocol.pdl
+++ b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
@@ -7988,6 +7988,7 @@ domain Page
NoResponseHead
Unknown
ActivationNavigationsDisallowedForBug1234857
+ ErrorDocument
#Blocklisted features
WebSocket
WebTransport