Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(56)

Issue 2977833002: Fix dispatch_once deadlock (Closed)

Created:
3 years, 5 months ago by jzw1
Modified:
3 years, 5 months ago
CC:
chromium-reviews, Eugene But (OOO till 7-30), ios-reviews+web_chromium.org, ios-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix dispatch_once deadlock BUG= Review-Url: https://codereview.chromium.org/2977833002 Cr-Commit-Position: refs/heads/master@{#486099} Committed: https://chromium.googlesource.com/chromium/src/+/4153aff036de400191be6373e81b2bd6b9f195bc

Patch Set 1 #

Total comments: 3

Patch Set 2 : avoid we #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -2 lines) Patch
M ios/web_view/internal/web_view_web_main_delegate.mm View 1 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
jzw1
On 2017/07/12 19:55:05, jzw1 wrote: > mailto:jzw@chromium.org changed reviewers: > + mailto:eugenebut@chromium.org, mailto:ichikawa@chromium.org, mailto:michaeldo@chromium.org PTAL
3 years, 5 months ago (2017-07-12 19:55:31 UTC) #3
michaeldo
https://codereview.chromium.org/2977833002/diff/1/ios/web_view/internal/web_view_web_main_delegate.mm File ios/web_view/internal/web_view_web_main_delegate.mm (right): https://codereview.chromium.org/2977833002/diff/1/ios/web_view/internal/web_view_web_main_delegate.mm#newcode21 ios/web_view/internal/web_view_web_main_delegate.mm:21: // We're using CWVHTMLElement instead of CWVWebView and Avoid ...
3 years, 5 months ago (2017-07-12 20:04:32 UTC) #4
Eugene But (OOO till 7-30)
Looks strange, but it's not like I can offer anything better. lgtm https://codereview.chromium.org/2977833002/diff/1/ios/web_view/internal/web_view_web_main_delegate.mm File ios/web_view/internal/web_view_web_main_delegate.mm ...
3 years, 5 months ago (2017-07-12 20:05:37 UTC) #5
jzw1
https://codereview.chromium.org/2977833002/diff/1/ios/web_view/internal/web_view_web_main_delegate.mm File ios/web_view/internal/web_view_web_main_delegate.mm (right): https://codereview.chromium.org/2977833002/diff/1/ios/web_view/internal/web_view_web_main_delegate.mm#newcode21 ios/web_view/internal/web_view_web_main_delegate.mm:21: // We're using CWVHTMLElement instead of CWVWebView and On ...
3 years, 5 months ago (2017-07-12 20:23:38 UTC) #6
michaeldo
lgtm
3 years, 5 months ago (2017-07-12 20:30:53 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2977833002/20001
3 years, 5 months ago (2017-07-12 20:31:50 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/4153aff036de400191be6373e81b2bd6b9f195bc
3 years, 5 months ago (2017-07-12 21:47:36 UTC) #13
Hiroshi Ichikawa
LGTM but maybe alternatively define a dummy empty class in web_view_web_main_delegate.mm and use it?
3 years, 5 months ago (2017-07-13 04:46:19 UTC) #14
jzw1
3 years, 5 months ago (2017-07-13 06:20:17 UTC) #15
Message was sent while issue was closed.
On 2017/07/13 04:46:19, Hiroshi Ichikawa wrote:
> LGTM but maybe alternatively define a dummy empty class in
> web_view_web_main_delegate.mm and use it?

That's actually a pretty good idea...I'll make that change.

Powered by Google App Engine
This is Rietveld 408576698