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

Issue 882800 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 784904



Sign in to add a comment

Decouple DocumentLoader from WebDocumentLoader

Project Member Reported by ahemery@google.com, Sep 11

Issue description

We currently use a dummy DocumentLoader when we are starting a navigation from the renderer side, which sole purpose is to store some state as ExtraData.

This is causing issues and complexity with the introduction of the new NavigationClient interface lifetime.

We are proposing to make FrameLoader be responsible for the navigation state instead. This will both simplify the NavigationClient handling as well as enable a complete removal of the "Provisional DocumentLoader".

Link to design doc:
https://docs.google.com/document/d/1ZiVoD57tViO39cmuYK3HZcS-T3sVT_O8ABWBUns0woM/edit?usp=sharing

 
Description: Show this description
Description: Show this description
Blocking: 784904
Labels: -Type-Feature Type-Task
Cc: dgozman@chromium.org dcheng@chromium.org
Components: UI>Browser>Navigation
Labels: -OS-Linux -Via-Wizard-Other
Owner: ahemery@chromium.org
Status: Assigned (was: Unconfirmed)
Description: Show this description
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 11

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

commit 24436c3d1abceb29cbd4a2a0dbc3cef9fa79a63d
Author: Arthur Hemery <ahemery@chromium.org>
Date: Tue Sep 11 16:55:28 2018

Removing non-functional skip in WillSendRequest.

If condition used pending_navigation_params_ that is always false there.
Usage might have been corrupted during refactorings without being noticed,
or was never working.

Skip was removed, CHECK added as well as a TODO to reintroduce the
functionality.

Bug: 882800
Change-Id: I0cafc9dda082a395d48783cac03ec0d25a1ec73e
Reviewed-on: https://chromium-review.googlesource.com/1202462
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590352}
[modify] https://crrev.com/24436c3d1abceb29cbd4a2a0dbc3cef9fa79a63d/content/renderer/render_frame_impl.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 11

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

commit 53a4eba99f7fcbc9a9fe96f5896e69ede00c5aa3
Author: Arthur Hemery <ahemery@chromium.org>
Date: Tue Sep 11 17:27:56 2018

Rework of CommitFailedNavigation.

Allowed the navigation data and params to be passed directly to
DidFailProvisionalLoadInternal, instead of revolving around obscure
pending_navigation_params_ management.

Explicited the forwarding to observers without relying on nullptr
DocumentLoader in DidFailProvisionalLoadInternal.

Finally, modified a test expectation that would otherwise cause a very
ugly hack to become necessary.

Bug: 882800
Change-Id: I6ba713939a1ed192e1212e888323b55b1c6e6af5
Reviewed-on: https://chromium-review.googlesource.com/1169464
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590371}
[modify] https://crrev.com/53a4eba99f7fcbc9a9fe96f5896e69ede00c5aa3/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/53a4eba99f7fcbc9a9fe96f5896e69ede00c5aa3/content/renderer/render_frame_impl.h
[modify] https://crrev.com/53a4eba99f7fcbc9a9fe96f5896e69ede00c5aa3/third_party/WebKit/LayoutTests/http/tests/loading/bad-server-subframe-expected.txt

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 12

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

commit c21b935f67e16d8c8fa6ab64922ed6257b1c8c9a
Author: Arthur Hemery <ahemery@chromium.org>
Date: Wed Sep 12 19:26:20 2018

Removing PendingNavigationParams

All the meaningful usages of PendingNavigationParams were removed
in previous patches, only remaining usages are for creating
DocumentState, which we can do directly instead of going through
temporary parameters.

Bug: 882800
Change-Id: I44550cdbe1125253fb59ab7b25c4cc4cd064e5d1
Reviewed-on: https://chromium-review.googlesource.com/1219890
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590784}
[modify] https://crrev.com/c21b935f67e16d8c8fa6ab64922ed6257b1c8c9a/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c21b935f67e16d8c8fa6ab64922ed6257b1c8c9a/content/renderer/render_frame_impl.h

Sign in to add a comment