New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 785248 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 791096
issue 791573



Sign in to add a comment

MDC roll blocked due to new dependencies

Project Member Reported by sdefresne@chromium.org, Nov 15 2017

Issue description

The following dependencies where added to MDC:
- motion_animator_objc
- motion_interchange_objc

The roll also requires fixing some #import in MDC (to use framework style import instead of relative imports).


 
I've created 3 push requests for MDC, MAO and MIO to fix the issues.

I've got the code to build with those patches applied locally, and the necessary dependencies fetched from github and BUILD.gn files created.

Will need to get the push requests landed, the new dependencies approved by eng/security/licensing, mirror created and then added to DEPS.
Cc: olivierrobin@chromium.org lpromero@chromium.org
These dependecies must also be added to the autoroller.

Comment 5 by pkl@chromium.org, Nov 16 2017

How do we get the mirror created?

Comment 6 by pkl@chromium.org, Nov 16 2017

Labels: M-64 ReleaseBlock-Beta
I assume that we need this before we branch in 2 weeks.

Comment 7 by pkl@chromium.org, Nov 16 2017

Cc: cma...@chromium.org
We need to follow procedure here to get the new dependencies approved:
https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_third_party.md

Then create tickets to have the git repositories mirrored.

Comment 9 by mmoroz@chromium.org, Nov 16 2017

Cc: mmoroz@chromium.org
Is https://github.com/material-motion/motion-interchange-objc/issues/5 something we are interested in resolving of? I'm not aware of the context, but the issue summary sounds like a positive change from the security standpoint.
Components: Internals
I currently received the following approval:
- motion_animator_objc: eng (brettw), security (mmoroz)
- motion_interchange_objc: eng (brettw), security (mmoroz)

Still waiting for the open-source third-party review.
I don't really know what is the policy for open-source third-party review "pinging". Should I "ping" them now or not?
I'd kindly ping.

Note: there is also https://github.com/material-motion/motion-transitioning-objc.git.
https://github.com/material-components/material-components-ios/pull/2400/files

They told me it was the last one in the foreseeable future.
sdefresne: did you get any feedback? Let me know if we need to offload to the current or next MDC gardener.
As Sylvain pointed out IRL, we don't actually depend on the components that use https://github.com/material-motion/motion-transitioning-objc.git, so we don't need it yet. No need to add it as 3rd party dependency of Chromium.
Pinged the two Open Source review threads today.
Work remaining to do:

1. get Open Source Review approval (pkl is cc: on the thread and can ping)
2. once approval has been given, file tickets to mirror github repositories
  2.1. https://github.com/material-motion/motion-interchange-objc
    -> $chromium_git/external/github.com/material-animation/motion-interchange-objc.git
  2.2. https://github.com/material-motion/motion-animator-objc
    -> $chromium_git/external/github.com/material-animation/motion-animator-objc.git
3. once the mirror have been set up, land the CLs in order
  3.1. https://chromium-review.googlesource.com/c/chromium/src/+/774738
  3.2. https://chromium-review.googlesource.com/c/chromium/src/+/774739
  3.3. https://chromium-review.googlesource.com/c/chromium/src/+/788877
  3.4. https://chromium-review.googlesource.com/c/chromium/src/+/771694
4. (optional) land cleanup CLs in order
  4.1. https://chrome-internal-review.googlesource.com/c/chrome/ios_internal/+/513952
  4.2. https://chromium-review.googlesource.com/c/chromium/src/+/789332

Anyone can take care of steps 2-4, just download the CLs and re-upload them if necessary.

Progress:

1. Approval received.
2.
  2.1 b/69869089
  2.2 b/69869105

Wrong bugs, as these are for mirroring on github.gogglesource.com.
So:
2. Completed:
  2.1 https://crbug.com/790546: fixed
  2.2 https://crbug.com/790544: fixed

Onto landed Sylvain's CLs.
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 1 2017

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

commit 070940de3114ea1ccb895c28c23ec6acd1bcb517
Author: mrefaat <mrefaat@chromium.org>
Date: Fri Dec 01 02:59:38 2017

Add dependency on Motion Interchange for Objective-C.

The library Motion Interchange for Objective-C is a new dependency
of the Material Components for iOS. Library description:

    This library defines a format for representing motion in
    Objective-C and Swift applications. The primary data type,
    MotionTiming, allows you to describe the duration, delay,
    timing curve, and repetition for an animation.

Bug:  785248 
Change-Id: I8a86c3df9832635911c284f637fe0482d774a1c8
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/802086
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520825}
[modify] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/.gitignore
[modify] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/DEPS
[add] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/ios/third_party/motion_interchange_objc/BUILD.gn
[add] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/ios/third_party/motion_interchange_objc/LICENSE
[add] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/ios/third_party/motion_interchange_objc/OWNERS
[add] https://crrev.com/070940de3114ea1ccb895c28c23ec6acd1bcb517/ios/third_party/motion_interchange_objc/README.chromium

Project Member

