New issue
Advanced search Search tips

Issue 859691 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Task



Sign in to add a comment

document.open() should have more granular UseCounters

Project Member Reported by timothygu@chromium.org, Jul 2

Issue description

There are two types of document.open() in the HTML Standard: a two-argument and a three-argument version [1]. The two-argument variant will potentially be revised by WHATWG [2], and requires more telemetry data available. We do have a UseCounter for document.open() right now (DocumentOpen), but it covers both variants and is not very helpful for this purpose.

[1]: https://html.spec.whatwg.org/multipage/dom.html#the-document-object:dom-document-open
[2]: https://github.com/whatwg/html/issues/3564

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 3

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

commit 136369cac7fc33a14bea3c6650af8ac3bbaf6813
Author: Timothy Gu <timothygu@chromium.org>
Date: Tue Jul 03 19:20:27 2018

More granular UseCounter for document.open()

Bug:  859691 
Change-Id: I9f1fbb3dac9b794dcf1f069447096c8559e03d5b
Reviewed-on: https://chromium-review.googlesource.com/1123202
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572316}
[add] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-0args.html
[add] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-1arg.html
[add] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-2args-replace.html
[add] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-2args.html
[add] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-3args.html
[modify] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/third_party/blink/renderer/core/dom/document.idl
[modify] https://crrev.com/136369cac7fc33a14bea3c6650af8ac3bbaf6813/tools/metrics/histograms/enums.xml

Status: Fixed (was: Started)
Labels: -Pri-3 Merge-Request-68 Pri-2
Requesting merge to 68. This way, we can get more data coming in sooner to inform further specification work on document.open().
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 9

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please mark all OS's this is impacting. 
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Thanks for the fix! how safe is this merge? Why is this critical for M68 vs waiting until 69?
Hi abdulsyed@,

As an intern, Timothy has limited time to work on this project, and the extra six weeks of stable use-counter data would be quite beneficial to our progress in his remaining time.

This is very safe to merge, as it changes no user- or web-developer-visible behavior, only adds/fixes telemetry.

Hope that helps!
Labels: -Merge-Review-68 Merge-Approved-68
Approving merge to M68. Branch:3440
Project Member

Comment 10 by sheriffbot@chromium.org, Jul 17

Cc: abdulsyed@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 11 by bugdroid1@chromium.org, Jul 17

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/852a546ce779bec6ff2e452d740c23597c5b41f8

commit 852a546ce779bec6ff2e452d740c23597c5b41f8
Author: Timothy Gu <timothygu@chromium.org>
Date: Tue Jul 17 18:03:58 2018

More granular UseCounter for document.open()

TBR=timothygu@chromium.org

(cherry picked from commit 136369cac7fc33a14bea3c6650af8ac3bbaf6813)

Bug:  859691 
Change-Id: I9f1fbb3dac9b794dcf1f069447096c8559e03d5b
Reviewed-on: https://chromium-review.googlesource.com/1123202
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#572316}
Reviewed-on: https://chromium-review.googlesource.com/1140205
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#696}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[add] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-0args.html
[add] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-1arg.html
[add] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-2args-replace.html
[add] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-2args.html
[add] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/WebKit/LayoutTests/fast/dom/Document/document-open-use-counters-3args.html
[modify] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/blink/public/platform/web_feature.mojom
[modify] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/third_party/blink/renderer/core/dom/document.idl
[modify] https://crrev.com/852a546ce779bec6ff2e452d740c23597c5b41f8/tools/metrics/histograms/enums.xml

Labels: -Hotlist-Merge-Review
Status: Verified (was: Fixed)
Labels: M-68

Sign in to add a comment