Re: Issue 520857 in chromium: Chrome: Crash Report - GURL::GetOrigin

[chro...@googlecode.com]

Updates:
Status: Available
Cc: -aba...@chromium.org -bre...@chromium.org ho...@chromium.org
Labels: -Restrict-View-Google -Pri-2 -Needs-Triage -OS-Android -OS-Chrome
Pri-1 Cr-Blink-ServiceWorker
Mergedinto:

Comment #3 on issue 520857 by fal...@chromium.org: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

(De-duping, I don't think that bug is related to this crash.)

This looks like a crash in DidDispatchFetchEvent. provider_host_ or
request() is null?? Neither makes much sense.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[chro...@googlecode.com]

Updates:
Status: Started
Owner: fal...@chromium.org

Comment #4 on issue 520857 by fal...@chromium.org: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

Looks like provider_host_ is null.

[chro...@googlecode.com]

Comment #5 on issue 520857 by bugd...@chromium.org: Chrome: Crash Report
- GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857#c5

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/a6c778d2a0525c8891d4f6fbd8cee0b08f087f1b

commit a6c778d2a0525c8891d4f6fbd8cee0b08f087f1b
Author: falken <fal...@chromium.org>
Date: Wed Aug 26 09:06:33 2015

Fix crash with null ServiceWorkerProviderHost in DidDispatchFetchEvent

The provider host is a WeakPtr and can be destroyed if the tab was
closed.

BUG=520857

Review URL: https://codereview.chromium.org/1304033004

Cr-Commit-Position: refs/heads/master@{#345568}

[modify]
http://crrev.com/a6c778d2a0525c8891d4f6fbd8cee0b08f087f1b/content/browser/service_worker/service_worker_url_request_job.cc
[modify]
http://crrev.com/a6c778d2a0525c8891d4f6fbd8cee0b08f087f1b/content/browser/service_worker/service_worker_url_request_job_unittest.cc

[chro...@googlecode.com]

Updates:
Status: Fixed

Comment #6 on issue 520857 by fal...@chromium.org: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

Will request merge to m46 and maybe m45 after it passes through canary.

[chro...@googlecode.com]

Updates:
Labels: Merge-Request-46

Comment #8 on issue 520857 by fal...@chromium.org: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

Request merge to M46. This went through Canary ok with no new crashes.

I think M45 doesn't need a merge: the crash happens very rarely there M45
(21 reports[1]). The change looks safe but all merges have some risk and
it's probably not worth it in this case. It happens much more often in M46
since |provider_host_| is used in the code added at
https://codereview.chromium.org/1282193002

[1]
https://crash.corp.google.com/browse?q=Product.Name%20CONTAINS%20%27Chrome%27%20AND%20product.Version%3E%3D%2745%27%20AND%20product.Version%3C%2746%27%20OMIT%20RECORD%20IF%20SUM(CrashedStackTrace.StackFrame.FunctionName%20CONTAINS%20%27content%3A%3AServiceWorkerURLRequestJob%3A%3ADidDispatchFetchEvent%27)%20%3D%200

[chro...@googlecode.com]

Updates:
Labels: -Merge-Request-46 Merge-Approved-46 Hotlist-Merge-Approved

Comment #9 on issue 520857 by penny...@google.com: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857#c9

Approved for M46 (branch: 2490)

[chro...@googlecode.com]

Updates:
Labels: -Merge-Approved-46 merge-merged-2490

Comment #10 on issue 520857 by bugd...@chromium.org: Chrome: Crash Report
- GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857#c10

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/8b1e8e4e3fb7ba22ce51a72f22f103f776b72666

commit 8b1e8e4e3fb7ba22ce51a72f22f103f776b72666
Author: Matt Falkenhagen <fal...@chromium.org>
Date: Mon Aug 31 05:44:34 2015

(Merge to M46) Fix crash with null ServiceWorkerProviderHost in

DidDispatchFetchEvent

The provider host is a WeakPtr and can be destroyed if the tab was
closed.

BUG=520857

Review URL: https://codereview.chromium.org/1304033004

Cr-Commit-Position: refs/heads/master@{#345568}

(cherry picked from commit a6c778d2a0525c8891d4f6fbd8cee0b08f087f1b)
TBR=horo

Review URL: https://codereview.chromium.org/1304263010 .

Cr-Commit-Position: refs/branch-heads/2490@{#88}
Cr-Branched-From:
7790a3535f2a81a03685eca31a32cf69ae0c114f-refs/heads/master@{#344925}

[modify]
http://crrev.com/8b1e8e4e3fb7ba22ce51a72f22f103f776b72666/content/browser/service_worker/service_worker_url_request_job.cc
[modify]
http://crrev.com/8b1e8e4e3fb7ba22ce51a72f22f103f776b72666/content/browser/service_worker/service_worker_url_request_job_unittest.cc

[chro...@googlecode.com]

Comment #11 on issue 520857 by bugd...@chromium.org: Chrome: Crash Report
- GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857#c11

The following revision refers to this bug:

https://chrome-internal.googlesource.com/bling/chromium.git/+/8b1e8e4e3fb7ba22ce51a72f22f103f776b72666

commit 8b1e8e4e3fb7ba22ce51a72f22f103f776b72666
Author: Matt Falkenhagen <fal...@chromium.org>
Date: Mon Aug 31 05:44:34 2015

[chro...@googlecode.com]

Updates:
Labels: TE-Verified-M47 TE-Verified-47.0.2503.0

Comment #12 on issue 520857 by durga.be...@chromium.org: Chrome: Crash

Report - GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

Crashes are not seen on M 47 builds after the fix landed.
Crashes on latest Build are as below.
47.0.2498.0 0.76% 14 --Dev
46.0.2490.13 0.05% 1 --Beta
45.0.2454.85 0.16% 3 --Stable

Link to complete list of Builds:
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27GURL%3A%3AGetOrigin%27#-samplereports:5,productversion:1000

falken@ : As we are still seeing the crashes on latest M 46 build
46.0.2490.13, could you please merge it into M 46.

[chro...@googlecode.com]

Updates:
Cc: f...@chromium.org

Comment #13 on issue 520857 by fal...@chromium.org: Chrome: Crash Report -
GURL::GetOrigin
https://code.google.com/p/chromium/issues/detail?id=520857

Hi durga.behera, I had already merged to M46 at 46.0.2490.11. What you're
seeing is a different crash.

The crashes in 46.0.2490.13 have callstack:
0x000007feec2e3b23 (chrome.dll -gurl.cc:315 ) GURL::GetOrigin()
0x000007feec6e8744 (chrome.dll -permission_bubble_manager.cc:115 )
PermissionBubbleManager::AddRequest(PermissionBubbleRequest *)
0x000007feed185a38 (chrome.dll -permission_context_base.cc:157 )
PermissionContextBase::DecidePermission(content::WebContents
*,PermissionRequestID const &,GURL const &,GURL const
&,bool,base::Callback<void > const &)
0x000007feed185f1e (chrome.dll -permission_context_base.cc:49 )
PermissionContextBase::RequestPermission(content::WebContents
*,PermissionRequestID const &,GURL const &,bool,base::Callback<void > const
&)

That is not the ServiceWorkerURLRequestJob::DidDispatchFetchEvent callsite.

The PermissionBubbleManager crash looks fixed in issue 457091. That fix was
merged to 46.0.2490.14.