New issue
Advanced search Search tips

Issue 836741 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 803766



Sign in to add a comment

OOR-CORS: some wpt preflight tests crash only on release build

Project Member Reported by toyoshim@chromium.org, Apr 25 2018

Issue description

Components: Blink>SecurityFeature>CORS Blink>Loader
Labels: OOR-CORS
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 28 2018

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

commit 1f675b46670aff95516ec8b9d61bc0f55e930a47
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Sat Apr 28 09:45:43 2018

OOR-CORS: store more information to CORSErrorStatus on preflight errors

Current implementation does not store rejected method or header
information to CORSErrorStatus on CORS-preflight checks, and
results in missing hint parameter on CreateErrorString() in Blink.
This patch provides required information to generate right error
messages, and fixes some crashed layout tests.

Bug:  803766 ,  836741 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If4de6b916a77d204812a7b010e39080bf059bc9e
Tbr: kinuko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1030670
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554642}
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/services/network/cors/preflight_controller.cc
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/services/network/public/cpp/cors/cors_error_status.cc
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/services/network/public/cpp/cors/cors_error_status.h
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/services/network/public/cpp/network_param_ipc_traits.h
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/third_party/blink/renderer/core/loader/document_threadable_loader.cc
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/third_party/blink/renderer/platform/loader/cors/cors_error_string.cc
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/third_party/blink/renderer/platform/loader/cors/cors_error_string.h
[modify] https://crrev.com/1f675b46670aff95516ec8b9d61bc0f55e930a47/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 30 2018

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

commit c4189fd6b16899bef539939ab6d3028d17415232
Author: Christos Froussios <cfroussios@chromium.org>
Date: Mon Apr 30 09:12:04 2018

Revert "OOR-CORS: store more information to CORSErrorStatus on preflight errors"

This reverts commit 1f675b46670aff95516ec8b9d61bc0f55e930a47.

Reason for revert: Breaks builder WebKit Linux Trusty ASAN

Original change's description:
> OOR-CORS: store more information to CORSErrorStatus on preflight errors
> 
> Current implementation does not store rejected method or header
> information to CORSErrorStatus on CORS-preflight checks, and
> results in missing hint parameter on CreateErrorString() in Blink.
> This patch provides required information to generate right error
> messages, and fixes some crashed layout tests.
> 
> Bug:  803766 ,  836741 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Change-Id: If4de6b916a77d204812a7b010e39080bf059bc9e
> Tbr: kinuko@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/1030670
> Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554642}

TBR=kinuko@chromium.org,toyoshim@chromium.org,yhirano@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  803766 ,  836741 ,  838057 
Change-Id: Ia8eb9bf63b1b0c6edda93eb7d45c1bb20d26a7b7
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1034532
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554712}
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/services/network/cors/preflight_controller.cc
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/services/network/public/cpp/cors/cors_error_status.cc
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/services/network/public/cpp/cors/cors_error_status.h
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/services/network/public/cpp/network_param_ipc_traits.h
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/third_party/blink/renderer/core/loader/document_threadable_loader.cc
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/third_party/blink/renderer/platform/loader/cors/cors_error_string.cc
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/third_party/blink/renderer/platform/loader/cors/cors_error_string.h
[modify] https://crrev.com/c4189fd6b16899bef539939ab6d3028d17415232/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 30 2018

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

commit 4741ed4fb65821a94134605ad9ecaf11b61b6786
Author: Christos Froussios <cfroussios@chromium.org>
Date: Mon Apr 30 12:25:55 2018

Reland "OOR-CORS: store more information to CORSErrorStatus on preflight errors"

This reverts commit c4189fd6b16899bef539939ab6d3028d17415232.

Reason for revert: Reverting caused other, bigger failures. I will
disable the test instead.

Original change's description:
> Revert "OOR-CORS: store more information to CORSErrorStatus on preflight errors"
> 
> This reverts commit 1f675b46670aff95516ec8b9d61bc0f55e930a47.
> 
> Reason for revert: Breaks builder WebKit Linux Trusty ASAN
> 
> Original change's description:
> > OOR-CORS: store more information to CORSErrorStatus on preflight errors
> > 
> > Current implementation does not store rejected method or header
> > information to CORSErrorStatus on CORS-preflight checks, and
> > results in missing hint parameter on CreateErrorString() in Blink.
> > This patch provides required information to generate right error
> > messages, and fixes some crashed layout tests.
> > 
> > Bug:  803766 ,  836741 
> > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> > Change-Id: If4de6b916a77d204812a7b010e39080bf059bc9e
> > Tbr: kinuko@chromium.org
> > Reviewed-on: https://chromium-review.googlesource.com/1030670
> > Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
> > Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#554642}
> 
> TBR=kinuko@chromium.org,toyoshim@chromium.org,yhirano@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  803766 ,  836741 ,  838057 
> Change-Id: Ia8eb9bf63b1b0c6edda93eb7d45c1bb20d26a7b7
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
> Reviewed-on: https://chromium-review.googlesource.com/1034532
> Reviewed-by: Christos Froussios <cfroussios@chromium.org>
> Commit-Queue: Christos Froussios <cfroussios@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554712}

TBR=kinuko@chromium.org,toyoshim@chromium.org,yhirano@chromium.org,cfroussios@chromium.org

Change-Id: Iaf20d807cf9f71976a3d1c8a46678afb48682029
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  803766 ,  836741 ,  838057 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1034702
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Commit-Queue: Christos Froussios <cfroussios@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554726}
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/services/network/cors/preflight_controller.cc
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/services/network/public/cpp/cors/cors_error_status.cc
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/services/network/public/cpp/cors/cors_error_status.h
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/services/network/public/cpp/network_param_ipc_traits.h
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/third_party/blink/renderer/core/loader/document_threadable_loader.cc
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/third_party/blink/renderer/platform/loader/cors/cors_error_string.cc
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/third_party/blink/renderer/platform/loader/cors/cors_error_string.h
[modify] https://crrev.com/4741ed4fb65821a94134605ad9ecaf11b61b6786/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

Cc: yhirano@chromium.org
Status: Fixed (was: Assigned)
These test were already fixed to run well, and the TestExpectations does not include them any more.

Sign in to add a comment