New issue
Advanced search Search tips

Issue 748580 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

[iOS] Typo to check which iOS version (between 10 and 11)

Project Member Reported by jlebel@chromium.org, Jul 25 2017

Issue description

While fixing an ugly bug, instead of checking for iOS 11, I checked for iOS 10:
https://cs.chromium.org/chromium/src/components/signin/ios/browser/account_consistency_service.mm?q=account_consist&sq=package:chromium&l=423

which should have matched with:
https://cs.chromium.org/chromium/src/components/signin/ios/browser/account_consistency_service.mm?q=account_consist&sq=package:chromium&l=398

The right version is base::ios::IsRunningOnIOS11OrLater().

This bug has been introduced with crrev.com/c/577807
 

Comment 1 by msarda@chromium.org, Jul 26 2017

Labels: -Pri-3 ReleaseBlock-Stable Pri-1
It is not clear to me though if [web_view_ removeFromSuperview] crashes or not when the |web_view| was not added to a superview previously.

The CL that introduced the bug was part of M61 (as it landed last week before branch point).  I think this bug deserves to be a P1 and we should also mark it as RBS for M61 as I think we should merge it back to M61.

Comment 2 by jlebel@chromium.org, Jul 26 2017

I didn't try, but as far as I know when calling -[UIView removeFromSuperview] for a view that is not in a view, it does nothing. There is no crash.

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2017

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

commit 7bd91a687e5eead254808a07f4b8741bf06f864e
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Wed Jul 26 09:45:57 2017

Typo between IsRunningOnIOS11OrLater() and IsRunningOnIOS10OrLater()

Hack introduced for iOS 11 to fix the issue related to WKWebView which
has to be part of the view hierarchy. This hack should be run only on
iOS 11, and not iOS 10.

Bug introduced with crrev.com/b/577807

Bug:  748580 
Change-Id: If133cfbf345961204fdae0773bc728898a2c7284
Reviewed-on: https://chromium-review.googlesource.com/584878
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489589}
[modify] https://crrev.com/7bd91a687e5eead254808a07f4b8741bf06f864e/components/signin/ios/browser/account_consistency_service.mm

Comment 4 by jlebel@chromium.org, Jul 26 2017

Status: Fixed (was: Started)

Comment 5 by jlebel@chromium.org, Jul 26 2017

Labels: Merge-Request-61

Comment 6 by cma...@chromium.org, Jul 26 2017

Labels: -Merge-Request-61 Merge-Approved-61
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e978f217ac595bb58ead87aa32985e21f74871e

commit 5e978f217ac595bb58ead87aa32985e21f74871e
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Thu Jul 27 11:22:32 2017

Typo between IsRunningOnIOS11OrLater() and IsRunningOnIOS10OrLater()

Hack introduced for iOS 11 to fix the issue related to WKWebView which
has to be part of the view hierarchy. This hack should be run only on
iOS 11, and not iOS 10.

Bug introduced with crrev.com/b/577807

TBR=jlebel@chromium.org

(cherry picked from commit 7bd91a687e5eead254808a07f4b8741bf06f864e)

Bug:  748580 
Change-Id: If133cfbf345961204fdae0773bc728898a2c7284
Reviewed-on: https://chromium-review.googlesource.com/584878
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489589}
Reviewed-on: https://chromium-review.googlesource.com/589171
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#76}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/5e978f217ac595bb58ead87aa32985e21f74871e/components/signin/ios/browser/account_consistency_service.mm

Sign in to add a comment