Remove net dependency from ui/gfx

Remove net dependency from ui/gfx

ui/gfx/ depended on net/ for two things, SortedDisplayURL and gfx::ElideUrl().
SortedDisplayURL was not used by anything other than its own unit test and is
deleted in this patch.  ElideUrl() is only used from
chrome/browser/ui/(gtk|cocoa|views), so this patch moves the functionality up
to chrome/browser/ui/elide_url.(h|cc) and the unit test to
chrome/browser/ui/tests/elide_url_unittest.cc.

R=sky

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247820

[sky, 2014-01-21 21:41:48]

Nice, LGTM

[tfarina, 2014-01-21 23:08:14]

Ben and I like this, for sure! ;_)

Thanks for doing this James!

[tfarina, 2014-01-21 23:09:47]

https://codereview.chromium.org/143463006/diff/30001/chrome/browser/ui/tests/...
File chrome/browser/ui/tests/elide_url_unittest.cc (right):

https://codereview.chromium.org/143463006/diff/30001/chrome/browser/ui/tests/...
chrome/browser/ui/tests/elide_url_unittest.cc:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
is it a new trend to put things separate from their implementation? If not, can
this live near elide_url.*, i.e, in chrome/browser/ui/elide_url_unittest.cc?

[jamesr, 2014-01-21 23:10:45]

On 2014/01/21 23:09:47, tfarina wrote:
>
https://codereview.chromium.org/143463006/diff/30001/chrome/browser/ui/tests/...
> File chrome/browser/ui/tests/elide_url_unittest.cc (right):
> 
>
https://codereview.chromium.org/143463006/diff/30001/chrome/browser/ui/tests/...
> chrome/browser/ui/tests/elide_url_unittest.cc:1: // Copyright 2014 The
Chromium
> Authors. All rights reserved.
> is it a new trend to put things separate from their implementation? If not,
can
> this live near elide_url.*, i.e, in chrome/browser/ui/elide_url_unittest.cc?

There aren't any *_unittest.cc files in c/b/ui/, so I followed the existing
example (2 examples) of having the tests in c/b/ui/tests/.  I don't think this
is common in general.

[commit-bot: I haz the power, 2014-01-22 21:13:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/143463006/30001

[commit-bot: I haz the power, 2014-01-23 03:53:19]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2014-01-24 00:13:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/143463006/30001

[commit-bot: I haz the power, 2014-01-24 07:15:35]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[jamesr, 2014-01-24 18:38:10]

On 2014/01/24 07:15:35, I haz the power (commit-bot) wrote:
> The commit queue went berserk retrying too often for a
> seemingly flaky test on builder android_dbg:
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

The output is confusing, but looks like TextEliderTest is actually failing on
android dbg in the unit_tests target:

[----------] 1 test from TextEliderTest
[ RUN      ] TextEliderTest.TestGeneralEliding
[ERROR:platform_font_android.cc(13)] Not implemented reached in static
gfx::PlatformFont* gfx::PlatformFont::CreateDefault()
[ERROR:platform_font_android.cc(13)] Not implemented reached in static
gfx::PlatformFont* gfx::PlatformFont::CreateDefault()
[ERROR:text_utils_android.cc(17)] Not implemented reached in float
gfx::GetStringWidthF(const string16&, const gfx::FontList&)
[ERROR:text_utils_android.cc(17)] Not implemented reached in float
gfx::GetStringWidthF(const string16&, const gfx::FontList&)
[ERROR:text_utils_android.cc(17)] Not implemented reached in float
gfx::GetStringWidthF(const string16&, const gfx::FontList&)
../../chrome/browser/ui/tests/elide_url_unittest.cc:33: Failure
Value of: ElideUrl(url, font_list, available_width, std::string())
  Actual: www.google.com/intl/en/ads/
Expected: UTF8ToUTF16(testcases[i].output)
Which is: google.com/intl/…/ads/

investigating...

[tfarina, 2014-01-25 23:11:28]

James, in the mean time, would you put a separate CL to remove
SortedDisplayURL()?

[jamesr, 2014-01-28 19:11:26]

The issue is these tests are disabled in a very surprising way on android.  I'll
convert the disabling mechanism over to use macros and then reland.  Filed
https://code.google.com/p/chromium/issues/detail?id=338784 about the tests being
disabled.

[sky, 2014-01-28 21:57:40]

Maybe they should be in their own file and the file not compiled on android?

On Tue, Jan 28, 2014 at 11:11 AM,  <jamesr@chromium.org> wrote:
> The issue is these tests are disabled in a very surprising way on android.
> I'll
> convert the disabling mechanism over to use macros and then reland.  Filed
> https://code.google.com/p/chromium/issues/detail?id=338784 about the tests
> being
> disabled.
>
> https://codereview.chromium.org/143463006/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jamesr, 2014-01-28 23:27:43]

This excludes the chrome/browser/ui/elide_url.cc and test from the android build
via gyp instead of having inline annotations on the individual tests.

[sky, 2014-01-29 04:02:14]

Nice, LGTM

https://codereview.chromium.org/143463006/diff/610001/chrome/browser/ui/elide...
File chrome/browser/ui/elide_url.h (right):

https://codereview.chromium.org/143463006/diff/610001/chrome/browser/ui/elide...
chrome/browser/ui/elide_url.h:33: base::string16 ElideUrl(const GURL& url,
nit: forward declare GURL.

https://codereview.chromium.org/143463006/diff/610001/chrome/browser/ui/elide...
chrome/browser/ui/elide_url.h:37: 
nit: only one newline here.

[commit-bot: I haz the power, 2014-01-29 19:05:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/143463006/630001

[commit-bot: I haz the power, 2014-01-29 19:41:19]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2014-01-29 20:48:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/143463006/650001

[commit-bot: I haz the power, 2014-01-29 22:04:28]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2014-01-29 22:43:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/143463006/650001

[commit-bot: I haz the power, 2014-01-30 03:37:37]

Change committed as 247820