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 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 left a comment

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 Jun 28, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1662738 branch 6 times, most recently from 1e9c02e to d416b33 Jul 8, 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}
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1662738 branch from d416b33 to 70d6cd5 Jul 12, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 457bec5 into master Jul 12, 2019
11 checks passed
11 checks passed
manifest-build-and-tag manifest-build-and-tag
Details
website-build-and-publish website-build-and-publish
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - safari[experimental] Safari results
Details
Azure Pipelines Build #20190712.74 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1662738 branch Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.