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

Issue 635029 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Does UseCounter::DocumentBeforeUnloadFired mess up frame vs subframes?

Project Member Reported by mustaq@chromium.org, Aug 5 2016

Issue description

This came up during a code review:
https://codereview.chromium.org/2205523004/#msg10

In EventTarget::fireEventListeners(Event*, EventTargetData*, EventListenerVector&), UseCounter::DocumentBeforeUnloadFired is counted under the condition:

  if (executingWindow->top())

which used to be:

  if (executingWindow != executingWindow->top())

before https://codereview.chromium.org/22564003.

Any chance the original was more accurate in terms of frame vs subframe distinction?

 

Comment 1 by bokan@chromium.org, Aug 8 2016

Owner: bokan@chromium.org
Status: Started (was: Available)
Ojan mentioned in the review that it's wrong. I've got a CL up for review.
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 9 2016

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

commit 7d7dac7ca90eb803aa238e09ee6a183338e216c8
Author: bokan <bokan@chromium.org>
Date: Tue Aug 09 07:13:00 2016

Fix UseCounter for SubframeBeforeUnloadFired.

The condition used to check whether the window is a subframe or not was
wrong. It was originally `executingWindow != executingWindow->top()` but
got accidentally changed in https://codereview.chromium.org/22564003.
This CL fixes that error.

BUG= 635029 

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

[modify] https://crrev.com/7d7dac7ca90eb803aa238e09ee6a183338e216c8/third_party/WebKit/Source/core/events/EventTarget.cpp
[modify] https://crrev.com/7d7dac7ca90eb803aa238e09ee6a183338e216c8/third_party/WebKit/Source/web/tests/WebViewTest.cpp

Comment 3 by bokan@chromium.org, Aug 9 2016

Status: Fixed (was: Started)

Sign in to add a comment