Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LayoutNG] Fix crash for very long line with `break-word` #16847

Merged
merged 1 commit into from May 17, 2019

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

chromium-wpt-export-bot commented May 15, 2019

This patch fixes NGLineBreaker crash when handling a very
long line.

Because line breaker determines overflow by the position
being larger than the available width, it cannot stop if the
available width is |LayoutUnit::Max()|. This patch clamps the
available width by |LayoutUnit::NearlyMax()| to prevent that.

The word-wrap: break-word is not necessary to cause the
failure, but it makes things worse by setting |ShapeResult|
to |nullptr| when overflow occurs for the performance reason.

This patch also changes |BreakText| to return whether the
overflow occurred or not, rather than callers to determine it
from the position. Doing so helps clearer communication with
the callers.

Bug: 961987
Change-Id: I17eb758aca8b9b6d3e2f328e5b1b49acb83c0a5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613138
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660871}

Copy link
Collaborator

wpt-pr-bot left a comment

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1613138 branch 3 times, most recently from f3b37a5 to 15184fd May 15, 2019
This patch fixes NGLineBreaker crash when handling a very
long line.

Because line breaker determines overflow by the position
being larger than the available width, it cannot stop if the
available width is |LayoutUnit::Max()|. This patch clamps the
available width by |LayoutUnit::NearlyMax()| to prevent that.

The `word-wrap: break-word` is not necessary to cause the
failure, but it makes things worse by setting |ShapeResult|
to |nullptr| when overflow occurs for the performance reason.

This patch also changes |BreakText| to return whether the
overflow occurred or not, rather than callers to determine it
from the position. Doing so helps clearer communication with
the callers.

Bug: 961987
Change-Id: I17eb758aca8b9b6d3e2f328e5b1b49acb83c0a5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613138
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660871}
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1613138 branch from 15184fd to 3adfdc3 May 17, 2019
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 89c336c into master May 17, 2019
15 checks passed
15 checks passed
manifest-build-and-tag manifest-build-and-tag
Details
website-build-and-publish website-build-and-publish
Details
Azure Pipelines Build #20190517.44 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests (Safari Technology Preview)) affected tests (Safari Technology Preview) succeeded
Details
Azure Pipelines (affected tests without changes (Safari Technology Preview)) affected tests without changes (Safari Technology Preview) succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
staging.wpt.fyi - chrome[experimental] Chrome results
Details
staging.wpt.fyi - firefox[experimental] Firefox results
Details
staging.wpt.fyi - safari[experimental] Safari results
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1613138 branch May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.