Comment 21 by bugdroid1@chromium.org, Dec 1 2017

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

commit b4b86231366cdc3b8df956b361d9c6943fd79e57
Author: mrefaat <mrefaat@chromium.org>
Date: Fri Dec 01 20:03:05 2017

Add dependency on Motion Animator for Objective-C.

The library Motion Animator for Objective-C is a new dependency
of the Material Components for iOS. Library description:

  This library provides APIs that turn Motion Interchange motion
  specifications into animations.

Bug:  785248 
Change-Id: I22cbe863f6887c1098d3d33d481dc8788ef0d121
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/801978
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521028}
[modify] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/.gitignore
[modify] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/DEPS
[add] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/ios/third_party/motion_animator_objc/BUILD.gn
[add] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/ios/third_party/motion_animator_objc/LICENSE
[add] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/ios/third_party/motion_animator_objc/OWNERS
[add] https://crrev.com/b4b86231366cdc3b8df956b361d9c6943fd79e57/ios/third_party/motion_animator_objc/README.chromium

Project Member

Comment 22 by bugdroid1@chromium.org, Dec 2 2017

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

commit 33a62f666063f1eb32ec22a785e28c26b22b13d5
Author: mrefaat <mrefaat@chromium.org>
Date: Sat Dec 02 00:32:46 2017

Roll MDFInternationalization to v1.0.3.

Notable change:
-   define IS_BAZEL_BUILD to build as a static library instead of
    building a framework bundle

Bug:  785248 
Change-Id: Ie1aa8ff9439dcae4be1ec8c2185057b4b171b453
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/802694
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521149}
[modify] https://crrev.com/33a62f666063f1eb32ec22a785e28c26b22b13d5/DEPS
[modify] https://crrev.com/33a62f666063f1eb32ec22a785e28c26b22b13d5/ios/third_party/material_components_ios/BUILD.gn
[modify] https://crrev.com/33a62f666063f1eb32ec22a785e28c26b22b13d5/ios/third_party/material_internationalization_ios/BUILD.gn

Blocking: 791096
We also need motion-transitioning-objc.

Sent mail requests for approval to the lists.
CL: https://chromium-review.googlesource.com/c/chromium/src/+/803621
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/8d146c8393932d07b04bfaf179cafbff5159cb9b

commit 8d146c8393932d07b04bfaf179cafbff5159cb9b
Author: Sylvain Defresne <sdefresne@google.com>
Date: Mon Dec 04 13:17:12 2017

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 4 2017

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

commit 67255bc1360d01590552c3fa97aac58619f2288e
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Mon Dec 04 14:54:21 2017

Remove obsolete targets.

Bug:  785248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Icc7ca4649797a1d6f69e0b32af447db6e7a6982e
Reviewed-on: https://chromium-review.googlesource.com/789332
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521337}
[modify] https://crrev.com/67255bc1360d01590552c3fa97aac58619f2288e/ios/third_party/material_internationalization_ios/BUILD.gn

Blocking: 791573
All approval received for motion-transitioning-objc to be mirrored and added to DEPS.

Next steps:

1- Get issue 792259 done to get that repo mirrored to Chromium infra.
2- Once that task is done, push https://chromium-review.googlesource.com/c/chromium/src/+/803621.
3- Get https://github.com/material-components/material-components-ios/pull/2608 approved and merged.
4- Wait for Material release. Let's hope that 2017-12-06's release include the aforementioned fix.
5- Roll MDC with https://chromium-review.googlesource.com/c/chromium/src/+/810047.
1- is done.
2- is waiting for pkl's approval.
3- was rejected. We need to find another solution. I start a discussion doc at go/bling-mdc-imports.
What prompts us to make this a 64 blocker? I know that there are several iPhone X fixes that we should get in when MDC is rolled.

As a mitigation, we could sync trunk then M64 branch to the first release of MDC that added the #imports. That could let us get some iPhone X fixes.

Should I try to see up to which revision I can sync MDC before being broken by the imports?

Comment 31 by pkl@chromium.org, Dec 7 2017

Labels: -ReleaseBlock-Beta
I don't think RBB is warranted since build is not broken and any testing we do with the current version is not going to be invalidated by future changes.

Keep the priority up (P1, or possibly P0) since we need to be able to cherrypick specific fixes if there are RBS in the future.
I agree
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 7 2017

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

commit e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9
Author: Louis Romero <lpromero@chromium.org>
Date: Thu Dec 07 01:28:17 2017

Add dependency on Motion Transitioning for Objective-C.

The library Motion Transitioning for Objective-C is a new dependency
of the Material Components for iOS. Library description:

  Light-weight API for building UIViewController transitions.

Bug:  785248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ifcc89ff8871b1630c7a9feb04c39cc61d9408335
Reviewed-on: https://chromium-review.googlesource.com/803621
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Max Moroz <mmoroz@chromium.org>
Commit-Queue: Louis Romero <lpromero@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522292}
[modify] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/.gitignore
[modify] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/DEPS
[add] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/ios/third_party/motion_transitioning_objc/BUILD.gn
[add] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/ios/third_party/motion_transitioning_objc/LICENSE
[add] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/ios/third_party/motion_transitioning_objc/OWNERS
[add] https://crrev.com/e1d7329eff6eb0747cc5c1d905b5212a3a0ca8c9/ios/third_party/motion_transitioning_objc/README.chromium

