Remove unreleased web app frame code from ChromeOS

Remove unreleased web app frame code from ChromeOS

This UI was experimental and is not going to be released.

BUG=562280
Committed: https://crrev.com/8e885367b432753eb4cc76e39faa3f32cbb529e4
Cr-Commit-Position: refs/heads/master@{#439737}

[benwells, 2016-12-06 05:01:27]

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

[commit-bot: I haz the power, 2016-12-06 05:01:58]

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-06 06:33:15]

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

[commit-bot: I haz the power, 2016-12-06 06:33:16]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[benwells, 2016-12-06 07:10:43]

benwells@chromium.org changed reviewers:
+ tapted@chromium.org

[benwells, 2016-12-06 07:10:44]

tapted for initial local review :)

[tapted, 2016-12-06 23:52:28]

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h (left):

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:38: void
UpdateToolbar() override;
Can this be deleted from BrowserNonClientFrameView? (and callers of it updated)
BrowserNonClientFrameViewMus also has an override, but it also does nothing -
same as the default implementation.

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:39:
views::View* GetLocationIconView() const override;
Same here - I think it's just ash/mus that override this.
BrowserFrame::GetLocationIconView() can probably be purged as well (and
BrowserFrame::UpdateToolbar)

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc (right):

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc:410: return
browser->is_app();
should this be a verbatim copy of
`BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle()`? It's probably Ok
that it's copy-paste, but having them not be identical suggests that they're
different in some way (but I don't think they are).

[benwells, 2016-12-15 05:28:21]

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

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

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-15 05:44:06]

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

[commit-bot: I haz the power, 2016-12-15 05:44:07]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[benwells, 2016-12-15 05:48:58]

Patchset #3 (id:40001) has been deleted

[benwells, 2016-12-15 06:26:54]

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

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

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

[benwells, 2016-12-15 23:08:41]

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h (left):

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:38: void
UpdateToolbar() override;
On 2016/12/06 23:52:28, tapted wrote:
> Can this be deleted from BrowserNonClientFrameView? (and callers of it
updated)
> BrowserNonClientFrameViewMus also has an override, but it also does nothing -
> same as the default implementation.

Done.

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:39:
views::View* GetLocationIconView() const override;
On 2016/12/06 23:52:28, tapted wrote:
> Same here - I think it's just ash/mus that override this.
> BrowserFrame::GetLocationIconView() can probably be purged as well (and
> BrowserFrame::UpdateToolbar)

Done.

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc (right):

