Assigned
Status Update
No update yet.
Comments
bu...@chromium.org
The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src.git/+/c717927a3712cf6391962971e0e4084a8f159ba0
commit c717927a3712cf6391962971e0e4084a8f159ba0
Author: creis <creis@chromium.org>
Date: Wed May 04 03:35:58 2016
Don't use pending NavigationEntries for navigation transfers (try #2).
Cross-process transfers for navigations that are about to commit
had been using NavigationControllerImpl::LoadURLWithParams (as
an artifact of previously going through OpenURL). Most of that
code is unnecessary, in particular the fact that it creates a
new pending NavigationEntry. That's problematic for subframe
transfers, where we should not affect any existing pending
NavigationEntry (e.g., for a slow main frame navigation).
This CL shortcuts the transfer by creating a NavigationEntry
without making it the pending one. A future CL can eliminate
the entry entirely by validating and passing the parameters
to the right RenderFrameHost directly.
This also makes it possible to do cross-process navigations
in subframes of the initial blank page, fixing a crash.
This CL is a second attempt that fixes crashes seen after
https://crrev.com/384009 , and adds tests for those cases.
It works aroundhttps://crbug.com/608402 for the time being.
BUG=495161, 584739, 608402
TEST=Create OOPIF on initial blank page of a tab.
TEST=Cross-process subframe navigation during slow main frame navigation.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
Review-Url:https://codereview.chromium.org/1871293002
Cr-Commit-Position: refs/heads/master@{#391439}
[modify]https://crrev.com/c717927a3712cf6391962971e0e4084a8f159ba0/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify]https://crrev.com/c717927a3712cf6391962971e0e4084a8f159ba0/content/browser/frame_host/navigator_impl.cc
commit c717927a3712cf6391962971e0e4084a8f159ba0
Author: creis <creis@chromium.org>
Date: Wed May 04 03:35:58 2016
Don't use pending NavigationEntries for navigation transfers (try #2).
Cross-process transfers for navigations that are about to commit
had been using NavigationControllerImpl::LoadURLWithParams (as
an artifact of previously going through OpenURL). Most of that
code is unnecessary, in particular the fact that it creates a
new pending NavigationEntry. That's problematic for subframe
transfers, where we should not affect any existing pending
NavigationEntry (e.g., for a slow main frame navigation).
This CL shortcuts the transfer by creating a NavigationEntry
without making it the pending one. A future CL can eliminate
the entry entirely by validating and passing the parameters
to the right RenderFrameHost directly.
This also makes it possible to do cross-process navigations
in subframes of the initial blank page, fixing a crash.
This CL is a second attempt that fixes crashes seen after
It works around
BUG=495161, 584739, 608402
TEST=Create OOPIF on initial blank page of a tab.
TEST=Cross-process subframe navigation during slow main frame navigation.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
Review-Url:
Cr-Commit-Position: refs/heads/master@{#391439}
[modify]
[modify]
wj...@chromium.org
[Empty comment from Monorail migration]
wj...@chromium.org
[Empty comment from Monorail migration]
wj...@chromium.org
With the work on session-history and frame-tree traversals for isolated origins, this issue is no longer a blocker for 1042415.
al...@chromium.org
I recently encountered another case similar to repro 1 in https://crbug.com/chromium/608402#c0 where a subframe with a javascript URL doesn't create a FrameNavigationEntry in https://crbug.com/chromium/1090125 , so I'll mark that as a blocker here.
ck...@chromium.org
[Empty comment from Monorail migration]
is...@google.com
This issue was migrated from crbug.com/chromium/608402?no_tracker_redirect=1
[Auto-CCs applied]
[Multiple monorail components: Internals>Sandbox>SiteIsolation, UI>Browser>Navigation]
[Monorail blocked-on:crbug.com/chromium/1090125 ]
[Monorail blocking:crbug.com/chromium/1062719 , crbug.com/chromium/1099264 ]
[Monorail components added to Component Tags custom field.]
[Auto-CCs applied]
[Multiple monorail components: Internals>Sandbox>SiteIsolation, UI>Browser>Navigation]
[Monorail blocked-on:
[Monorail blocking:
[Monorail components added to Component Tags custom field.]
ap...@google.com
Project: chromium/src
Branch: main
commit f21cd1873e01568ca7a58bd8a130faa6ad144e13
Author: Charlie Reis <creis@chromium.org>
Date: Fri Aug 09 21:44:52 2024
Support null frame_entry cases in CreateNavigationRequestFromLoadParams.
It is sometimes possible to reach CreateNavigationRequestFromLoadParams
for subframes, such as when loading a PDF in a subframe (in which case
the PDF extension is loaded via NavigationController::LoadURL).
Perhttps://crbug.com/40467594 , there are some cases when the
FrameNavigationEntry will be missing for subframes, which is causing
crashes in CreateNavigationRequestFromLoadParams. This CL adds support
for having a null frame_entry, untilhttps://crbug.com/40467594 is
fixed.
Bug: 358084015, 40467594
Change-Id: I615316a812a846f691a0c13513d4fb5c3097c975
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/5775515
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1339887}
M chrome/browser/pdf/pdf_extension_test.cc
M content/browser/renderer_host/navigation_controller_impl.cc
https://chromium-review.googlesource.com/5775515
Branch: main
commit f21cd1873e01568ca7a58bd8a130faa6ad144e13
Author: Charlie Reis <creis@chromium.org>
Date: Fri Aug 09 21:44:52 2024
Support null frame_entry cases in CreateNavigationRequestFromLoadParams.
It is sometimes possible to reach CreateNavigationRequestFromLoadParams
for subframes, such as when loading a PDF in a subframe (in which case
the PDF extension is loaded via NavigationController::LoadURL).
Per
FrameNavigationEntry will be missing for subframes, which is causing
crashes in CreateNavigationRequestFromLoadParams. This CL adds support
for having a null frame_entry, until
fixed.
Bug: 358084015, 40467594
Change-Id: I615316a812a846f691a0c13513d4fb5c3097c975
Reviewed-on:
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1339887}
M chrome/browser/pdf/pdf_extension_test.cc
M content/browser/renderer_host/navigation_controller_impl.cc
ap...@google.com
Project: chromium/src
Branch: refs/branch-heads/6613
commit c9359897e4481eb66fcf00b7fd0f4e2e80bb6510
Author: Charlie Reis <creis@chromium.org>
Date: Mon Aug 12 20:09:27 2024
[M128] Support null frame_entry cases in CreateNavigationRequestFromLoadParams.
It is sometimes possible to reach CreateNavigationRequestFromLoadParams
for subframes, such as when loading a PDF in a subframe (in which case
the PDF extension is loaded via NavigationController::LoadURL).
Perhttps://crbug.com/40467594 , there are some cases when the
FrameNavigationEntry will be missing for subframes, which is causing
crashes in CreateNavigationRequestFromLoadParams. This CL adds support
for having a null frame_entry, untilhttps://crbug.com/40467594 is
fixed.
(cherry picked from commit f21cd1873e01568ca7a58bd8a130faa6ad144e13)
Bug: 358084015, 40467594
Change-Id: I615316a812a846f691a0c13513d4fb5c3097c975
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/5775515
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1339887}
Reviewed-on:https://chromium-review.googlesource.com/c/chromium/src/+/5782222
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6613@{#962}
Cr-Branched-From: 03c1799e6f9c7239802827eab5e935b9e14fceae-refs/heads/main@{#1331488}
M chrome/browser/pdf/pdf_extension_test.cc
M content/browser/renderer_host/navigation_controller_impl.cc
https://chromium-review.googlesource.com/5782222
Branch: refs/branch-heads/6613
commit c9359897e4481eb66fcf00b7fd0f4e2e80bb6510
Author: Charlie Reis <creis@chromium.org>
Date: Mon Aug 12 20:09:27 2024
[M128] Support null frame_entry cases in CreateNavigationRequestFromLoadParams.
It is sometimes possible to reach CreateNavigationRequestFromLoadParams
for subframes, such as when loading a PDF in a subframe (in which case
the PDF extension is loaded via NavigationController::LoadURL).
Per
FrameNavigationEntry will be missing for subframes, which is causing
crashes in CreateNavigationRequestFromLoadParams. This CL adds support
for having a null frame_entry, until
fixed.
(cherry picked from commit f21cd1873e01568ca7a58bd8a130faa6ad144e13)
Bug: 358084015, 40467594
Change-Id: I615316a812a846f691a0c13513d4fb5c3097c975
Reviewed-on:
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1339887}
Reviewed-on:
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6613@{#962}
Cr-Branched-From: 03c1799e6f9c7239802827eab5e935b9e14fceae-refs/heads/main@{#1331488}
M chrome/browser/pdf/pdf_extension_test.cc
M content/browser/renderer_host/navigation_controller_impl.cc
Description
OS: All
There are several cases in which we do not create subframe FrameNavigationEntries in OOPIF-enabled modes of Chrome. This mostly matches the behavior of HistoryController in default Chrome, where WebHistoryItems are not tracked for these frames. In practice this means we cannot go back/forward in these subframes.
This case also led to crashes in
Repro 1: No commit in parent.
1) Create an iframe on a page that does not commit (e.g.,
2) Create a nested iframe inside it (e.g., to
3) Click the link to simple.html.
Bug: Going back reloads the main frame rather than going back in the subframe.
Note: Same thing happens with a slow URL in the parent, rather than a 204 response. Same for download.
Repro 2: No record of the parent.
1) Do an in-page navigation to #foo on a page.
2) Create an iframe on the page (e.g.,
3) Go back.
4) Create a nested iframe inside the iframe (e.g., to
5) Click the link to simple.html.
Bug: Incorrectly triggers the mixed content blocker. If you use HTTP URLs instead, then going back reloads the main frame rather than going back in the subframe.
Repro 3: Change name of parent.
1) Create an iframe on the page (e.g.,
2) Change the name of the iframe (
3) Create a nested iframe inside the iframe (e.g., to
4) Click the link to simple.html.
Bug: Going back reloads the main frame rather than going back in the subframe.
These all have the same root cause: not finding the parent frame's FrameNavigationEntry, and thus not having anywhere to put the subframe's FrameNavigationEntry. See also