New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 784904 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 882800
issue 809488

Blocking:
issue 872803



Sign in to add a comment

Merge navigation IPCs in a per-navigation Mojo interface.

Project Member Reported by clamy@chromium.org, Nov 14 2017

Issue description

Comment 1 Deleted

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01a8a155955ed905e39b1d83224a94b553b86caa

commit 01a8a155955ed905e39b1d83224a94b553b86caa
Author: Arthur Hemery <ahemery@chromium.org>
Date: Fri Jan 05 17:05:12 2018

Navigation: Extracting UpdateZoomLevel in its own function

This CL aims to provide a separate function that handles zoom update
during the commit process. Preparatory work for
https://chromium-review.googlesource.com/c/chromium/src/+/789857

BUG=784904

Change-Id: Ib52f04b08d66f0a28f46ac3665f94664bf063be2
Reviewed-on: https://chromium-review.googlesource.com/852294
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527300}
[modify] https://crrev.com/01a8a155955ed905e39b1d83224a94b553b86caa/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/01a8a155955ed905e39b1d83224a94b553b86caa/content/renderer/render_frame_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c613fa4b6be539b0e97933554dc1a24d18688f0

commit 2c613fa4b6be539b0e97933554dc1a24d18688f0
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 08 10:47:13 2018

Navigation: Extracting UpdateHistory in its own function

This CL aims to provide a separate function that handles history update
during the commit process. Preparatory work for
https://chromium-review.googlesource.com/c/chromium/src/+/789857

BUG=784904

Change-Id: Ia62eb3bad4f2a46aa19cdb1cf3d19522268e7182
Reviewed-on: https://chromium-review.googlesource.com/852454
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527603}
[modify] https://crrev.com/2c613fa4b6be539b0e97933554dc1a24d18688f0/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2c613fa4b6be539b0e97933554dc1a24d18688f0/content/renderer/render_frame_impl.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b94d3db41640d9128df08d11548f5f10c6a63af2

commit b94d3db41640d9128df08d11548f5f10c6a63af2
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 08 16:34:42 2018

Navigation: Extracting NotifyObservers in its own function

This CL aims to provide a separate function that handles notifying
observers of a RenderView that it is in the commit process. Preparatory
work for
https://chromium-review.googlesource.com/c/chromium/src/+/789857

BUG=784904

Change-Id: I72dd4a0bd85a9db1568a5358a495fb7ba990fd4e
Reviewed-on: https://chromium-review.googlesource.com/852144
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527643}
[modify] https://crrev.com/b94d3db41640d9128df08d11548f5f10c6a63af2/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b94d3db41640d9128df08d11548f5f10c6a63af2/content/renderer/render_frame_impl.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/268be3223f72880b7e5f9048bb1815300f7e5f68

commit 268be3223f72880b7e5f9048bb1815300f7e5f68
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 08 20:24:38 2018

Navigation: Extracting commit params creation in its own function

This CL aims to provide a separate function that handles parameter creation
during the commit process. Preparatory work for
https://chromium-review.googlesource.com/c/chromium/src/+/789857

BUG=784904

Change-Id: Ibf6b43a0c43a1b9d29ac5d60a9df79231ddf637f
Reviewed-on: https://chromium-review.googlesource.com/852496
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527738}
[modify] https://crrev.com/268be3223f72880b7e5f9048bb1815300f7e5f68/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/268be3223f72880b7e5f9048bb1815300f7e5f68/content/renderer/render_frame_impl.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c23999c08bd0b3a70f6397e26a66c0391b951ae7

commit c23999c08bd0b3a70f6397e26a66c0391b951ae7
Author: Arthur Hemery <ahemery@chromium.org>
Date: Thu Jan 11 19:29:10 2018

Navigation: Factoring state update during commit in RenderFrame

This CL aims to factor a function that does pre commit update
of the navigation state in the RenderFrameImpl. Preparatory work for
https://chromium-review.googlesource.com/c/chromium/src/+/789857

BUG=784904

Change-Id: I9d131ef23df6c48b3cb35e2136d52c7a235965d2
Reviewed-on: https://chromium-review.googlesource.com/854133
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528706}
[modify] https://crrev.com/c23999c08bd0b3a70f6397e26a66c0391b951ae7/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c23999c08bd0b3a70f6397e26a66c0391b951ae7/content/renderer/render_frame_impl.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c33801720c00e5ea75f26baa3fbd1041e7a09c3b

commit c33801720c00e5ea75f26baa3fbd1041e7a09c3b
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 22 14:00:17 2018