https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc:410: return
browser->is_app();
On 2016/12/06 23:52:28, tapted wrote:
> should this be a verbatim copy of
> `BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle()`? It's probably Ok
> that it's copy-paste, but having them not be identical suggests that they're
> different in some way (but I don't think they are).

Um, yeah. Made identical.

[benwells, 2016-12-15 23:09:56]

On 2016/12/15 23:08:41, benwells wrote:
>
https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
> File chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h (left):
> 
>
https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
> chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:38: void
> UpdateToolbar() override;
> On 2016/12/06 23:52:28, tapted wrote:
> > Can this be deleted from BrowserNonClientFrameView? (and callers of it
> updated)
> > BrowserNonClientFrameViewMus also has an override, but it also does nothing
-
> > same as the default implementation.
> 
> Done.
> 
>
https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
> chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h:39:
> views::View* GetLocationIconView() const override;
> On 2016/12/06 23:52:28, tapted wrote:
> > Same here - I think it's just ash/mus that override this.
> > BrowserFrame::GetLocationIconView() can probably be purged as well (and
> > BrowserFrame::UpdateToolbar)
> 
> Done.
> 
>
https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
> File chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc
(right):
> 
>
https://codereview.chromium.org/2552903003/diff/1/chrome/browser/ui/views/fra...
> chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc:410: return
> browser->is_app();
> On 2016/12/06 23:52:28, tapted wrote:
> > should this be a verbatim copy of
> > `BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle()`? It's probably
Ok
> > that it's copy-paste, but having them not be identical suggests that they're
> > different in some way (but I don't think they are).
> 
> Um, yeah. Made identical.

Oops hold up, just saw something to fix.

[benwells, 2016-12-15 23:34:39]

ok, should be good for review now

[tapted, 2016-12-16 03:30:36]

lgtm

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
File chrome/browser/ui/extensions/hosted_app_browser_controller.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browser_controller.cc:7: #include
"base/command_line.h"
nit: this can probably be removed too

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
File chrome/browser/ui/extensions/hosted_app_browsertest.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browsertest.cc:9: #include
"chrome/browser/devtools/devtools_window_testing.h"
nit: this one is probably unused.

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browsertest.cc:13: #include
"chrome/browser/ui/browser_finder.h"
I think this was just used for GetBrowserCount. nothing else screamed at me.

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view.cc:1288: else
this logic isn't quite the same, but I guess location_bar() is guaranteed to
return non-null, so that should be fine.

[benwells, 2016-12-19 03:59:05]

benwells@chromium.org changed reviewers:
+ pkasting@chromium.org

[benwells, 2016-12-19 03:59:06]

+pkasting for chrome/browser/ui/*.* and chrome/browser/ui/views/frame/

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
File chrome/browser/ui/extensions/hosted_app_browser_controller.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browser_controller.cc:7: #include
"base/command_line.h"
On 2016/12/16 03:30:36, tapted wrote:
> nit: this can probably be removed too

Done.

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
File chrome/browser/ui/extensions/hosted_app_browsertest.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browsertest.cc:9: #include
"chrome/browser/devtools/devtools_window_testing.h"
On 2016/12/16 03:30:36, tapted wrote:
> nit: this one is probably unused.

Done.

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/exte...
chrome/browser/ui/extensions/hosted_app_browsertest.cc:13: #include
"chrome/browser/ui/browser_finder.h"
On 2016/12/16 03:30:36, tapted wrote:
> I think this was just used for GetBrowserCount. nothing else screamed at me.

Done.

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/2552903003/diff/100001/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_view.cc:1288: else
On 2016/12/16 03:30:36, tapted wrote:
> this logic isn't quite the same, but I guess location_bar() is guaranteed to
> return non-null, so that should be fine.

Yep, that was my thinking. I think this is clearer than what the alternative
would be.

[Peter Kasting, 2016-12-20 02:44:04]

LGTM

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
chrome/browser/ui/browser.cc:2408: if (hosted_app_controller_)
Nit: Shorter (but maybe you don't like this):

  return !!hosted_app_controller_;

[benwells, 2016-12-20 03:44:20]

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
chrome/browser/ui/browser.cc:2408: if (hosted_app_controller_)
On 2016/12/20 02:44:03, Peter Kasting wrote:
> Nit: Shorter (but maybe you don't like this):
> 
>   return !!hosted_app_controller_;

Hmmm ... yeah personally I prefer the explicit return as it is now. I'm not an
owner of this code though ... if you as an owner want me to change it, let me
know, otherwise i'll land it as is.

[Peter Kasting, 2016-12-20 04:47:31]

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/2552903003/diff/140001/chrome/browser/ui/brow...
chrome/browser/ui/browser.cc:2408: if (hosted_app_controller_)
On 2016/12/20 03:44:20, benwells wrote:
> On 2016/12/20 02:44:03, Peter Kasting wrote:
> > Nit: Shorter (but maybe you don't like this):
> > 
> >   return !!hosted_app_controller_;
> 
> Hmmm ... yeah personally I prefer the explicit return as it is now. I'm not an
> owner of this code though ... if you as an owner want me to change it, let me
> know, otherwise i'll land it as is.

I'm fine with it as it is.

[benwells, 2016-12-20 04:52:09]

The CQ bit was checked by benwells@chromium.org

[benwells, 2016-12-20 04:52:10]

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

[commit-bot: I haz the power, 2016-12-20 04:52:33]

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-20 07:48:56]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1482209529505300,
"parent_rev": "2e94177b01e569cd469e9736ba3de9ac10b183cc", "commit_rev":
"c9f44f8b0e1122610c4f42298f1af6903bd1536f"}

[commit-bot: I haz the power, 2016-12-20 07:49:25]

Description was changed from

==========
Remove unreleased web app frame code from ChromeOS

This UI was experimental and is not going to be released.

BUG=562280
==========

to

==========
Remove unreleased web app frame code from ChromeOS

This UI was experimental and is not going to be released.

BUG=562280

Review-Url: https://codereview.chromium.org/2552903003
==========

[commit-bot: I haz the power, 2016-12-20 07:49:26]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-12-20 07:52:12]

Description was changed from

==========
Remove unreleased web app frame code from ChromeOS

This UI was experimental and is not going to be released.

BUG=562280

Review-Url: https://codereview.chromium.org/2552903003
==========

to

==========
Remove unreleased web app frame code from ChromeOS

This UI was experimental and is not going to be released.

BUG=562280

Committed: https://crrev.com/8e885367b432753eb4cc76e39faa3f32cbb529e4
Cr-Commit-Position: refs/heads/master@{#439737}
==========

[commit-bot: I haz the power, 2016-12-20 07:52:14]

Patchset 7 (id:??) landed as
https://crrev.com/8e885367b432753eb4cc76e39faa3f32cbb529e4
Cr-Commit-Position: refs/heads/master@{#439737}