Skip to content

Commit

Permalink
[m103] DevTools: omit loaderId in Page.navigate response when navigat…
Browse files Browse the repository at this point in the history
…ing same document

... because that's how the client is supposed to tell same-document navigation.
Reporting loaderId also doesn't make sense in this case because actual renderer
side loader id is going to be different (i.e. remain from the actual navigation
that caused the document to be loaded).

This was originally regressed by https://crrev.com/c/3640544

(cherry picked from commit 76d3945)

Bug: 1325782, 1324138
Change-Id: I4c528a55991b236394c68735b3c6319e350d1559
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3649348
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003895}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3653099
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Reviewed-by: Peter Kvitek <kvitekp@chromium.org>
Cr-Commit-Position: refs/branch-heads/5060@{#86}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
caseq authored and Chromium LUCI CQ committed May 18, 2022
1 parent e4723d1 commit ecf3586
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 4 deletions.
8 changes: 5 additions & 3 deletions content/browser/devtools/protocol/page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,11 @@ void DispatchNavigateCallback(
Maybe<std::string> opt_error;
if (request->GetNetErrorCode() != net::OK)
opt_error = net::ErrorToString(request->GetNetErrorCode());
callback->sendSuccess(frame_id,
request->devtools_navigation_token().ToString(),
std::move(opt_error));
Maybe<std::string> loader_id =
request->IsSameDocument()
? Maybe<std::string>()
: request->devtools_navigation_token().ToString();
callback->sendSuccess(frame_id, std::move(loader_id), std::move(opt_error));
}

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7634,7 +7634,8 @@ domain Page
returns
# Frame id that has navigated (or failed to navigate)
FrameId frameId
# Loader identifier.
# Loader identifier. This is omitted in case of same-document navigation,
# as the previously committed loaderId would not change.
optional Network.LoaderId loaderId
# User friendly error message, present if and only if navigation has failed.
optional string errorText
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
(async function(testRunner) {
const {dp} = await testRunner.startBlank(
`Tests that Page.navigate within same document does not report loaderId`);

await dp.Page.enable();
const result = (await dp.Page.navigate({url: testRunner.url('../resources/inspector-protocol-page.html#foo')})).result;
// Assure the above is same-document navigation.
await dp.Page.onceNavigatedWithinDocument();
testRunner.log(`loaderId (undefined expected): ${result.loaderId}`);
testRunner.completeTest();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Tests that Page.navigate within same document does not report loaderId
loaderId (undefined expected): undefined

0 comments on commit ecf3586

Please sign in to comment.