Navigation: Extend NavigationRequest lifetime up to Commit

This CL removes the NavigationHandle ownership transfer at
ReadyToCommit stage. The NavigationRequest will now stay active all the
way up to commit and will hold the NavigationHandle for the entire
duration.

BUG=784904

Change-Id: Idfec6a6d2025c9ab24a7b3013ec5ba10a8fd3870
Reviewed-on: https://chromium-review.googlesource.com/808974
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530853}
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/frame_tree_node.h
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/c33801720c00e5ea75f26baa3fbd1041e7a09c3b/content/test/test_render_frame_host.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1df7b96528838e2f6a317503648ff9600a40dac

commit b1df7b96528838e2f6a317503648ff9600a40dac
Author: clamy <clamy@chromium.org>
Date: Thu Feb 01 17:38:17 2018

Introduce a mojo method to send renderer debug URLS to the renderer

This CL introduces a mojo method to send renderer debug URLS to the renderer
for handling, instead of using CommitNavigation. This allows not to create a
NavigationRequest nor a NavigationHandle for these URLs, which do not represent
actual navigations.

Bug: 784904,803859
Change-Id: Ica9b17a071d2f9faa7f43eb428b0d73424a180e9
Reviewed-on: https://chromium-review.googlesource.com/817560
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533719}
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/common/frame.mojom
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/renderer/render_frame_impl.h
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/test/content_browser_test_test.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/content/test/test_render_frame_host.cc
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/third_party/WebKit/Source/core/frame/FrameTestHelpers.cpp
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.h
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/third_party/WebKit/public/web/WebLocalFrame.h
[modify] https://crrev.com/b1df7b96528838e2f6a317503648ff9600a40dac/tools/perf/core/stacktrace_unittest.py

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b0ae4937a81a11b2563ffc31dc7751d520284e1

commit 7b0ae4937a81a11b2563ffc31dc7751d520284e1
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Feb 05 16:04:45 2018

Navigation: Separate same document navigation DidCommit path.

This patch aims to split the currently unique path for same document and
cross document navigation DidCommit. We introduce a new FrameHost
interface method DidCommitSameDocumentNavigation while preserving the
existing DidCommitProvisionalLoad IPC. Same document navigation will use
the new mojo interface method while cross document navigation will keep
on using DidCommitProvisionalLoad.

This change is part of the broader goal of providing a clear mojo
interface for renderer/RFH navigation communication. It would solve
a number of complex race conditions. This specific change helps the
Browser process classify navigations as a same document. This is a step
forward unifying browser and renderer initiated same document navigation
code paths.

Bug:784904

Change-Id: Ie937ece170643ec2aa651c763df27fae3c150ac2
Reviewed-on: https://chromium-review.googlesource.com/789857
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534392}
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/bad_message.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/interstitial_page_navigator_impl.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/interstitial_page_navigator_impl.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigator.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/security_exploit_browsertest.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/common/frame.mojom
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/common/frame_messages.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/renderer/render_frame_impl.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/shell/test_runner/web_frame_test_client.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/shell/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/test/test_render_frame.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/test/test_render_frame_host.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/test/test_render_frame_host.h
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/test/test_render_view_host.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/content/test/test_web_contents.cc
[modify] https://crrev.com/7b0ae4937a81a11b2563ffc31dc7751d520284e1/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394

commit af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394
Author: clamy <clamy@chromium.org>
Date: Tue Feb 06 10:54:36 2018

Introduce a mojom::Frame::CommitSameDocumentNavigation

This CL introduces a CommitSameDocumentNavigation method in mojom::Frame, along
with a specific path to handle it in FrameLoader. Same document navigations are
handled very differently from different document navigations in the renderer,
and this simplifies the code path for their handling when the same-document
navigation is browser-initiated. This is a preparatory work for introducing
per-navigation Mojo interfaces. These interfaces will only be used for
different document navigations. Same-document navigations will still be handled
by the Frame directly, through the IPC this CL introduces.

This CL also fixes an issue with same-document navigations in out-of-process
iframes. Browser-initiated same-document navigations would see their
FrameLoadType recomputed at commit type, which sometimes caused them to be
wrongly classified as non same-document and dropped. By taking a same-document
path from the start, we ensure that FrameLoader doesn't recompute the
FrameLoadType for same-document navigations.

