New issue
Advanced search Search tips

Issue 896768 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Background Fetch: match() crashes on debug builds

Project Member Reported by nator@google.com, Oct 18

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Oct 22

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

commit 2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e
Author: Rayan Kanso <rayankans@chromium.org>
Date: Mon Oct 22 12:44:15 2018

[Background Fetch] Fix match() crash.

Calling `match` was causing crashes in debug mode due to accessing
base::Optional<>::value without initialization. Calling match with
an unmatched request was crashing due to hitting a DCHECK. This returns
undefined now as per the spec.

Bug:  896768 
Change-Id: I5d82e68ddca157a240ceaaec8990d2553366fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/1289269
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601541}
[modify] https://crrev.com/2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e/third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window-expected.txt
[modify] https://crrev.com/2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e/third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window.js
[modify] https://crrev.com/2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e/third_party/WebKit/LayoutTests/external/wpt/background-fetch/service_workers/sw.js
[modify] https://crrev.com/2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e/third_party/blink/renderer/modules/background_fetch/background_fetch_registration.cc

Labels: Merge-Request-71
Cc: rayankans@chromium.org
Please add appropriate OSs.
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
As component is set to Blink, adding all OSs except iOS.
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 25

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 7 by bugdroid1@chromium.org, Oct 25

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110

commit 9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110
Author: Rayan Kanso <rayankans@chromium.org>
Date: Thu Oct 25 11:29:13 2018

[Background Fetch] Fix match() crash.

Calling `match` was causing crashes in debug mode due to accessing
base::Optional<>::value without initialization. Calling match with
an unmatched request was crashing due to hitting a DCHECK. This returns
undefined now as per the spec.

(cherry picked from commit 2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e)

Bug:  896768 
Change-Id: I5d82e68ddca157a240ceaaec8990d2553366fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/1289269
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601541}
Reviewed-on: https://chromium-review.googlesource.com/c/1299153
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#315}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110/third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window-expected.txt
[modify] https://crrev.com/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110/third_party/WebKit/LayoutTests/external/wpt/background-fetch/fetch.https.window.js
[modify] https://crrev.com/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110/third_party/WebKit/LayoutTests/external/wpt/background-fetch/service_workers/sw.js
[modify] https://crrev.com/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110/third_party/blink/renderer/modules/background_fetch/background_fetch_registration.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110

Commit: 9afaebecbc14ec8fa29da8e15ca9f5b0dcbcd110
Author: rayankans@chromium.org
Commiter: rayankans@chromium.org
Date: 2018-10-25 11:29:13 +0000 UTC

[Background Fetch] Fix match() crash.

Calling `match` was causing crashes in debug mode due to accessing
base::Optional<>::value without initialization. Calling match with
an unmatched request was crashing due to hitting a DCHECK. This returns
undefined now as per the spec.

(cherry picked from commit 2fa238e3ac6f79fe0e11c8a1065fed41b4ac4a1e)

Bug:  896768 
Change-Id: I5d82e68ddca157a240ceaaec8990d2553366fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/1289269
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601541}
Reviewed-on: https://chromium-review.googlesource.com/c/1299153
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#315}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Status: Fixed (was: Started)
Thanks for taking this on, Rayan!

Sign in to add a comment