[iOS Clean] Added dialogs UI support.

[iOS Clean] Added dialogs UI support.

This CL adds an interface for UI elements that can display dialogs.
Coordinators utilizing these classes will be added in subsequent CLs.

BUG=none

[kkhorimoto, 2017-06-07 08:02:29]

kkhorimoto@chromium.org changed reviewers:
+ edchin@chromium.org, marq@chromium.org

[kkhorimoto, 2017-06-08 00:37:17]

Added DialogMediator

[marq (ping after 24h), 2017-06-14 10:52:16]

Overall this is a direction I like, just the usual raft of tweaks.

I expected to see a DIalogCoordinator superclass as well, but I'll see how the
other CLs look.

Beyond that, the major change I suggest is to not have the dialog mediator have
the dispatcher, but rather have the coordinator set up dispatching to it. That
may mean a DialogCoordinator superclass is more useful.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/commands/dialog_commands.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/commands/dialog_commands.h:9: @protocol
DialogDismissalCommands
Maybe just -DialogCommands, so if (as seems likely) we have a -showDialog ... 
command, it has a logical home.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/commands/dialog_commands.h:11: // Called to dismiss
the dialog.  |buttonTag| is the tag of the DialogButtonItem
Commands shouldn't presume a particular UI implementation. I'm fine if there's
an opaque 'dialog identifier' that's used, but how it's generated, and what it
corresponds to in the UI (if anything) isn't part of the command definition.

Also, 'tag' should be reserved for its use in the UIView sense (an integer
identifier), so calling something a 'tag' that's an object type (id) is
confusing.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:11: enum class
DialogButtonStyle : short { DEFAULT, CANCEL, DESTRUCTIVE };
Define the semantics of each value.

Set an explicit value for the first item.