Bug:  788901 ,784904
Change-Id: Ie2fbef9afd9c99d22b7bb6d33170eaa5e32de622
Reviewed-on: https://chromium-review.googlesource.com/817296
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534675}
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/DEPS
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/common/frame.mojom
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/renderer/render_frame_impl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/content/test/test_render_frame_host.cc
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/loader/FrameLoader.cpp
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/Source/core/loader/FrameLoader.h
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/web/WebLocalFrame.h
[add] https://crrev.com/af4bf2d9fdaa6ca56ac275c2a53b85f9cf51c394/third_party/WebKit/public/web/commit_result.mojom

Comment 11 by kbr@chromium.org, Feb 7 2018

Blockedon: 809488
Project Member

Comment 12 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10e551559d87ac1241109bb5de49f144390938a4

commit 10e551559d87ac1241109bb5de49f144390938a4
Author: Arthur Hemery <ahemery@chromium.org>
Date: Wed May 16 10:07:14 2018

Navigation: Add the per-navigation-mojo-interface flag.

This CL introduces a new flag to enable the use of per navigations mojo
interfaces and allow for progressive rollout of the feature. See bug
for the feature design doc.

* Add content::feature PerNavigationMojoInterface

Bug: 784904
Change-Id: I5bb97a9ea0dbec5b34fe3d247d0c40f05a145fd7
Reviewed-on: https://chromium-review.googlesource.com/1057674
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559029}
[modify] https://crrev.com/10e551559d87ac1241109bb5de49f144390938a4/content/public/common/browser_side_navigation_policy.cc
[modify] https://crrev.com/10e551559d87ac1241109bb5de49f144390938a4/content/public/common/browser_side_navigation_policy.h
[modify] https://crrev.com/10e551559d87ac1241109bb5de49f144390938a4/content/public/common/content_features.cc
[modify] https://crrev.com/10e551559d87ac1241109bb5de49f144390938a4/content/public/common/content_features.h
[modify] https://crrev.com/10e551559d87ac1241109bb5de49f144390938a4/tools/metrics/histograms/enums.xml

Project Member

Comment 13 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7

commit 512fcf511f65a7cf3dd454c66513b5d2dbc18dd7
Author: Arthur Hemery <ahemery@chromium.org>
Date: Thu May 24 16:20:53 2018

Navigation: Recycling commit verification in tests

Currently tests that check that a RFH tried to commit in unit tests used
MockRenderProcessHost::did_frame_commit_navigation(). This was set by
intercepting the interface commit using
TestRenderFrameHost::NavigationInterceptor. This is problematic because in the
future the RFH will not be the one holding the interface directly, making it
harder to use this mechanism. Instead, we just check that the RFH has a
navigation request, which is equivalent to it trying to commit.

We also remove TestRenderFrameHost::pending_commit() that was never used and
now redundant.

Bug: 784904
Change-Id: I7ea3c1410d70f9e164ea700a257e167d6ce3ae8c
Reviewed-on: https://chromium-review.googlesource.com/1071608
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561518}
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/browser/frame_host/navigator_impl_unittest.cc
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/browser/frame_host/render_frame_host_manager_unittest.cc
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/test/test_render_frame_host.cc
[modify] https://crrev.com/512fcf511f65a7cf3dd454c66513b5d2dbc18dd7/content/test/test_render_frame_host.h

Project Member

Comment 14 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3011f6d2cb2b4ea9c8c28af0f8446006e808181

commit d3011f6d2cb2b4ea9c8c28af0f8446006e808181
Author: Arthur Hemery <ahemery@chromium.org>
Date: Wed May 30 10:38:44 2018

Navigation: Introducing the NavigationClient mojo interface.

This CL introduces a new interface called NavigationClient,
implemented on the renderer side. This interface should allow to
solve a number of race conditions as well as enabling
other things. See the related BUG for more information on this.

As a first step we replace two IPCs by reworking BeginNavigation.
The idea is to send an interface pointer over BeginNavigation
to allow for further control of the navigation.

An important part of this CL is getting the proper lifetime for the
Interface. We think that provisional_document_loader_ in the
FrameLoader is a good candidate to own the binding. On the browser
side,the NavigationRequest is in charge of holding the interface
endpoint.

BeginNavigation gets a new parameter: the AssociatedPtrInfo of the
transmitted interface. FrameMsg_DroppedNavigation is replaced by the
browser closing the pipe. FrameHostMsg_AbortNavigation is replaced by
the renderer closing the pipe.

The new interface duplicates the CommitNavigation and
CommitFailedNavigation methods of the FrameNavigationControl interface.
These new methods are used instead of the old interface ones for
navigations that originated from a BeginNavigation. In future patches,
it will be a complete replacement for both renderer and browser
initiated navigations.

