[Password Manager] Check node visibility with isFocusable instead of hasNonEmptyLayoutSize

[Password Manager] Check node visibility with isFocusable instead of hasNonEmptyLayoutSize

Autofill already uses isFocusable

BUG=593246, 589533
Review-Url: https://codereview.chromium.org/2769023003
Cr-Commit-Position: refs/heads/master@{#460064}
Committed: https://chromium.googlesource.com/chromium/src/+/7717d24507eb8aa830a8fdf7d5c202a43a8803ae

[kolos1, 2017-03-23 10:03:10]

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

[commit-bot: I haz the power, 2017-03-23 10:03:27]

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

[kolos1, 2017-03-23 10:04:10]

Description was changed from

==========
Merge branch 'master' of https://chromium.googlesource.com/chromium/src into
is_focusable
[Password Manager] Check node visibility with isFocusable instead of
hasNonEmptyLayoutSize


Autofill already uses isFocusable

BUG=593246, 589533
==========

to

==========
[Password Manager] Check node visibility with isFocusable instead of
hasNonEmptyLayoutSize

Autofill already uses isFocusable

BUG=593246, 589533
==========

[kolos1, 2017-03-23 12:32:37]

Patchset #3 (id:40001) has been deleted

[kolos1, 2017-03-23 12:32:45]

Patchset #1 (id:1) has been deleted

[kolos1, 2017-03-23 12:32:51]

Patchset #1 (id:20001) has been deleted

[kolos1, 2017-03-23 12:33:19]

kolos@chromium.org changed reviewers:
+ vabr@chromium.org

[kolos1, 2017-03-23 12:33:20]

Hi Vaclav,

Please review.

Regards,
Maxim

[kolos1, 2017-03-23 12:48:46]

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

[commit-bot: I haz the power, 2017-03-23 12:49:04]

Dry run: 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-23 14:24:34]

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

[commit-bot: I haz the power, 2017-03-23 14:24:35]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2017-03-23 20:14:10]

Thanks, Maxim.

This LGTM, but please check whether hasNonEmptyLayoutSize cannot cause layout,
to prevent reviving https://crbug.com/595078.

Cheers,
Vaclav

https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
components/autofill/content/renderer/form_autofill_util.cc:1337: bool res =
g_prevent_layout ? element.hasNonEmptyLayoutSize()
I don't know whether hasNonEmptyLayoutSize can trigger layout or not. Did you
check with somebody who knows?

[kolos1, 2017-03-24 14:02:35]

Patchset #2 (id:80001) has been deleted

[kolos1, 2017-03-24 14:03:49]

Patchset #2 (id:100001) has been deleted

[kolos1, 2017-03-24 14:05:29]

Patchset #2 (id:120001) has been deleted

[kolos1, 2017-03-24 14:06:09]

Patchset #2 (id:80002) has been deleted

[kolos1, 2017-03-24 14:06:37]

Patchset #2 (id:150001) has been deleted

[kolos1, 2017-03-28 09:34:30]

https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
File components/autofill/content/renderer/form_autofill_util.cc (right):

https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
components/autofill/content/renderer/form_autofill_util.cc:1337: bool res =
g_prevent_layout ? element.hasNonEmptyLayoutSize()
Added a comment here. Please review.

[vabr (Chromium), 2017-03-28 10:13:33]

On 2017/03/28 09:34:30, kolos1 wrote:
>
https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
> File components/autofill/content/renderer/form_autofill_util.cc (right):
> 
>
https://codereview.chromium.org/2769023003/diff/60001/components/autofill/con...
> components/autofill/content/renderer/form_autofill_util.cc:1337: bool res =
> g_prevent_layout ? element.hasNonEmptyLayoutSize()
> Added a comment here. Please review.

LGTM, thanks!

[kolos1, 2017-03-28 10:34:12]

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

[commit-bot: I haz the power, 2017-03-28 10:34:22]

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

[kolos1, 2017-03-28 11:25:56]

The CQ bit was unchecked by kolos@chromium.org

[kolos1, 2017-03-28 11:25:56]

The CQ bit was checked by kolos@chromium.org

[commit-bot: I haz the power, 2017-03-28 11:26:14]

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-28 12:16:47]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1490700356795250,
"parent_rev": "40ccdb1b22185c9b07ddf1702c52afd4ffe0412e", "commit_rev":
"7717d24507eb8aa830a8fdf7d5c202a43a8803ae"}

[commit-bot: I haz the power, 2017-03-28 12:21:23]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1490700356795250,
"parent_rev": "40ccdb1b22185c9b07ddf1702c52afd4ffe0412e", "commit_rev":
"7717d24507eb8aa830a8fdf7d5c202a43a8803ae"}

[commit-bot: I haz the power, 2017-03-28 12:21:25]

Description was changed from

==========
[Password Manager] Check node visibility with isFocusable instead of
hasNonEmptyLayoutSize

Autofill already uses isFocusable

BUG=593246, 589533
==========

to

==========
[Password Manager] Check node visibility with isFocusable instead of
hasNonEmptyLayoutSize

Autofill already uses isFocusable

BUG=593246, 589533

Review-Url: https://codereview.chromium.org/2769023003
Cr-Commit-Position: refs/heads/master@{#460064}
Committed:
https://chromium.googlesource.com/chromium/src/+/7717d24507eb8aa830a8fdf7d5c2...
==========

[commit-bot: I haz the power, 2017-03-28 12:21:26]

Committed patchset #2 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/7717d24507eb8aa830a8fdf7d5c2...