Replace GIF defines with enums and constants [pdfium : main]

[Lei Zhang (Gerrit)]

Attention needed from Helmut Januschka and Tom Sepez

Lei Zhang added 2 comments

File core/fxcodec/gif/cfx_gif.h

Line 33, Patchset 2 (Latest):inline constexpr uint8_t kGifSubBlockTerminator = 0x00;

Lei Zhang . unresolved

`inline` probably not needed here and below.

File core/fxcodec/gif/cfx_gifcontext.cpp

Line 73, Patchset 2 (Latest): case fxcrt::to_underlying(GifSignature::kExtension):

Lei Zhang . unresolved

How about adding a helper function to convert `signature` to `std::optional<GifSignature>` instead?

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka
• Tom Sepez

Submit Requirements:

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

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I1843ae2803b05838213d6feeaf5af1d6ec1e7e0b

Gerrit-Change-Number: 148810

Gerrit-PatchSet: 2

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: pdfium...@luci-project-accounts.iam.gserviceaccount.com <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Attention: Tom Sepez <tse...@chromium.org>

Gerrit-Comment-Date: Tue, 23 Jun 2026 20:57:33 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Tom Sepez (Gerrit)]

Attention needed from Helmut Januschka

Tom Sepez added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Tom Sepez . resolved

LG after doing what Lei said.

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka

Submit Requirements:

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

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I1843ae2803b05838213d6feeaf5af1d6ec1e7e0b

Gerrit-Change-Number: 148810

Gerrit-PatchSet: 2

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: pdfium...@luci-project-accounts.iam.gserviceaccount.com <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Comment-Date: Wed, 24 Jun 2026 18:54:24 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Helmut Januschka (Gerrit)]

Attention needed from Lei Zhang

Helmut Januschka added 2 comments

File core/fxcodec/gif/cfx_gif.h

Line 33, Patchset 2 (Latest):inline constexpr uint8_t kGifSubBlockTerminator = 0x00;

Lei Zhang . resolved

`inline` probably not needed here and below.

Helmut Januschka

Done

File core/fxcodec/gif/cfx_gifcontext.cpp

Line 73, Patchset 2 (Latest): case fxcrt::to_underlying(GifSignature::kExtension):

Lei Zhang . resolved

How about adding a helper function to convert `signature` to `std::optional<GifSignature>` instead?

Helmut Januschka

Done

Open in Gerrit

Related details

Attention is currently required from:

• Lei Zhang

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I1843ae2803b05838213d6feeaf5af1d6ec1e7e0b

Gerrit-Change-Number: 148810

Gerrit-PatchSet: 2

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: pdfium...@luci-project-accounts.iam.gserviceaccount.com <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Lei Zhang <the...@chromium.org>

Gerrit-Comment-Date: Wed, 24 Jun 2026 23:29:54 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Lei Zhang <the...@chromium.org>

[Lei Zhang (Gerrit)]

Attention needed from Helmut Januschka

Lei Zhang voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

• Helmut Januschka

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

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

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I1843ae2803b05838213d6feeaf5af1d6ec1e7e0b

Gerrit-Change-Number: 148810

Gerrit-PatchSet: 3

Gerrit-Owner: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Helmut Januschka <hel...@januschka.com>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

Gerrit-CC: pdfium...@luci-project-accounts.iam.gserviceaccount.com <pdfium...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Attention: Helmut Januschka <hel...@januschka.com>

Gerrit-Comment-Date: Wed, 24 Jun 2026 23:33:49 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes