Omnibox app

Omnibox app

R=sky@chromium.org
BUG=

Committed: https://crrev.com/41fe041068754896e6a95125fde15657e4987d0c
Cr-Commit-Position: refs/heads/master@{#329546}

[Ben Goodger (Google), 2015-05-12 21:35:20]

ben@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2015-05-12 21:49:54]

https://codereview.chromium.org/1133893003/diff/100001/mandoline/services/cor...
File mandoline/services/core_services/BUILD.gn (right):

https://codereview.chromium.org/1133893003/diff/100001/mandoline/services/cor...
mandoline/services/core_services/BUILD.gn:38: "//mandoline/ui/omnibox:lib",
nit: sort.

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/browser/b...
File mandoline/ui/browser/browser.h (right):

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/browser/b...
mandoline/ui/browser/browser.h:43: const mojo::String& current_url() const {
return current_url_; }
Can we keep usage of mojo types only to interfaces? In other words can
current_url_ be a GURL or std::string if you need it to be invalid for a while?

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/browser/b...
mandoline/ui/browser/browser.h:78: mojo::ApplicationImpl* app_impl_;
You don't seem to set or use this.

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
File mandoline/ui/omnibox/omnibox_impl.cc (right):

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
mandoline/ui/omnibox/omnibox_impl.cc:108: client_->OpenURL(
mojo::String::From<base::string16>(sender->text()));
nit: '( m' -> '(m'

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
mandoline/ui/omnibox/omnibox_impl.cc:130: url_ = url;
Don't you want to push url_ to the edit_?

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
File mandoline/ui/omnibox/omnibox_impl.h (right):

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
mandoline/ui/omnibox/omnibox_impl.h:5: #ifndef MANDOLINE_UI_OMNIBOX_OMNIBOX_H_
MANDOLINE_UI_OMNIBOX_OMNIBOX_IMPL_H

[Ben Goodger (Google), 2015-05-12 23:03:56]

all done, except:

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
File mandoline/ui/omnibox/omnibox_impl.cc (right):

https://codereview.chromium.org/1133893003/diff/100001/mandoline/ui/omnibox/o...
mandoline/ui/omnibox/omnibox_impl.cc:130: url_ = url;
On 2015/05/12 21:49:54, sky wrote:
> Don't you want to push url_ to the edit_?

I do this in OnEmbed() because the textfield doesn't exist yet. None of the
views stuff can be initialized until AuraInit is instantiated. That can't happen
in the ctor because the ctor is run before the command line is initialized. I
really need to restructure this class into a couple.

[sky, 2015-05-12 23:14:17]

LGTM

[Ben Goodger (Google), 2015-05-12 23:24:29]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-05-12 23:24:29]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1133893003/#ps140001
(title: ".")

[commit-bot: I haz the power, 2015-05-12 23:24:52]

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

[commit-bot: I haz the power, 2015-05-12 23:39:38]

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

[commit-bot: I haz the power, 2015-05-12 23:39:39]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Ben Goodger (Google), 2015-05-12 23:54:41]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-05-12 23:54:42]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1133893003/#ps150020
(title: ".")

[commit-bot: I haz the power, 2015-05-12 23:55:46]

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

[commit-bot: I haz the power, 2015-05-13 00:07:28]

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

[commit-bot: I haz the power, 2015-05-13 00:07:29]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Ben Goodger (Google), 2015-05-13 00:11:16]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2015-05-13 00:11:17]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1133893003/#ps170001
(title: ".")

[commit-bot: I haz the power, 2015-05-13 00:11:46]

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

[commit-bot: I haz the power, 2015-05-13 00:48:05]

Committed patchset #10 (id:170001)

[commit-bot: I haz the power, 2015-05-13 00:49:07]

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