Skip to content

Commit

Permalink
MPArch: FencedFrame should not change the current page quality
Browse files Browse the repository at this point in the history
This CL makes RenderTabHelper::DidFinishNavigation method only work
on the primary main frame. To ensure that, this CL adds a test to
check if the current page quality of SnapshotController is not
changed by a fenced frame's navigation. Because DidFinishNavigation
should not reset the current page quality in FencedFrame.

TEST: RecentTabHelperFencedFrameTest.
      FencedFrameDoesNotChangePageQuality

Bug: 1311529
Change-Id: I1ef0122ac4054af1408d46e06fa4a2ff879dbb56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3559673
Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/main@{#988248}
  • Loading branch information
Gyuyoung authored and Chromium LUCI CQ committed Apr 2, 2022
1 parent 8c4ad7c commit 8f50819
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
6 changes: 3 additions & 3 deletions chrome/browser/offline_pages/recent_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ bool RecentTabHelper::EnsureInitialized() {

void RecentTabHelper::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
if (!navigation_handle->IsInMainFrame() ||
if (!navigation_handle->IsInPrimaryMainFrame() ||
!navigation_handle->HasCommitted() ||
navigation_handle->IsSameDocument()) {
DVLOG_IF(1, navigation_handle->IsInMainFrame())
<< "Main frame navigation ignored (reasons: "
DVLOG_IF(1, navigation_handle->IsInPrimaryMainFrame())
<< "Primary main frame navigation ignored (reasons: "
<< !navigation_handle->HasCommitted() << ", "
<< navigation_handle->IsSameDocument()
<< ") to: " << web_contents()->GetLastCommittedURL().spec();
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/offline_pages/recent_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ class RecentTabHelper
const std::string& origin);

private:
FRIEND_TEST_ALL_PREFIXES(RecentTabHelperFencedFrameTest,
FencedFrameDoesNotChangePageQuality);

struct SnapshotProgressInfo;

explicit RecentTabHelper(content::WebContents* web_contents);
Expand Down
50 changes: 50 additions & 0 deletions chrome/browser/offline_pages/recent_tab_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/test/navigation_simulator.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"

namespace offline_pages {

Expand Down Expand Up @@ -1110,4 +1111,53 @@ TEST_F(RecentTabHelperTest, NoSaveOfflinePageCacheForPost) {
ASSERT_EQ(1U, GetAllPages().size());
}

class RecentTabHelperFencedFrameTest : public RecentTabHelperTest {
public:
RecentTabHelperFencedFrameTest() {
scoped_feature_list_.InitAndEnableFeatureWithParameters(
blink::features::kFencedFrames, {{"implementation_type", "mparch"}});
}
~RecentTabHelperFencedFrameTest() override = default;

content::RenderFrameHost* CreateFencedFrame(
content::RenderFrameHost* parent) {
content::RenderFrameHost* fenced_frame =
content::RenderFrameHostTester::For(parent)->AppendFencedFrame();
return fenced_frame;
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

// Tests that FencedFrame does not change the current page quality via resetting
// the snapshot controller.
TEST_F(RecentTabHelperFencedFrameTest, FencedFrameDoesNotChangePageQuality) {
// Navigate and finish loading, then move the snapshot controller's time
// forward so it sets the current page quality to FAIR_AND_IMPROVING.
NavigateAndCommitPost(GURL("http://mystery.site/foo.html"));

recent_tab_helper()->PrimaryMainDocumentElementAvailable();
FastForwardSnapshotController();

EXPECT_EQ(recent_tab_helper()->snapshot_controller_->current_page_quality(),
SnapshotController::PageQuality::FAIR_AND_IMPROVING);

// Create a fenced frame.
content::RenderFrameHostTester::For(main_rfh())
->InitializeRenderFrameIfNeeded();
content::RenderFrameHost* fenced_frame_rfh = CreateFencedFrame(main_rfh());
GURL kFencedFrameUrl("https://fencedframe.com");
std::unique_ptr<content::NavigationSimulator> navigation_simulator =
content::NavigationSimulator::CreateForFencedFrame(kFencedFrameUrl,
fenced_frame_rfh);
navigation_simulator->Commit();
EXPECT_TRUE(fenced_frame_rfh->IsFencedFrameRoot());

// Navigating the fenced frame to the fenced frame url should not change the
// current page quality to POOR.
EXPECT_EQ(recent_tab_helper()->snapshot_controller_->current_page_quality(),
SnapshotController::PageQuality::FAIR_AND_IMPROVING);
}

} // namespace offline_pages

0 comments on commit 8f50819

Please sign in to comment.