Add Blink trace signal for meta charset disposition [chromium/src : main]

0 views

Skip to first unread message

Mason Freed (Gerrit)

unread,

Feb 19, 2026, 5:16:47 PM (20 hours ago) Feb 19

to Helmut Januschka, Charlie Harrison, Chromium LUCI CQ, chromium...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, loading-rev...@chromium.org

Attention needed from Charlie Harrison and Helmut Januschka

Mason Freed added 6 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Mason Freed . resolved

Looks overall reasonable to me. Just some smaller comments.

File third_party/blink/renderer/core/dom/decoded_data_document_parser.cc

Line 90, Patchset 3 (Latest): MaybeEmitMetaCharsetTraceEvent();

Mason Freed . unresolved

Maybe you could move this to `UpdateDocument`?

Line 135, Patchset 3 (Latest):

  Document* document = GetDocument();
  if (!document) {
    return;
  }

  LocalFrame* frame = document->GetFrame();
  if (!frame) {
    return;
  }

Mason Freed . unresolved

Perhaps move these above the switch()?

File third_party/blink/renderer/core/html/parser/html_meta_charset_parser.cc

Line 99, Patchset 3 (Latest): kBytesToCheckUnconditionally

Mason Freed . unresolved

Hmm. Feels a little weird to have a named constant that happens to be 1024, combined with the explicit 1024 in these enums. Perhaps you could just add a static_assert here that kBytesToCheckUnconditionally is 1024, and a comment to do something about that if someone breaks the assert?

File third_party/blink/renderer/core/html/parser/text_resource_decoder.h

Line 52, Patchset 3 (Latest): enum class MetaCharsetDisposition {

Mason Freed . unresolved

We shouldn't have the same enum class in two places.

File third_party/blink/renderer/core/html/parser/text_resource_decoder.cc

Line 53, Patchset 3 (Latest):


TextResourceDecoder::MetaCharsetDisposition ToMetaCharsetDisposition(
    HTMLMetaCharsetParser::MetaCharsetDisposition parser_disposition) {
  switch (parser_disposition) {
    case HTMLMetaCharsetParser::MetaCharsetDisposition::kUnknown:
      return TextResourceDecoder::MetaCharsetDisposition::kUnknown;
    case HTMLMetaCharsetParser::MetaCharsetDisposition::kFoundInFirst1024Bytes:
      return TextResourceDecoder::MetaCharsetDisposition::
          kFoundInFirst1024Bytes;
    case HTMLMetaCharsetParser::MetaCharsetDisposition::
        kFoundAfterFirst1024Bytes:
      return TextResourceDecoder::MetaCharsetDisposition::
          kFoundAfterFirst1024Bytes;
    case HTMLMetaCharsetParser::MetaCharsetDisposition::kNotFound:
      return TextResourceDecoder::MetaCharsetDisposition::kNotFound;
  }

  return TextResourceDecoder::MetaCharsetDisposition::kUnknown;

Mason Freed . unresolved

Any way to combine them to avoid this?

Open in Gerrit

Related details

Attention is currently required from:

Charlie Harrison
Helmut Januschka

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages