Omnibox improvements and fixes.

Omnibox improvements and fixes.

- Show the correct security icon on the omnibar when out of WebVR.  This
  change uses the tab's security level to determine whether the security
  warnings are shown, rather than the secure origin flag that was
  plumbed through the WebVR code path.
- Keep the omnibox visible at all times in 2D browsing.
- Take URLs only from the main frame on a page.

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/340d1da550dbf8290c7da82add22af59b0fa47e4
Cr-Commit-Position: refs/heads/master@{#436974}

[cjgrant, 2016-11-29 16:18:00]

Description was changed from

==========
Omnibox improvements and fixes.

- Show the correct security icon on the omnibar when out of WebVR.  This
  change uses the tab's security level to determine whether the security
  warnings are shown, rather than the secure origin flag that was
  plumbed through the WebVR code path.
- Keep the omnibox visible at all times in 2D browsing.
- Take URLs only from the main frame on a page.

BUG=641508
==========

to

==========
Omnibox improvements and fixes.

- Show the correct security icon on the omnibar when out of WebVR.  This
  change uses the tab's security level to determine whether the security
  warnings are shown, rather than the secure origin flag that was
  plumbed through the WebVR code path.
- Keep the omnibox visible at all times in 2D browsing.
- Take URLs only from the main frame on a page.

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[cjgrant, 2016-11-29 16:33:52]

cjgrant@chromium.org changed reviewers:
+ bajones@chromium.org, clamy@chromium.org, dcheng@chromium.org,
klausw@chromium.org, mthiesse@chromium.org

[cjgrant, 2016-11-29 16:33:52]

https://codereview.chromium.org/2536223002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:61: const auto*
helper = SecurityStateTabHelper::FromWebContents(web_contents());
dcheng@ and clamy@:

This bit of code works for demo purposes, but I'm not comfortable with it, and
would like your advice.

In https://codereview.chromium.org/2361533003, you discuss how isSecureContext()
is currently generated by the renderer, but will (in the future) be made
available to the browser process (crbug.com/650203 tracks this).  I don't know
the difference between isSecureContext(), and the security level I'm querying
with my CL.  So far, I know that this method yields the same basic result for
https and non-https WebVR sites (ie. warnings are shown only on http sites).

Assuming that what I'm doing here is okay, my next question is, why isn't
security level exposes via a WebContentsObserver callback?  WebContentsObserver
exposes something called Security "Style" - I haven't yet figured out what that
is, but it's not the level.  :)

Finally, please note that the code here driving the omnibox is demo-grade. 
Eventually I expect we'll have a fully-functional omnibox that's similar to
Clank's version, but done solely in C++.  Details are a separate, huge
discussion.

[mthiesse, 2016-11-29 16:40:19]

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:916: // TODO(cjgrant): Remove this
method, and all related plumbing back to the
hurray!

[mthiesse, 2016-11-29 16:40:32]

lgtm

[cjgrant, 2016-11-29 16:53:12]

Brandon, you had mentioned that this change should preserve "exceptions for
localhost".  Can you elaborate a bit, Chrome-newb style?  What's the easiest way
to verify I'm not degrading things?

[bajones, 2016-11-29 17:53:23]

LGTM, as long as the localhost scenario works.

What I meant was that WebVR content run from localhost should be treated as
secure and not show the warning. This is crucial for developers, who obviously
don't want to have to push to a public server repeatedly during development.

To test, you could clone some WebVR content to your local machine
(https://github.com/toji/webvr.info should work well) and run a local server
("python -m SimpleHTTPServer" FTW!) The open an instance of Chrome on that same
machine, visit "about:inspect" and ensure that port forwarding is enabled and is
forwarding the right ports. Now if you connect a phone via USB cable and use it
to browse to localhost:8000 (or whatever port you're serving up) you should see
the site served from the desktop machine but presented as if it's localhost from
the phone. Now you can enter VR mode on one of the samples and see if it causes
the warning to show up.

If you're not familiar with the process it's a pain to get going the first time,
so don't hesitate to ask if you run into problems!

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:916: // TODO(cjgrant): Remove this
method, and all related plumbing back to the
On 2016/11/29 16:40:19, mthiesse wrote:
> hurray!

+1!

[cjgrant, 2016-11-29 17:57:42]

On 2016/11/29 17:53:23, bajones wrote:
> LGTM, as long as the localhost scenario works.
> 
> What I meant was that WebVR content run from localhost should be treated as
> secure and not show the warning. This is crucial for developers, who obviously
> don't want to have to push to a public server repeatedly during development.
> 
> To test, you could clone some WebVR content to your local machine
> (https://github.com/toji/webvr.info should work well) and run a local server
> ("python -m SimpleHTTPServer" FTW!) The open an instance of Chrome on that
same
> machine, visit "about:inspect" and ensure that port forwarding is enabled and
is
> forwarding the right ports. Now if you connect a phone via USB cable and use
it
> to browse to localhost:8000 (or whatever port you're serving up) you should
see
> the site served from the desktop machine but presented as if it's localhost
from
> the phone. Now you can enter VR mode on one of the samples and see if it
causes
> the warning to show up.
> 
> If you're not familiar with the process it's a pain to get going the first
time,
> so don't hesitate to ask if you run into problems!
> 
>
https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
> File chrome/browser/android/vr_shell/vr_shell.cc (right):
> 
>
https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
> chrome/browser/android/vr_shell/vr_shell.cc:916: // TODO(cjgrant): Remove this
> method, and all related plumbing back to the
> On 2016/11/29 16:40:19, mthiesse wrote:
> > hurray!
> 
> +1!

I'm definitely familiar with the port forwarding and local-hosting parts.  I'm
hazier on the content part, because if I host my own simple test page via
localhost, it shows as insecure (both when viewed via the desktop browser and
via the phone's forwarded port).  I'll try it with the WebVR content to be sure.
 Thanks!

[cjgrant, 2016-11-29 20:59:33]

Conclusion:  This CL would break the WebVR warnings as-is.  I think I need to
sync with dcheng@ on how to rework this.

[cjgrant, 2016-11-30 20:33:56]

Brandon, I reworked this to use the new security level query for omnibox, but
the original webVR secure origin plumbing for the warnings, so as not to change
behavior.

From what I can see, the security level isn't computed as secure unless the
connection is https, independent of whether we're looking at localhost or not (I
think).

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2536223002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_shell.cc:916: // TODO(cjgrant): Remove this
method, and all related plumbing back to the
On 2016/11/29 17:53:23, bajones wrote:
> On 2016/11/29 16:40:19, mthiesse wrote:
> > hurray!
> 
> +1!

Sorry that this decoupling isn't happening yet.  The logic is different, so
we'll have to maintain as-is for now.

[bajones, 2016-11-30 21:27:43]

On 2016/11/30 20:33:56, cjgrant wrote:
> Brandon, I reworked this to use the new security level query for omnibox, but
> the original webVR secure origin plumbing for the warnings, so as not to
change
> behavior.
> 
> From what I can see, the security level isn't computed as secure unless the
> connection is https, independent of whether we're looking at localhost or not
(I
> think).
>
> Sorry that this decoupling isn't happening yet.  The logic is different, so
> we'll have to maintain as-is for now.

That's OK, still LGTM. I'm a bit confused, though, since the
SecurityStateTabHelper::GetSecurityInfo call seems end up here:

https://cs.chromium.org/chromium/src/content/common/origin_util.cc?sq=package...

And that explicitly has a branch that returns true for localhost. Maybe we can
just call content::IsOriginSecure directly when determining the WebVR warning
state?

[cjgrant, 2016-12-01 14:45:54]

cjgrant@chromium.org changed reviewers:
+ bshe@chromium.org
- clamy@chromium.org, dcheng@chromium.org

[cjgrant, 2016-12-01 14:45:54]

Adding Biao to check the UI JS change.

Removing clamy@ and dcheng@ - that discussion is now offline.

[bshe, 2016-12-01 15:32:46]

lgtm

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:62: CHECK(helper);
Perhaps a DCHECK here so the code is not in production?

[cjgrant, 2016-12-01 21:21:51]

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:62: CHECK(helper);
On 2016/12/01 15:32:46, bshe wrote:
> Perhaps a DCHECK here so the code is not in production?

The last time we discussed CHECK/DCHECK, we settled on using CHECK for anything
security-impacting (see ui_scene.cc, etc).  This is definitely security
impacting, so it seems prudent to bail out if we can't obtain the correct
security status, right?

[bshe, 2016-12-02 14:21:37]

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:62: CHECK(helper);
On 2016/12/01 21:21:51, cjgrant wrote:
> On 2016/12/01 15:32:46, bshe wrote:
> > Perhaps a DCHECK here so the code is not in production?
> 
> The last time we discussed CHECK/DCHECK, we settled on using CHECK for
anything
> security-impacting (see ui_scene.cc, etc).  This is definitely security
> impacting, so it seems prudent to bail out if we can't obtain the correct
> security status, right?

My understanding is that it is better to use DCHECK for nullity. Since if helper
is null, the code would crash anyway without the CHECK.
And the point of using DCHECK is to let other commiters know that you are
expecting helper never be null
 and they shouldn't try to add code which handle null case below.

[cjgrant, 2016-12-02 15:39:50]

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/40001/chrome/browser/android/...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:62: CHECK(helper);
On 2016/12/02 14:21:36, bshe wrote:
> On 2016/12/01 21:21:51, cjgrant wrote:
> > On 2016/12/01 15:32:46, bshe wrote:
> > > Perhaps a DCHECK here so the code is not in production?
> > 
> > The last time we discussed CHECK/DCHECK, we settled on using CHECK for
> anything
> > security-impacting (see ui_scene.cc, etc).  This is definitely security
> > impacting, so it seems prudent to bail out if we can't obtain the correct
> > security status, right?
> 
> My understanding is that it is better to use DCHECK for nullity. Since if
helper
> is null, the code would crash anyway without the CHECK.
> And the point of using DCHECK is to let other commiters know that you are
> expecting helper never be null
>  and they shouldn't try to add code which handle null case below.

Done.

[cjgrant, 2016-12-02 15:40:07]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-12-02 15:40:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bajones@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2536223002/#ps80001
(title: "Revert unintential Javascript change.")

[commit-bot: I haz the power, 2016-12-02 15:40:24]

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, 2016-12-02 15:43:10]

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

[commit-bot: I haz the power, 2016-12-02 15:43:11]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[cjgrant, 2016-12-02 19:31:56]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-12-02 19:31:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bajones@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2536223002/#ps100001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-12-02 19:33:10]

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, 2016-12-02 19:37:24]

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

[commit-bot: I haz the power, 2016-12-02 19:37:25]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[estark, 2016-12-02 23:48:49]

estark@chromium.org changed reviewers:
+ estark@chromium.org

[estark, 2016-12-02 23:48:50]

https://codereview.chromium.org/2536223002/diff/100001/chrome/browser/android...
File chrome/browser/android/vr_shell/vr_web_contents_observer.cc (right):

https://codereview.chromium.org/2536223002/diff/100001/chrome/browser/android...
chrome/browser/android/vr_shell/vr_web_contents_observer.cc:40:
SetSecurityLevel();
Drive-by (not intending to block this CL, just noting something that should be
addressed in future)... the security level can also change after a navigation
has completed, see WebContents::DidChangeVisibleSecurityState. (For example,
mixed content can appear on the page dynamically.)

There isn't currently a WebContentsObserver method that does exactly what you
want to pick up DidChangeVisibleSecurityState events. I think we can make that
work with a little refactoring though: I'd suggest renaming
WebContentsObserver::SecurityStyleChanged to
WebContentsObserver::VisibleSecurityStateChanged and removing its arguments.
Implementations that use the arguments can obtain them directly from
WebContents::delegate()->GetSecurityStyle(). Then you can update the security
level from a WebContentsObserver::VisibleSecurityStateChanged implementation
here.

(This should be better documented somewhere, but in case you're wondering,
SecurityStyleChanged conveys security state info to observers in //content. The
blink::WebSecurityStyle enum is a lossy representation of SecurityLevel, since
the latter contains Chrome-specific security information that //content doesn't
know about.)

[cjgrant, 2016-12-05 15:28:28]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-12-05 15:28:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bajones@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2536223002/#ps120001
(title: "Rebase again.")

[commit-bot: I haz the power, 2016-12-05 15:28:49]

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, 2016-12-05 15:30:59]

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

[commit-bot: I haz the power, 2016-12-05 15:31:00]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[cjgrant, 2016-12-05 20:53:43]

cjgrant@chromium.org changed reviewers:
- estark@chromium.org, klausw@chromium.org

[cjgrant, 2016-12-05 20:53:44]

Dropping a couple people to limit noise while the patch conflicts are hammered
out.

[cjgrant, 2016-12-05 20:57:51]

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

[commit-bot: I haz the power, 2016-12-05 20:58:37]

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, 2016-12-05 21:03:23]

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

[commit-bot: I haz the power, 2016-12-05 21:03:24]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[cjgrant, 2016-12-05 21:39:40]

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

[commit-bot: I haz the power, 2016-12-05 21:40:08]

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

[cjgrant, 2016-12-05 21:40:25]

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

[commit-bot: I haz the power, 2016-12-05 21:40:38]

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, 2016-12-05 22:38:56]

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

[commit-bot: I haz the power, 2016-12-05 22:38:57]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[cjgrant, 2016-12-06 14:45:15]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-12-06 14:45:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
mthiesse@chromium.org, bajones@chromium.org, bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2536223002/#ps160001
(title: "And, onto Michael's rename change.")

[commit-bot: I haz the power, 2016-12-06 14:45:28]

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, 2016-12-06 15:59:14]

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

[commit-bot: I haz the power, 2016-12-06 15:59:15]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[cjgrant, 2016-12-06 18:35:51]

This change is getting interesting.  Unit tests exposed that my approach to
obtaining security level is a layer violation.  I need to go back to the drawing
board on this.

estart@ earlier proposed a means of reworking WebContentsObserver to do what we
need, and I'm going to try that.

[cjgrant, 2016-12-06 18:35:55]

This change is getting interesting.  Unit tests exposed that my approach to
obtaining security level is a layer violation.  I need to go back to the drawing
board on this.

estart@ earlier proposed a means of reworking WebContentsObserver to do what we
need, and I'm going to try that.

[cjgrant, 2016-12-07 15:02:06]

cjgrant@chromium.org changed reviewers:
- bajones@chromium.org

[cjgrant, 2016-12-07 15:02:07]

Dropping bajones@.

Michael and Biao, PTAL at the new unit test dependency change.  I dislike it,
but it's probably fine for now.

[bshe, 2016-12-07 15:06:13]

On 2016/12/07 15:02:07, cjgrant wrote:
> Dropping bajones@.
> 
> Michael and Biao, PTAL at the new unit test dependency change.  I dislike it,
> but it's probably fine for now.

lgtm

[cjgrant, 2016-12-07 15:06:25]

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

[commit-bot: I haz the power, 2016-12-07 15:06:45]

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

[mthiesse, 2016-12-07 15:53:48]

lgtm

https://codereview.chromium.org/2536223002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/vr_shell/vr_shell_ui.js (right):

https://codereview.chromium.org/2536223002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/vr_shell/vr_shell_ui.js:350: let secure = this.level ==
2 || this.level == 3;
nit: enum names for these constants?

[commit-bot: I haz the power, 2016-12-07 16:09:31]

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

[commit-bot: I haz the power, 2016-12-07 16:09:32]

Dry run: This issue passed the CQ dry run.

[cjgrant, 2016-12-07 16:18:13]

The CQ bit was checked by cjgrant@chromium.org

[cjgrant, 2016-12-07 16:18:13]

The patchset sent to the CQ was uploaded after l-g-t-m from bajones@chromium.org
Link to the patchset: https://codereview.chromium.org/2536223002/#ps180001
(title: "Make unit tests compile by including chrome/browser and blink.")

