New issue
Advanced search Search tips

Issue 759783 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

DevTools: Target.createTarget reports wrong target type

Project Member Reported by dgozman@chromium.org, Aug 28 2017

Issue description

After sending Target.createTarget, next notification Target.targetCreated reports "other" instead of "page".

Observed in puppeteer:
https://github.com/GoogleChrome/puppeteer/pull/554/files/b330a8ef05c41e26acfd4e4baa69e7d271f6c160#diff-c2118b51ae641c280453330fd5ebf030R54
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 12 2017

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

commit 05273a8ee434a38484b8a8e04d04d0a4c79d51d7
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Tue Sep 12 21:48:15 2017

[DevTools] Mark targets as 'page' by default in ChromeDevToolsManagerDelegate

Currently we mark targets as 'other' based on WebContents being present
in TabStripModel, which is timing-dependent. This patch marks everything
not quialified for other types as 'page', making target type consistent.

Bug:  759783 
Change-Id: I0b26ee12474b004121e3e0083d67eb809b4eba11
Reviewed-on: https://chromium-review.googlesource.com/639024
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501410}
[modify] https://crrev.com/05273a8ee434a38484b8a8e04d04d0a4c79d51d7/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
[modify] https://crrev.com/05273a8ee434a38484b8a8e04d04d0a4c79d51d7/chrome/browser/devtools/devtools_sanity_browsertest.cc
[modify] https://crrev.com/05273a8ee434a38484b8a8e04d04d0a4c79d51d7/third_party/WebKit/Source/devtools/front_end/Tests.js

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20 2017

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

commit 2d4fdfad5531b8a7c4733dc537b4a4f38892f723
Author: Justin TerAvest <teravest@chromium.org>
Date: Wed Sep 20 23:32:55 2017

Revert "[DevTools] Mark targets as 'page' by default in ChromeDevToolsManagerDelegate"

This reverts commit 05273a8ee434a38484b8a8e04d04d0a4c79d51d7.

Reason for revert: Broke perf benchmarks. crbug.com/763834

Original change's description:
> [DevTools] Mark targets as 'page' by default in ChromeDevToolsManagerDelegate
> 
> Currently we mark targets as 'other' based on WebContents being present
> in TabStripModel, which is timing-dependent. This patch marks everything
> not quialified for other types as 'page', making target type consistent.
> 
> Bug:  759783 
> Change-Id: I0b26ee12474b004121e3e0083d67eb809b4eba11
> Reviewed-on: https://chromium-review.googlesource.com/639024
> Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
> Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#501410}

TBR=dgozman@chromium.org,pfeldman@chromium.org

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

Bug:  759783 
Change-Id: I85e53f4e0b9900cb1c3027e4e1e56b450628509a
Reviewed-on: https://chromium-review.googlesource.com/675805
Reviewed-by: Justin TerAvest <teravest@chromium.org>
Commit-Queue: Justin TerAvest <teravest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503280}
[modify] https://crrev.com/2d4fdfad5531b8a7c4733dc537b4a4f38892f723/chrome/browser/devtools/chrome_devtools_manager_delegate.cc
[modify] https://crrev.com/2d4fdfad5531b8a7c4733dc537b4a4f38892f723/chrome/browser/devtools/devtools_sanity_browsertest.cc
[modify] https://crrev.com/2d4fdfad5531b8a7c4733dc537b4a4f38892f723/third_party/WebKit/Source/devtools/front_end/Tests.js

Sign in to add a comment