Note: This patch is fully behind the PerNavigationMojoInterface flag.

Please see
https://chromium-review.googlesource.com/c/chromium/src/+/1072469
for the trybot runs with the PerNavigationMojoInterface flag on.

Bug: 784904
Change-Id: Ifca1b3f6bdab1b3988f867c7cce44b7292e6668e
Reviewed-on: https://chromium-review.googlesource.com/881043
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562790}
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/browser_side_navigation_browsertest.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigator.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigator.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/navigator_impl.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/common/BUILD.gn
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/common/frame.mojom
[add] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/common/navigation_client.mojom
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/public/test/navigation_simulator.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/BUILD.gn
[add] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/navigation_client.cc
[add] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/navigation_client.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/navigation_state_impl.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/navigation_state_impl.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/renderer/render_frame_impl.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/BUILD.gn
[add] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/mock_navigation_client_impl.cc
[add] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/mock_navigation_client_impl.h
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/test_render_frame.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/test_render_frame_host.cc
[modify] https://crrev.com/d3011f6d2cb2b4ea9c8c28af0f8446006e808181/content/test/test_render_frame_host.h

Components: UI>Browser>Navigation
Blocking: 872803
Blockedon: 882800
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a35275fad262da570a3125572731679616324142

commit a35275fad262da570a3125572731679616324142
Author: Arthur Hemery <ahemery@chromium.org>
Date: Fri Jan 04 11:39:09 2019

Navigation: Replacing interface mocking by simple overrides.

After a long struggle to try and make mock interfaces for navigations,
(attempt here https://chromium-review.googlesource.com/c/chromium/src/+/1366334),
finally settling for a more direct approach that enables finer grain management
of callbacks.

By wrapping interface calls in functions in the RFHI we are able to override it
in the TRFH and have no reliance on mocks. Their only use would have been to
manage disconnections natively, but we can, in the rare cases where we need it
call the implementations directly.

In practice:
- 2 new functions wrapper in RFHI
- 2 overrides of these functions in the TRFH
- 4 storage spaces in TRFH
- The same unified callback method in NavigationSimulator
With this, we know what navigation interface was called, and what callback was
provided.

Note: Might implement a structure wrapping all Commit parameters in the future,
because this is getting quite nasty to change a simple parameter in there.

Bug: 784904
Change-Id: I46dc45e3ada1bc00120f1f45f84f1edb8d43906d
Reviewed-on: https://chromium-review.googlesource.com/c/1386104
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619912}
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/public/test/navigation_simulator.h
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/test/BUILD.gn
[delete] https://crrev.com/e107337ecaa8f8fd6daaf189b3d2c4712479490d/content/test/mock_navigation_client_impl.cc
[delete] https://crrev.com/e107337ecaa8f8fd6daaf189b3d2c4712479490d/content/test/mock_navigation_client_impl.h
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/test/test_render_frame_host.cc
[modify] https://crrev.com/a35275fad262da570a3125572731679616324142/content/test/test_render_frame_host.h

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e079d2fd07af621a233c993050158dc82a17b7e

commit 2e079d2fd07af621a233c993050158dc82a17b7e
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 07 15:45:45 2019

Navigation: Commit browser initiated requests through NavigationClient.

This CL introduces the possibility for the NavigationRequest to lazily
initialize a NavigationClient mojo interface when it does not have one.
To do so, it will get one through the remote interface provider in the
RenderFrameImpl.

This is will allow browser initiated navigations (includes cross-site
renderer initiated navigations) to commit via the NavigationClient
interface instead of the FrameNavigationControl currently used.

Note: This patch is fully behind the PerNavigationMojoInterface flag.

Please see
https://chromium-review.googlesource.com/c/chromium/src/+/1072476
for the trybot runs with the PerNavigationMojoInterface flag on.

Please see
https://docs.google.com/document/d/1mXjxYJptb_bZ_EqGMF-c4LTSnhjt6Gn_WVvSrsinpq8/edit
for the design doc.

Bug: 784904
Change-Id: Iffb65ee1849dd523dbceb63b014a53b57e85f1e5
Reviewed-on: https://chromium-review.googlesource.com/c/1072475
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620330}
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/navigation_request.h
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/renderer/navigation_state.cc
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/renderer/navigation_state.h
[modify] https://crrev.com/2e079d2fd07af621a233c993050158dc82a17b7e/content/renderer/render_frame_impl.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46833eabf321c7976c64523eab26ddc8e0797063