[commit-bot: I haz the power, 2016-12-07 16:18:27]

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, 2016-12-07 16:24:08]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1481127493177240,
"parent_rev": "2ebc48a1568b776b3b7113c09460cb1d549f0862", "commit_rev":
"6e1e663bc49f03f2947984d1d30246e74f42042a"}

[commit-bot: I haz the power, 2016-12-07 16:24:42]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-12-07 16:27:23]

Description was changed from

==========
Omnibox improvements and fixes.

- Show the correct security icon on the omnibar when out of WebVR.  This
  change uses the tab's security level to determine whether the security
  warnings are shown, rather than the secure origin flag that was
  plumbed through the WebVR code path.
- Keep the omnibox visible at all times in 2D browsing.
- Take URLs only from the main frame on a page.

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Omnibox improvements and fixes.

- Show the correct security icon on the omnibar when out of WebVR.  This
  change uses the tab's security level to determine whether the security
  warnings are shown, rather than the secure origin flag that was
  plumbed through the WebVR code path.
- Keep the omnibox visible at all times in 2D browsing.
- Take URLs only from the main frame on a page.

BUG=641508
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/340d1da550dbf8290c7da82add22af59b0fa47e4
Cr-Commit-Position: refs/heads/master@{#436974}
==========

[commit-bot: I haz the power, 2016-12-07 16:27:25]

Patchset 10 (id:??) landed as
https://crrev.com/340d1da550dbf8290c7da82add22af59b0fa47e4
Cr-Commit-Position: refs/heads/master@{#436974}