New issue
Advanced search Search tips

Issue 658983 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 640844



Sign in to add a comment

Document.write intervention: warning is using the incorrect term "cross origin" when we meant "cross site (i.e. different eTLD+1)"

Project Member Reported by kenjibaheux@chromium.org, Oct 25 2016

Issue description

The intervention allows scripts hosted on sub domains (e.g. js.example.com inserted via doc.write on www.example.com).

We should update the warning to say "cross site (i.e. different eTLD+1)" instead of "cross origin". 

Context: https://github.com/WICG/interventions/issues/17#issuecomment-255914571

Surfacing into the launch bug but marking as P3 because this is a hint for developers: it would be nice to merge if we can but having it in canary/dev would be a decent state.
 
As I commented on the intent to implement, can we make the use of this terminology mean the same thing across Chromium?

Currently, the intervention only takes into account etld + 1, which does not match the term "cross site" as used elsewhere in Chromium (which is etld + 1 *and* scheme).
I assume that etld+1 and scheme is what "cross-site" means in spec land.
So, +1 to align on the same definition.

Would it be possible to record a UMA for the following:
 - Same-site mitigation: 
   bucket for (same etld+1, any scheme)
   bucket for (same etld+1, same scheme)

I'm hoping that (same etld+1, same scheme) isn't significantly different from what we've currently implemented.
Since http script from an https document is not allowed, the only scenario we are left with, is loading an https script from an http document with the same etld+1. 
There could be arguments to go either way: being consistent with the spec and blocking such a script or not to block it because the reason we decided to not block such scripts is to reduce breakage.
Bryan and I discussed it and think that the first step could be to add a counter 
to count the number of pages which have script(s) that are not blocked because of same etld+1 (but have different scheme). If the number is really low we could switch the logic to be consistent with the spec. If the number is high, on the other hand, we might want to stay with the current logic.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 30 2017

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

commit 52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1
Author: shivanisha <shivanisha@chromium.org>
Date: Mon Jan 30 16:20:48 2017

doc.write intervention warning message changed to replace cross-origin with cross
site (etld+1)
BUG= 658983 

Review-Url: https://codereview.chromium.org/2474943002
Cr-Commit-Position: refs/heads/master@{#446993}

[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-all-conn-types-expected.txt
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-effectively-2g-expected.txt
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-expected.txt
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-conn-type-expected.txt
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload-expected.txt
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/third_party/WebKit/public/platform/WebLoadingBehaviorFlag.h
[modify] https://crrev.com/52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1/tools/metrics/histograms/histograms.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 30 2017

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

commit 13905c2317d62f3f657e7463aa58c093567dd1cc
Author: shivanisha <shivanisha@chromium.org>
Date: Mon Jan 30 18:36:57 2017

Making the warning message more clear and adding an error message when the script actually gets blocked and a warning message if the script is not blocked due to cache hit.

BUG= 658983 

Review-Url: https://codereview.chromium.org/2640163002
Cr-Commit-Position: refs/heads/master@{#447018}

[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-all-conn-types-expected.txt
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-effectively-2g-expected.txt
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-expected.txt
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-conn-type-expected.txt
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload-expected.txt
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp
[modify] https://crrev.com/13905c2317d62f3f657e7463aa58c093567dd1cc/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Status: Fixed (was: Assigned)
Labels: Merge-Request-57
Project Member

Comment 9 by sheriffbot@chromium.org, Jan 31 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 1 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2965f49595126e50e902de401f7f53c70b7a80a1

commit 2965f49595126e50e902de401f7f53c70b7a80a1
Author: shivanisha <shivanisha@chromium.org>
Date: Wed Feb 01 15:55:16 2017

doc.write intervention warning message changed to replace cross-origin with cross site (etld+1) BUG= 658983 

Review-Url: https://codereview.chromium.org/2474943002
Cr-Commit-Position: refs/heads/master@{#446993}
(cherry picked from commit 52c2ea5fbafcf7efc85e8f8fc4865073f3b6aaf1)

BUG= 658983 
TBR=bmcquade@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2667013002
Cr-Commit-Position: refs/branch-heads/2987@{#246}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.cc
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-all-conn-types-expected.txt
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-effectively-2g-expected.txt
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-expected.txt
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-conn-type-expected.txt
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload-expected.txt
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/third_party/WebKit/public/platform/WebLoadingBehaviorFlag.h
[modify] https://crrev.com/2965f49595126e50e902de401f7f53c70b7a80a1/tools/metrics/histograms/histograms.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 1 2017

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

commit 5a01a9745dcc4eefd77725448cd279b94ad00eb0
Author: shivanisha <shivanisha@chromium.org>
Date: Wed Feb 01 16:27:36 2017

Making the warning message more clear and adding an error message when the script actually gets blocked and a warning message if the script is not blocked due to cache hit.

BUG= 658983 

Review-Url: https://codereview.chromium.org/2640163002
Cr-Commit-Position: refs/heads/master@{#447018}
(cherry picked from commit 13905c2317d62f3f657e7463aa58c093567dd1cc)

TBR=bmcquade@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2668813003
Cr-Commit-Position: refs/branch-heads/2987@{#247}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-all-conn-types-expected.txt
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-effectively-2g-expected.txt
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-block-expected.txt
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-conn-type-expected.txt
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/LayoutTests/http/tests/loading/doc-write-sync-third-party-script-reload-expected.txt
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp
[modify] https://crrev.com/5a01a9745dcc4eefd77725448cd279b94ad00eb0/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Sign in to add a comment