commit 46833eabf321c7976c64523eab26ddc8e0797063
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 07 16:10:59 2019

Navigation: Fix to Interface mocking through overrides.

Patch 1386104 removed NavigationClientRequest storage in the
NavigationSimulator. This is actually necessary to avoid getting
interface disconnect messages aborting ongoing navigations.

Went through and was only discovered in other CL tries with the
PerNavigationMojoInterface flag on.

Bug: 784904
Change-Id: Ib50c6bd4178445a5eab7424f4adb277fccdb9234
Reviewed-on: https://chromium-review.googlesource.com/c/1397615
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620336}
[modify] https://crrev.com/46833eabf321c7976c64523eab26ddc8e0797063/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/46833eabf321c7976c64523eab26ddc8e0797063/content/public/test/navigation_simulator.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c03573d4a5702513923d9322692c4fe1c6a30a4

commit 9c03573d4a5702513923d9322692c4fe1c6a30a4
Author: Arthur Hemery <ahemery@chromium.org>
Date: Mon Jan 07 16:12:41 2019

Navigation: Move DidCommit params to its own mojo file to be reusable.

DidCommitProvisionalParams has its own typemap file, we create the
matching .mojom file. This will be reused by the NavigationClient
interface in its callback. The file could also hold similar other types
that need to be kept synced between the two interfaces.

See use in
https://chromium-review.googlesource.com/c/chromium/src/+/1344091

Bug: 784904
Change-Id: I836248be9a069d252ee16d8f0d2bb451f94ce869
Reviewed-on: https://chromium-review.googlesource.com/c/1393325
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620339}
[modify] https://crrev.com/9c03573d4a5702513923d9322692c4fe1c6a30a4/content/common/BUILD.gn
[modify] https://crrev.com/9c03573d4a5702513923d9322692c4fe1c6a30a4/content/common/frame.mojom
[add] https://crrev.com/9c03573d4a5702513923d9322692c4fe1c6a30a4/content/common/frame_messages.mojom
[modify] https://crrev.com/9c03573d4a5702513923d9322692c4fe1c6a30a4/content/common/frame_messages.typemap

Project Member

Comment 22 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3226d212641c01a25452f9fb5d3f595ffa609410

commit 3226d212641c01a25452f9fb5d3f595ffa609410
Author: Arthur Hemery <ahemery@chromium.org>
Date: Tue Jan 08 15:57:38 2019

Navigation: NavigationSimulator delegates DidCommit calls to TestRFH.

TestRenderFrameHost handles the callbacks it receives from SendCommit*
transparently to the NavigationSimulator. It should also ideally handle
DidCommit messages. This will allow the NavigationSimulator to have a
single call point for all commits, and simplify future modifications
in interface callbacks.

Bug: 784904
Change-Id: I7697a337a274aa8fa21a047adfe3f9f2fd545ae8
Reviewed-on: https://chromium-review.googlesource.com/c/1394304
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620739}
[modify] https://crrev.com/3226d212641c01a25452f9fb5d3f595ffa609410/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/3226d212641c01a25452f9fb5d3f595ffa609410/content/public/test/navigation_simulator.h
[modify] https://crrev.com/3226d212641c01a25452f9fb5d3f595ffa609410/content/test/test_render_frame_host.cc
[modify] https://crrev.com/3226d212641c01a25452f9fb5d3f595ffa609410/content/test/test_render_frame_host.h

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 18 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c8c00e1e2c82bb02e77bb46072206f07fb1835c

commit 7c8c00e1e2c82bb02e77bb46072206f07fb1835c
Author: Arthur Hemery <ahemery@chromium.org>
Date: Fri Jan 18 10:51:55 2019

Navigation: Pass NavigationRequest directly to commit.

Currently we pass the navigation_id to the RenderFrameHostImpl and
then retrieve the NavigationRequest from the storage map. The
navigation_id is extracted from the NavigationRequest in the first
place so it does not make sense. The corner case where we don't have
a NavigationRequest (for certain interstitials), is simply checked for,
the same way we had a not found in map case.

Bug: 784904
Change-Id: I2671a10c4b4ff395af84ffa80c9896ea3a24d4d2
Reviewed-on: https://chromium-review.googlesource.com/c/1416255
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624063}
[modify] https://crrev.com/7c8c00e1e2c82bb02e77bb46072206f07fb1835c/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/7c8c00e1e2c82bb02e77bb46072206f07fb1835c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/7c8c00e1e2c82bb02e77bb46072206f07fb1835c/content/browser/frame_host/render_frame_host_impl.h

Sign in to add a comment