Make sure content area is focused when signin-view bubble is opened.

Make sure content area is focused when signin-view bubble is opened.

At present, when the inline login flow opens the bubble-based signin
view, focus is on the "back" arrow, and not the e-mail/password text
fields (as if we are expecting the user didn't really want to navigate
to this view in the first place). This forces the user to manually
focus the desired field through either a mouse-click or a touch before
they can start entering their information.

This CL causes the content area to be focused instead, either the
e-mail text field or th password field (if the e-mail is pre-populated).
This is consistent with how the in-tab signin page displays.

BUG=502370
Committed: https://crrev.com/62d093d69ebe6bb5a92d0bcce6fbf31c115e53be
Cr-Commit-Position: refs/heads/master@{#337271}

[wjmaclean, 2015-07-02 17:58:53]

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

[commit-bot: I haz the power, 2015-07-02 17:59:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1212403004/1

[wjmaclean, 2015-07-02 18:00:20]

wjmaclean@chromium.org changed reviewers:
+ thestig@chromium.org

[wjmaclean, 2015-07-02 18:00:21]

thestig@ Simple view-focus CL (with test) - please take a look?

[wjmaclean, 2015-07-02 18:40:45]

wjmaclean@chromium.org changed reviewers:
- thestig@chromium.org

[wjmaclean, 2015-07-02 18:42:19]

wjmaclean@chromium.org changed reviewers:
+ thakis@chromium.org

[wjmaclean, 2015-07-02 18:42:20]

thakis@ - can you look at a small CL? I originally sent it to thestig@, but just
noticed he's OOO ...

[commit-bot: I haz the power, 2015-07-02 18:58:16]

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

[commit-bot: I haz the power, 2015-07-02 18:58:17]

Dry run: This issue passed the CQ dry run.

[Nico, 2015-07-02 20:07:22]

https://codereview.chromium.org/1212403004/diff/1/chrome/browser/ui/views/pro...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(right):

https://codereview.chromium.org/1212403004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:78: if
(std::string(current->GetClassName()).find("WebView") !=
Can you introduce WebView::kViewClassName and compare to that? (cf FindTabView()
in tab_strip_unittest.cc)

[wjmaclean, 2015-07-02 20:47:19]

Comment addressed, ptal?

https://codereview.chromium.org/1212403004/diff/1/chrome/browser/ui/views/pro...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(right):

https://codereview.chromium.org/1212403004/diff/1/chrome/browser/ui/views/pro...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:78: if
(std::string(current->GetClassName()).find("WebView") !=
On 2015/07/02 20:07:22, Nico wrote:
> Can you introduce WebView::kViewClassName and compare to that? (cf
FindTabView()
> in tab_strip_unittest.cc)

Done.

[wjmaclean, 2015-07-02 20:47:51]

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

[commit-bot: I haz the power, 2015-07-02 20:48:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1212403004/20001

[Nico, 2015-07-02 20:54:58]

lgtm with comment

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(right):

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:80:
.find(views::WebView::kViewClassName) != std::string::npos) {
You can just compare with == now, right?

  if (current->GetClassName() == views::WebView::kViewClassName)

A pointer comparison should be enough.

[commit-bot: I haz the power, 2015-07-02 21:36:21]

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

[commit-bot: I haz the power, 2015-07-02 21:36:22]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2015-07-03 00:56:37]

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(right):

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:80:
.find(views::WebView::kViewClassName) != std::string::npos) {
On 2015/07/02 20:54:57, Nico wrote:
> You can just compare with == now, right?
> 
>   if (current->GetClassName() == views::WebView::kViewClassName)
> 
> A pointer comparison should be enough.

Done.

But I'm generally loathe to compare strings by direct pointer comparison when it
can be avoided, even when the implementation details are known.

[wjmaclean, 2015-07-03 00:56:44]

The CQ bit was checked by wjmaclean@chromium.org

[wjmaclean, 2015-07-03 00:56:45]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1212403004/#ps40001
(title: "Compare strings by direct pointer comparison.")

[commit-bot: I haz the power, 2015-07-03 00:56:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1212403004/40001

[Nico, 2015-07-03 01:00:24]

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
(right):

https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:80:
.find(views::WebView::kViewClassName) != std::string::npos) {
On 2015/07/03 00:56:37, wjmaclean wrote:
> On 2015/07/02 20:54:57, Nico wrote:
> > You can just compare with == now, right?
> > 
> >   if (current->GetClassName() == views::WebView::kViewClassName)
> > 
> > A pointer comparison should be enough.
> 
> Done.

Thanks!

> But I'm generally loathe to compare strings by direct pointer comparison when
it
> can be avoided, even when the implementation details are known.

I guess there's two ways to look at GetClassName()

1. It returns a token that can be compared with == and that happens to be a
valid string when used as a pointer and dereferenced

2. It returns a string

For a hand-rolled RTTI system, view 1 is imho much preferable, as 2 requires
string comparisons as you say, and RTTI systems that need string comparisons to
check for type equality aren't great.

[wjmaclean, 2015-07-03 01:31:19]

On 2015/07/03 01:00:24, Nico wrote:
>
https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
> File chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc
> (right):
> 
>
https://codereview.chromium.org/1212403004/diff/20001/chrome/browser/ui/views...
> chrome/browser/ui/views/profiles/profile_chooser_view_browsertest.cc:80:
> .find(views::WebView::kViewClassName) != std::string::npos) {
> On 2015/07/03 00:56:37, wjmaclean wrote:
> > On 2015/07/02 20:54:57, Nico wrote:
> > > You can just compare with == now, right?
> > > 
> > >   if (current->GetClassName() == views::WebView::kViewClassName)
> > > 
> > > A pointer comparison should be enough.
> > 
> > Done.
> 
> Thanks!
> 
> > But I'm generally loathe to compare strings by direct pointer comparison
when
> it
> > can be avoided, even when the implementation details are known.
> 
> I guess there's two ways to look at GetClassName()
> 
> 1. It returns a token that can be compared with == and that happens to be a
> valid string when used as a pointer and dereferenced
> 
> 2. It returns a string
> 
> For a hand-rolled RTTI system, view 1 is imho much preferable, as 2 requires
> string comparisons as you say, and RTTI systems that need string comparisons
to
> check for type equality aren't great.

I suppose we could have compared

current->kViewClassName == WebView::kViewClassName

or, if GetClassName() had been declared static,

current->GetClassName() == WebView::GetClassName()

either of which might have had the advantage of making it obvious we were
comparing compatible tokens (if we take your viewpoint #1).

But I doubt it will ultimately cause us any grief, so I'm not going to worry
about it. :-)

[commit-bot: I haz the power, 2015-07-03 01:31:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-07-03 01:32:40]

Patchset 3 (id:??) landed as
https://crrev.com/62d093d69ebe6bb5a92d0bcce6fbf31c115e53be
Cr-Commit-Position: refs/heads/master@{#337271}

[wjmaclean, 2015-07-03 01:43:19]

On 2015/07/03 01:31:19, wjmaclean wrote:
> I suppose we could have compared
> 
> current->kViewClassName == WebView::kViewClassName

Gah! Ignore this ... of course it won't work ...
 
> or, if GetClassName() had been declared static,
> 
> current->GetClassName() == WebView::GetClassName()

And obviously (for similar reasons) it makes no sense to expect we could make
GetClassName static.

Oh well.