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

Issue 800669 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 736308



Sign in to add a comment

Deprecate blink::SchemeRegistry::RegisterURLSchemeAsCORSEnabled()

Project Member Reported by toyoshim@chromium.org, Jan 10 2018

Issue description

As a part of out of renderer CORS support efforts, we will deprecate RegisterURLSchemeAsCORSEnabled interfaces soon.

The interface was removed half a year ago, but restored by Alexey because the interface was used by Electron (https://chromium-review.googlesource.com/c/chromium/src/+/768808).

Once we enable the network service, or browser side CORS support, we could not keep this interface as is. Since the Blink API does not have any promise to keep compatibility, we would just deprecate the interface.

See a design doc for "CORS support in Network Service", https://docs.google.com/document/d/1JNmUcvbw2UcjfdI2uyUpveHXCbae-DQ1n8d_sVs5fLg/edit?usp=sharing
 
CCing Alexey to notify Electron people.
> CCing Alexey to notify Electron people.

Thank you!
Update:
Now I'm writing actual implementation to make it happen (though actual code path switch will happen later). Once we switch the code path by enabling the out-of-renderer CORS migration, the only way to do similar hack is to call url::AddCORSEnabledScheme() in the browser process. But it should not be per-renderer or per-frame, but global setting per-system (means maybe per electron app?).
Project Member

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

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

commit cf046945877b6dbe27f96e3f80fbc317b073f0fa
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Thu Jan 18 14:48:31 2018

OOR-CORS: use network::cors version of CheckRedirectLocation

The legacy implementation depends on Blink APIs that allow
embedders to add more schemes to pass the redirect location check.

This patch still holds the original scheme check that counts in
embedders' desired schemes in the legacy mode, but ignore them
in the out-of-blink CORS support mode.

Bug: 736308,  800669 
Change-Id: I764126014c90fb5b14ee06393704269970be5639
Reviewed-on: https://chromium-review.googlesource.com/867828
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530140}
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/services/network/public/cpp/cors/cors.cc
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/services/network/public/cpp/cors/cors.h
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/services/network/public/cpp/cors/cors_unittest.cc
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/third_party/WebKit/Source/platform/exported/WebCORS.cpp
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/third_party/WebKit/Source/platform/loader/cors/CORS.cpp
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/third_party/WebKit/Source/platform/loader/cors/CORS.h
[modify] https://crrev.com/cf046945877b6dbe27f96e3f80fbc317b073f0fa/third_party/WebKit/public/platform/WebCORS.h

I'm removing RegisterURLSchemeAsCORSEnabled() now.
FYI, you can modify cors-enabled schemes by calling url::AddCORSEnabledScheme. You need to do that before calling any SchemeRegistry functions.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 6

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

commit 646270478deff3cc148dead899cbde169324b6e5
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Mon Aug 06 10:18:23 2018

Remove SchemeRegistry::RegisterURLSchemeAsCORSEnabled

It's not used in chromium, and we need to remove it because we are
moving CORS logic to the network service.

Bug:  800669 
Change-Id: I78d4b9ed641d3dca54ad80954872082e2450d2ef
Reviewed-on: https://chromium-review.googlesource.com/1157364
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580838}
[modify] https://crrev.com/646270478deff3cc148dead899cbde169324b6e5/third_party/blink/renderer/platform/weborigin/scheme_registry.cc
[modify] https://crrev.com/646270478deff3cc148dead899cbde169324b6e5/third_party/blink/renderer/platform/weborigin/scheme_registry.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 8

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

commit 43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Aug 08 07:37:45 2018

[OOR-CORS] Implement redirect location checks

This CL:
 - fixes CheckRedirectLocation implementation so that we can use it
   from CORSURLLoader.
 - fixes CORS flag handling in ResourceLoader so that it can use
   CheckRedirectLocation,
 - removes WebCORS::HandleRedirect, and has it implemented in
   blink::ResourceLoader instead,
 - integrates CheckRedirectLocation with CORSURLLoader, and
 - removes mojom::CORSError::kRedirectDisallowedScheme in favor of
   mojom::CORSError::kCORSDisabledScheme.

Bug: 736308,  800669 , 870173
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib31ab135b03dcc0beca137cbc39592fe3f314790
Reviewed-on: https://chromium-review.googlesource.com/1158089
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581495}
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/services/network/cors/cors_url_loader.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/services/network/public/cpp/cors/cors.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/services/network/public/cpp/cors/cors.h
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/services/network/public/cpp/cors/cors_unittest.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/services/network/public/mojom/cors.mojom
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/public/platform/web_cors.h
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/core/loader/threadable_loader.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/core/loader/threadable_loader.h
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/exported/web_cors.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/cors/cors.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/cors/cors.h
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/cors/cors_error_string.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/43b8facd3a2fbfb8f75b455f07d64acc2c13fb5c/third_party/blink/renderer/platform/loader/fetch/resource_loader.h

Status: Fixed (was: Started)
I believe yhirano's #c7 finished this work

Sign in to add a comment