New issue
Advanced search Search tips

Issue 617677 link

Starred by 1 user
Status: Untriaged
Owner: ----
Cc:
dcheng@chromium.org
alex...@chromium.org
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 642378


Sign in to add a comment

OOPIF: fix uses of Document::topDocument

Project Member Reported by alex...@chromium.org, Jun 6 2016 
There are currently 9 call sites for Document::topDocument(), dealing with cookies, scrolling, SVG, fullscreen, and WebGL.  They very likely need to be updated/fixed for OOPIF scenarios.

Currently, Document::topDocument just returns the local root's Document and also has this note:
    // FIXME: Not clear what topDocument() should do in the OOPI case--should it return the topmost
    // available Document, or something else?

I noticed it's used incorrectly in Document::firstPartyForCookies, which checks whether the top frame is local, but then assumes that topDocument() will return the top frame's Document, which won't work when called from the bottom frame in an A-B-A hierarchy.  We should probably  try to remove this API completely, given the confusion it's causing.
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 8 2016 

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

commit 66606be397f2a0ba853b32dbe335a131e59bd3c7
Author: alexmos <alexmos@chromium.org>
Date: Wed Jun 08 15:28:27 2016

Fix Document::firstPartyForCookies() for OOPIFs.

The current usage of topDocument() in firstPartyForCookies() isn't
correct, since topDocument() doesn't actually always return the top
frame's Document, even when it's local.  For example, this will happen
when called from the bottom frame in an A-B-A hierarchy.

In addition, when the top frame is remote, topDocumentURL should use
the top frame's origin, rather than the current frame's origin.

BUG=617677

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

[modify] https://crrev.com/66606be397f2a0ba853b32dbe335a131e59bd3c7/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/66606be397f2a0ba853b32dbe335a131e59bd3c7/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.cpp
[modify] https://crrev.com/66606be397f2a0ba853b32dbe335a131e59bd3c7/third_party/WebKit/Source/platform/weborigin/SchemeRegistry.h
[modify] https://crrev.com/66606be397f2a0ba853b32dbe335a131e59bd3c7/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Comment 2 by alex...@chromium.org, Sep 28 2016

Blockedon: 642378
Project Member

Comment 3 by sheriffbot@chromium.org, Sep 29 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

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

Comment 4 by lukasza@chromium.org, Sep 29 2017

Labels: -Hotlist-Recharge-Cold
Still 7 callsites:
- 1 in HTMLCanvasElement::GetExecutionContextUrl (returns top doc's url)
- 4 in AutoplayUmaHelper::RecordCrossOriginAutoplayResult (top doc passed as last arg of Platform::RecordRapporURL)
- 1 in RootScrollerController.cpp in FillsViewport helper function (https://crbug.com/642378)
- 1 in SVGRootPainter::PaintReplaced (PaintTiming::From(...<top document>...))
►

Sign in to add a comment