Project Member

Comment 34 by bugdroid1@chromium.org, Dec 14 2017

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

commit 352c6305a90e8c42f01e0820468e426945d6021c
Author: Louis Romero <lpromero@chromium.org>
Date: Thu Dec 14 14:44:42 2017

Revert "Remove obsolete targets."

To be able to roll MDC, we'll temporarily build MDFInternationalization
as a framework.
This is the first CL out of 3 CL, including one downstream.

Bug:  785248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ibbacbca687316252e28be985a7952b5449b164bb
Reviewed-on: https://chromium-review.googlesource.com/825202
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Louis Romero <lpromero@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524064}
[modify] https://crrev.com/352c6305a90e8c42f01e0820468e426945d6021c/ios/third_party/material_internationalization_ios/BUILD.gn

Project Member

Comment 35 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/76f60d97cb36d8f1651a726098eef6a7c2c00b76

commit 76f60d97cb36d8f1651a726098eef6a7c2c00b76
Author: Louis Romero <lpromero@google.com>
Date: Fri Dec 15 17:23:55 2017

Project Member

Comment 36 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/6839f58190d34ae09fb2a02d9738fc4edbbb5c9e

commit 6839f58190d34ae09fb2a02d9738fc4edbbb5c9e
Author: Louis Romero <lpromero@google.com>
Date: Tue Dec 19 13:05:48 2017

Project Member

Comment 37 by bugdroid1@chromium.org, Dec 19 2017

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

commit 1799d1d471646c94b083f89c96c116024aae510d
Author: Louis Romero <lpromero@chromium.org>
Date: Tue Dec 19 14:56:26 2017

Remove obsolete targets.

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/789332.

This reverts commit 352c6305a90e8c42f01e0820468e426945d6021c.

Bug:  785248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I44aeb1effc37cd25ca73c67ef1694df48cb8da07
Reviewed-on: https://chromium-review.googlesource.com/833879
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Louis Romero <lpromero@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525030}
[modify] https://crrev.com/1799d1d471646c94b083f89c96c116024aae510d/ios/third_party/material_internationalization_ios/BUILD.gn

Project Member

Comment 38 by bugdroid1@chromium.org, Dec 21 2017

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

commit 2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc
Author: Louis Romero <lpromero@chromium.org>
Date: Thu Dec 21 12:58:59 2017

Roll Material dependencies

This CL rolls different material repositories to their latest releases.
It also adds forwarding headers simulating a framework structure to be able to
compile Material and internal sources together.

Note: I had to add a drainUntilIdle in sign-in EG tests because the newest MDC
releases refactored ActivityIndicator animations. Those are no longer synced
with EG.

Note 2: I also add a remoting dependency on MDC, as it is currently importing
it but didn't declare it in its deps:
https://cs.chromium.org/chromium/src/remoting/ios/session/remoting_client.mm?l=14-15&rcl=8810b5f2950fd505a6916b91e6812792f05bd392

TBR=nicholss@chromium.org

Bug:  785248 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I72f7801aa698bda3acd4c8518a8644528a445245
Reviewed-on: https://chromium-review.googlesource.com/833911
Commit-Queue: Louis Romero <lpromero@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525677}
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/DEPS
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/chrome/test/earl_grey/chrome_earl_grey_ui.mm
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_components_ios/BUILD.gn
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_internationalization_ios/BUILD.gn
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_internationalization_ios/forwarding_headers/MDFInternationalization/MDFInternationalization.h
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_internationalization_ios/forwarding_headers/MDFInternationalization/MDFRTL.h
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_internationalization_ios/forwarding_headers/MDFInternationalization/UIImage+MaterialRTL.h
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_internationalization_ios/forwarding_headers/MDFInternationalization/UIView+MaterialRTL.h
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_text_accessibility_ios/BUILD.gn
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/material_text_accessibility_ios/forwarding_headers/MDFTextAccessibility/MDFTextAccessibility.h
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_animator_objc/BUILD.gn
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_animator_objc/forwarding_headers/MotionAnimator/MotionAnimator.h
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_interchange_objc/BUILD.gn
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_interchange_objc/forwarding_headers/MotionInterchange/MotionInterchange.h
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_transitioning_objc/BUILD.gn
[add] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/ios/third_party/motion_transitioning_objc/forwarding_headers/MotionTransitioning/MotionTransitioning.h
[modify] https://crrev.com/2c7e321cfb14d07f0a54d96941f4cbe8aa25a2fc/remoting/ios/session/BUILD.gn

Status: Fixed (was: Started)
Finally! I had to disable 6 tests around SignIn:
https://crbug.com/796618
https://crbug.com/796842

Sign in to add a comment