Side note: Should we standardize on 'enum class' in the new architecture?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:13: // An object
encapsulating the data necessary to set up a dialog's text field.
Class is for a 'button item', but the comment references a 'text field'. Please
clarify.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:16: // Factory method
for item creation.
Document the preconditions you are DCHECKING for in the initailizer

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm:34: _text = text;
[text copy]

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm:36: _tag = tag;
using a totally opaque type is slightly tricky here -- what if it's mutable,
retained by another object, and changes? Really we want this to be some kind of
opaque *value* type.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_consumer.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_consumer.h:14: // dialog.
Document what the expectations are around calling these methods on a consumer
more than once.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h:15: //
DialogMediator functionality exposed to subclasses.
Document that these methods/properties do nothing by default, and that
subclasses should override them.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h:21: // The title
to provide to the consumer.
Should these just be readonly properties?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.h:13: // Class responsible
for setting up a DialogConsumer.
Document that this class is expected to be subclassed, and that subclasses must
use the +Internal category, etc,

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm:40: [consumer
setDialogTitle:[self dialogTitle]];
Usually the mediator is created or handed a pointer to the consumer that it
keeps, often at init time.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm:47: [self.dispatcher
startDispatchingToTarget:self
Starting/stopping dispatch based on coordinator start/stop should be done in the
coordinator -- it's fine for a coordinator to arrange dispatch to objects other
than itself.

This means that this class should just publicly conform to <DialogCommands>
instead of taking the dispatcher on init.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:11: @interface
DialogTextFieldItem : NSObject
Consider changing "Item" to "Configuration", just to clarify that this object
just tells the UI what to show.

We should be consistent with how collection views are handled, however -- I'm
generally thinking that 'Configuration' is what the consumer protocol sends to
the UI layer, 'Item" is what the UI layer's internal model (if any) uses, and
the actual view-layer objects use the conventional names (cell, button, label,
etc).

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:22:
@property(nonatomic, readonly, strong) NSString* defaultText;
Technically these should also be 'copy' (same for the button item).

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:27: // Whether the
text field should be secure (f.e. for password).
nit: expand 'f.e.,', or use the more pretentious and opaque Latin abbreviation
"e.g.".

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.mm:30: _defaultText =
defaultText;
copy strings.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:14: // Class used
to display dialogs.
Is this expected to be subclassed?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:17: // Initializer
for a dialog with |style| that uses |dispatcher| to manager its
mananger -> manage

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:19: -
(instancetype)initWithStyle:(UIAlertControllerStyle)style
Style should always be UIAlertControllerStyleAlert, right?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:38:
dismissalDispatcher;
anti-nit: Convention so far has just been to name the dispatcher property in
view controllers 'dispatcher', but I actually like that you're purposefully
naming it here -- so this is a nit on every other view controller, but not this
one.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:41:
@property(nonatomic, readonly, strong) NSArray<DialogButtonItem*>* buttonItems;
Why not readwrite, copy for all of these?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:46: // The strings
corresponding with the text fields.
Maybe use a dictionary rather than depending on index correspondences?

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:89:
_userInputStrings = [[NSMutableArray alloc] initWithCapacity:itemCount];
Newline after brace-less if().

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:129: __weak
DialogViewController* weakSelf = self;
You don't need to do the weak/strong dance for dialog actions. The retain cycle
will resolve itself. 

(and, even then, you don't need to strongify for a single method call).

[kkhorimoto, 2017-06-23 06:24:03]

rebase & Mark's comments

[kkhorimoto, 2017-06-23 06:24:19]

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/commands/dialog_commands.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/commands/dialog_commands.h:9: @protocol
DialogDismissalCommands
On 2017/06/14 10:52:14, marq (ping after 24h) wrote:
> Maybe just -DialogCommands, so if (as seems likely) we have a -showDialog ... 
> command, it has a logical home.

I don't think there will be a |-showDialog| command, as the DialogCoordinator is
intended to be subclassed.  There might be commands in the future to show
specific types of dialogs, but they should be specific to the subclass rather
than the generalized dialogs support implemented by DialogConsumers.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/commands/dialog_commands.h:11: // Called to dismiss
the dialog.  |buttonTag| is the tag of the DialogButtonItem
On 2017/06/14 10:52:14, marq (ping after 24h) wrote:
> Commands shouldn't presume a particular UI implementation. I'm fine if there's
> an opaque 'dialog identifier' that's used, but how it's generated, and what it
> corresponds to in the UI (if anything) isn't part of the command definition.
> 
> Also, 'tag' should be reserved for its use in the UIView sense (an integer
> identifier), so calling something a 'tag' that's an object type (id) is
> confusing.

I changed tag to identifier and updated the comments to make them a little less
tightly coupled with UI assumptions.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:11: enum class
DialogButtonStyle : short { DEFAULT, CANCEL, DESTRUCTIVE };
On 2017/06/14 10:52:14, marq (ping after 24h) wrote:
> Define the semantics of each value.
> 
> Set an explicit value for the first item.
> 
> Side note: Should we standardize on 'enum class' in the new architecture?

Done.  I definitely think we should be using enum classes over normal enums. 
The only situation I can think of where I still use normal enums is for array
index constant (e.g. DeprecatedSerializationIndices in
crw_session_certificate_policy_cache_storage.mm).  These are usually
implementation details though, so I think it would be reasonable to use only
enum classes in header files.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:13: // An object
encapsulating the data necessary to set up a dialog's text field.
On 2017/06/14 10:52:14, marq (ping after 24h) wrote:
> Class is for a 'button item', but the comment references a 'text field'.
Please
> clarify.

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.h:16: // Factory method
for item creation.
On 2017/06/14 10:52:14, marq (ping after 24h) wrote:
> Document the preconditions you are DCHECKING for in the initailizer

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm:34: _text = text;
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> [text copy]

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_button_item.mm:36: _tag = tag;
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> using a totally opaque type is slightly tricky here -- what if it's mutable,
> retained by another object, and changes? Really we want this to be some kind
of
> opaque *value* type. 

The identifier is supplied by and sent back to the coordinator layer.  Since
it's supplied to the consumer with no type information, only the coordinator can
actually modify the identifier object, so I think it's reasonable to expect that
if a coordinator modifies the identifier somehow, it does it in a way in which
the buttons can still be uniquely identified.  Moreover, even if the objects
supplied are mutable, it wouldn't really make a different in the current
implementation, since it's relying on direct pointer comparison rather than any
selectors.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_consumer.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_consumer.h:14: // dialog.
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Document what the expectations are around calling these methods on a consumer
> more than once. 

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h:15: //
DialogMediator functionality exposed to subclasses.
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Document that these methods/properties do nothing by default, and that
> subclasses should override them.

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator+internal.h:21: // The title
to provide to the consumer.
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Should these just be readonly properties?

The main reason was that I couldn't decide on what ARC annotation to use.  The
implementation of these functions return autoreleased values or NSString
literals.  I could make these readonly weak properties, but it seems kind of odd
to use a weak property for a factory method.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.h:13: // Class responsible
for setting up a DialogConsumer.
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Document that this class is expected to be subclassed, and that subclasses
must
> use the +Internal category, etc,

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm:40: [consumer
setDialogTitle:[self dialogTitle]];
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Usually the mediator is created or handed a pointer to the consumer that it
> keeps, often at init time.

I was going to do that, but ran into style guide violations.  If we were to
update the consumer in |-init|, it makes most sense for this to happen in the
DialogMediator superclass, not the subclasses.  However, the subclasses have not
been initialized yet, so these getters are not available in the superclass's
initializer.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_mediator.mm:47: [self.dispatcher
startDispatchingToTarget:self
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Starting/stopping dispatch based on coordinator start/stop should be done in
the
> coordinator -- it's fine for a coordinator to arrange dispatch to objects
other
> than itself.
> 
> This means that this class should just publicly conform to <DialogCommands>
> instead of taking the dispatcher on init.

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:11: @interface
DialogTextFieldItem : NSObject
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Consider changing "Item" to "Configuration", just to clarify that this object
> just tells the UI what to show.
> 
> We should be consistent with how collection views are handled, however -- I'm
> generally thinking that 'Configuration' is what the consumer protocol sends to
> the UI layer, 'Item" is what the UI layer's internal model (if any) uses, and
> the actual view-layer objects use the conventional names (cell, button, label,
> etc).

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:22:
@property(nonatomic, readonly, strong) NSString* defaultText;
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Technically these should also be 'copy' (same for the button item).

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.h:27: // Whether the
text field should be secure (f.e. for password).
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> nit: expand 'f.e.,', or use the more pretentious and opaque Latin abbreviation
> "e.g.".

Went with the pretentious option :P

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_text_field_item.mm:30: _defaultText =
defaultText;
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> copy strings.

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:14: // Class used
to display dialogs.
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Is this expected to be subclassed?

No, I'll add a comment for that.  It should be more clear that only the
coordinator should be subclassed (now that there's a DialogCoordinator).

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:17: // Initializer
for a dialog with |style| that uses |dispatcher| to manager its
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> mananger -> manage

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.h:19: -
(instancetype)initWithStyle:(UIAlertControllerStyle)style
On 2017/06/14 10:52:15, marq (ping after 24h) wrote:
> Style should always be UIAlertControllerStyleAlert, right?

Nope, the dialog blocking confirmation is an action sheet on iPhones.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
File ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm (right):

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:38:
dismissalDispatcher;
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> anti-nit: Convention so far has just been to name the dispatcher property in
> view controllers 'dispatcher', but I actually like that you're purposefully
> naming it here -- so this is a nit on every other view controller, but not
this
> one.

Acknowledged.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:41:
@property(nonatomic, readonly, strong) NSArray<DialogButtonItem*>* buttonItems;
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> Why not readwrite, copy for all of these?

Moved to copy, but I left as readonly to discourage writing outside of the
DialogConsumer functions.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:46: // The strings
corresponding with the text fields.
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> Maybe use a dictionary rather than depending on index correspondences?

Done.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:89:
_userInputStrings = [[NSMutableArray alloc] initWithCapacity:itemCount];
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> Newline after brace-less if().

Is this part of the style guide?  I don't mind adding newlines/braces for
iteration loops since those are less commonly seen and the added formatting
helps, but it seems a bit excessive for small if statements, especially if
they're one-line lazy instantiation conditions like this.  It takes up move
space but doesn't really increase readbility.

https://codereview.chromium.org/2929563002/diff/20001/ios/clean/chrome/browse...
ios/clean/chrome/browser/ui/dialogs/dialog_view_controller.mm:129: __weak
DialogViewController* weakSelf = self;
On 2017/06/14 10:52:16, marq (ping after 24h) wrote:
> You don't need to do the weak/strong dance for dialog actions. The retain
cycle
> will resolve itself. 
> 
> (and, even then, you don't need to strongify for a single method call).

Done.