Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store |initiator_origin| in FrameNavigationEntry. #17497

Merged
merged 1 commit into from Jul 12, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jun 25, 2019

Changes in this CL

This CL:

  1. Updates FrameNavigationEntry::UpdateEntry and
    FrameNavigationEntry's constructor so that they both take
    |const base::Optional<url::Origin>& initiator_origin| which
    gets stored in a new FrameNavigationEntry::initiator_origin_ field.

  2. Updates callers of FNE::UpdateEntry and FNE's constructor to
    provide/propagate the initiator as needed. This includes
    adding an |initiator_origin| parameter to

    • NavigationEntryImpl's constructor
    • NavigationEntryImpl::AddOrUpdateFrameEntry
    • NavigationController::CreateNavigationEntry
      (the list above is not necessarily exhaustive/complete)
  3. Uses the new |FrameNavigationEntry::initiator_origin()| from
    NavigationEntryImpl::ConstructCommonNavigationParams (which
    used to always provide |base::nullopt| initiator for history
    navigations - always treating them as browser-initiated, rather
    than replaying the original initiator).

The changes above makes sure that the right Sec-Fetch-Site http request header is
"replayed" during history navigations. The CL adds browser tests and
WPT tests to cover the new, desired behavior.

Follow-up changes

This CL does not:

  • Use |FrameNavigationEntry::initiator_origin()| in GetOriginForURLLoaderFactory
    in render_frame_host_impl.cc (this will be done in a follow-up CL at
    https://crrev.com/c/1672176)

  • Handle persisting |FrameNavigationEntry::initiator_origin()| for
    session restore (this is tracked in a separate https://crbug.com/976055).

Bug: 946503
Change-Id: I4f92614873a5ec8d4b52d3ae22031c7089d87ed5
Tbr: skuhne@chromium.org for //components/sessions
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662738
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677046}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1662738 branch 4 times, most recently from 3f4e749 to b83e99d Compare July 2, 2019 21:45
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1662738 branch 6 times, most recently from 1e9c02e to d416b33 Compare July 12, 2019 18:49
Changes in this CL
==================

This CL:

1. Updates FrameNavigationEntry::UpdateEntry and
   FrameNavigationEntry's constructor so that they both take
   |const base::Optional<url::Origin>& initiator_origin| which
   gets stored in a new FrameNavigationEntry::initiator_origin_ field.

2. Updates callers of FNE::UpdateEntry and FNE's constructor to
   provide/propagate the initiator as needed.  This includes
   adding an |initiator_origin| parameter to
   - NavigationEntryImpl's constructor
   - NavigationEntryImpl::AddOrUpdateFrameEntry
   - NavigationController::CreateNavigationEntry
   (the list above is not necessarily exhaustive/complete)

3. Uses the new |FrameNavigationEntry::initiator_origin()| from
   NavigationEntryImpl::ConstructCommonNavigationParams (which
   used to always provide |base::nullopt| initiator for history
   navigations - always treating them as browser-initiated, rather
   than replaying the original initiator).

The changes above makes sure that the right Sec-Fetch-Site http request header is
"replayed" during history navigations.  The CL adds browser tests and
WPT tests to cover the new, desired behavior.

Follow-up changes
=================

This CL does not:

- Use |FrameNavigationEntry::initiator_origin()| in GetOriginForURLLoaderFactory
  in render_frame_host_impl.cc (this will be done in a follow-up CL at
  https://crrev.com/c/1672176)

- Handle persisting |FrameNavigationEntry::initiator_origin()| for
  session restore (this is tracked in a separate https://crbug.com/976055).

Bug: 946503
Change-Id: I4f92614873a5ec8d4b52d3ae22031c7089d87ed5
Tbr: skuhne@chromium.org for //components/sessions
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662738
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677046}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants