Use (i) icon and the same tooltip for the credit cards dialog and account chooser on Mac.

Use (i) icon and the same tooltip for the credit cards dialog and account chooser on Mac.

BUG=690920
Review-Url: https://codereview.chromium.org/2703253007
Cr-Commit-Position: refs/heads/master@{#453972}
Committed: https://chromium.googlesource.com/chromium/src/+/64229f842420145f8cc6ba4c35298f45b27d282d

[vasilii, 2017-02-21 13:38:36]

The CQ bit was checked by vasilii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 13:38:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vasilii, 2017-02-21 13:49:23]

vasilii@chromium.org changed reviewers:
+ estade@chromium.org, groby@chromium.org

[vasilii, 2017-02-21 13:49:23]

estade@chromium.org: Please review changes in 
autofill_scaled_resources.grdp

groby@chromium.org: Please review everything

[commit-bot: I haz the power, 2017-02-21 14:23:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 14:23:43]

Dry run: This issue passed the CQ dry run.

[groby-ooo-7-16, 2017-02-23 00:22:49]

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.h (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.h:27: -
(NSView*)enableIcon:(NSString*)tooltip;
Shouldn't this be called something like "addInfoIcon:"? It's modifying the
dialog, not just enabling something.

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.mm (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:136:
iconController_.reset([[AutofillTooltipController alloc]
Is this supposed to be called multiple times? If not, could you DCHECK
|iconController_|?

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:142: [self
addSubview:[iconController_ view]];
This means self already takes ownership - do you really need a member variable
to keep a reference?

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
File components/resources/autofill_scaled_resources.grdp (right):

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
components/resources/autofill_scaled_resources.grdp:40: <structure
type="chrome_scaled_image" name="IDR_AUTOFILL_TOOLTIP_ICON"
file="autofill/autofill_tooltip_icon.png" />
Any chance to make this an SVG resource?

[nsphu.cnht, 2017-02-24 12:38:22]

nsphu.cnht@gmail.com changed reviewers:
+ nsphu.cnht@gmail.com

[nsphu.cnht, 2017-02-24 12:38:22]

lgtm

[nsphu.cnht, 2017-02-24 12:40:16]

lgtm

[nsphu.cnht, 2017-02-24 12:40:19]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:40:24]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:40:52]

lgtm

[nsphu.cnht, 2017-02-24 12:40:54]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:40:54]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:44:10]

lgtm

[nsphu.cnht, 2017-02-24 12:44:11]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:44:13]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:50:40]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:50:40]

lgtm

[nsphu.cnht, 2017-02-24 12:52:20]

lgtm

[nsphu.cnht, 2017-02-24 12:52:21]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:22]

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:24]

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:26]

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:27]

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:30]

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:52:32]

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:57:43]

lgtm

[nsphu.cnht, 2017-02-24 12:57:44]

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:57:46]

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:57:46]

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:57:48]

lgtm

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:58:40]

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:58:44]

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:58:44]

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 12:58:45]

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

lgtm

[nsphu.cnht, 2017-02-24 14:05:45]

On 2017/02/21 13:38:36, vasilii wrote:
> The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry run

[nsphu.cnht, 2017-02-24 14:22:17]

On 2017/02/24 14:05:45, nsphu.cnht wrote:
> On 2017/02/21 13:38:36, vasilii wrote:
> > The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry run

[nsphu.cnht, 2017-02-24 14:28:31]

On 2017/02/24 14:22:17, nsphu.cnht wrote:
> On 2017/02/24 14:05:45, nsphu.cnht wrote:
> > On 2017/02/21 13:38:36, vasilii wrote:
> > > The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry run

690920

[nsphu.cnht, 2017-02-24 14:31:45]

On 2017/02/24 14:28:31, nsphu.cnht wrote:
> On 2017/02/24 14:22:17, nsphu.cnht wrote:
> > On 2017/02/24 14:05:45, nsphu.cnht wrote:
> > > On 2017/02/21 13:38:36, vasilii wrote:
> > > > The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry
run
> 
> 690920

[nsphu.cnht, 2017-02-24 14:56:31]

On 2017/02/24 14:05:45, nsphu.cnht wrote:
> On 2017/02/21 13:38:36, vasilii wrote:
> > The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry run

[nsphu.cnht, 2017-02-24 15:06:38]

On 2017/02/24 14:56:31, nsphu.cnht wrote:
> On 2017/02/24 14:05:45, nsphu.cnht wrote:
> > On 2017/02/21 13:38:36, vasilii wrote:
> > > The CQ bit was checked by mailto:vasilii@chromium.org to run a CQ dry run

[nsphu.cnht, 2017-02-24 15:09:15]

lgtm

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.h (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.h:27: -
(NSView*)enableIcon:(NSString*)tooltip;
On 2017/02/23 00:22:49, groby wrote:
> Shouldn't this be called something like "addInfoIcon:"? It's modifying the
> dialog, not just enabling something.

Done.

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.mm (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:136:
iconController_.reset([[AutofillTooltipController alloc]
On 2017/02/23 00:22:49, groby wrote:
> Is this supposed to be called multiple times? If not, could you DCHECK
> |iconController_|?

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:142: [self
addSubview:[iconController_ view]];
On 2017/02/23 00:22:49, groby wrote:
> This means self already takes ownership - do you really need a member variable
> to keep a reference?

Acknowledged.

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:142: [self
addSubview:[iconController_ view]];
On 2017/02/23 00:22:49, groby wrote:
> This means self already takes ownership - do you really need a member variable
> to keep a reference?

Done.

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
File components/resources/autofill_scaled_resources.grdp (right):

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
components/resources/autofill_scaled_resources.grdp:40: <structure
type="chrome_scaled_image" name="IDR_AUTOFILL_TOOLTIP_ICON"
file="autofill/autofill_tooltip_icon.png" />
On 2017/02/23 00:22:49, groby wrote:
> Any chance to make this an SVG resource?

Acknowledged.

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
components/resources/autofill_scaled_resources.grdp:40: <structure
type="chrome_scaled_image" name="IDR_AUTOFILL_TOOLTIP_ICON"
file="autofill/autofill_tooltip_icon.png" />
On 2017/02/23 00:22:49, groby wrote:
> Any chance to make this an SVG resource?

Acknowledged.

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
components/resources/autofill_scaled_resources.grdp:40: <structure
type="chrome_scaled_image" name="IDR_AUTOFILL_TOOLTIP_ICON"
file="autofill/autofill_tooltip_icon.png" />
On 2017/02/23 00:22:49, groby wrote:
> Any chance to make this an SVG resource?
>690920
Done.

[nsphu.cnht, 2017-02-24 15:09:51]

690920

[vasilii, 2017-02-24 17:52:31]

The CQ bit was checked by vasilii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 17:52:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vasilii, 2017-02-24 17:53:00]

vasilii@chromium.org changed reviewers:
- nsphu.cnht@gmail.com

[vasilii, 2017-02-24 17:53:01]

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.h (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.h:27: -
(NSView*)enableIcon:(NSString*)tooltip;
On 2017/02/23 00:22:49, groby wrote:
> Shouldn't this be called something like "addInfoIcon:"? It's modifying the
> dialog, not just enabling something.

Done.

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
File chrome/browser/ui/cocoa/passwords/credential_item_button.mm (right):

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:136:
iconController_.reset([[AutofillTooltipController alloc]
On 2017/02/23 00:22:49, groby wrote:
> Is this supposed to be called multiple times? If not, could you DCHECK
> |iconController_|?

Done.

https://codereview.chromium.org/2703253007/diff/1/chrome/browser/ui/cocoa/pas...
chrome/browser/ui/cocoa/passwords/credential_item_button.mm:142: [self
addSubview:[iconController_ view]];
On 2017/02/23 00:22:49, groby wrote:
> This means self already takes ownership - do you really need a member variable
> to keep a reference?

But I own the controller. If I understand correctly the view doesn't hold a
string reference to its controller but the other way around.

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
File components/resources/autofill_scaled_resources.grdp (right):

https://codereview.chromium.org/2703253007/diff/1/components/resources/autofi...
components/resources/autofill_scaled_resources.grdp:40: <structure
type="chrome_scaled_image" name="IDR_AUTOFILL_TOOLTIP_ICON"
file="autofill/autofill_tooltip_icon.png" />
On 2017/02/23 00:22:49, groby wrote:
> Any chance to make this an SVG resource?

Currently vector_icons/ isn't compiled on iOS. Thus, not in this CL. In general
I can ask the Bling folks if you want.

[commit-bot: I haz the power, 2017-02-24 18:20:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 18:20:40]

Dry run: This issue passed the CQ dry run.

[vasilii, 2017-02-27 11:59:36]

The comments are addressed. Ignore the spammer.

[Evan Stade, 2017-02-27 15:34:40]

On 2017/02/27 11:59:36, vasilii wrote:
> The comments are addressed. Ignore the spammer.

grdp file lgtm

[vasilii, 2017-02-28 13:37:39]

Rachel?

[vasilii, 2017-03-01 14:42:12]

vasilii@chromium.org changed reviewers:
+ avi@chromium.org

[vasilii, 2017-03-01 14:42:13]

Hi Avi,

could you review?

[Avi (use Gerrit), 2017-03-01 16:07:41]

LGTM

[vasilii, 2017-03-01 16:11:53]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2017-03-01 16:11:53]

The patchset sent to the CQ was uploaded after l-g-t-m from nsphu.cnht@gmail.com
Link to the patchset: https://codereview.chromium.org/2703253007/#ps20001
(title: "comments")

[commit-bot: I haz the power, 2017-03-01 16:12:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 16:14:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 16:15:00]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[nsphu.cnht, 2017-03-01 16:56:53]

nsphu.cnht@gmail.com changed reviewers:
+ nsphu.cnht@gmail.com

[nsphu.cnht, 2017-03-01 16:56:54]

lgtm

[vasilii, 2017-03-01 17:32:24]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2017-03-01 17:32:24]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
nsphu.cnht@gmail.com, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/2703253007/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-03-01 17:33:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 18:05:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488389544269320,
"parent_rev": "e4115eb7314d12ffa24ae0be0d192f2712693cf0", "commit_rev":
"64229f842420145f8cc6ba4c35298f45b27d282d"}

[commit-bot: I haz the power, 2017-03-01 18:06:08]

Description was changed from

==========
Use (i) icon and the same tooltip for the credit cards dialog and account
chooser on Mac.

BUG=690920
==========

to

==========
Use (i) icon and the same tooltip for the credit cards dialog and account
chooser on Mac.

BUG=690920

Review-Url: https://codereview.chromium.org/2703253007
Cr-Commit-Position: refs/heads/master@{#453972}
Committed:
https://chromium.googlesource.com/chromium/src/+/64229f842420145f8cc6ba4c3529...
==========

[commit-bot: I haz the power, 2017-03-01 18:06:11]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/64229f842420145f8cc6ba4c3529...