Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 7044013: Drop url_constants dependency in content/. (Closed)

Created:
9 years, 7 months ago by Avi (use Gerrit)
Modified:
9 years, 7 months ago
Reviewers:
jam
CC:
chromium-reviews, Avi (use Gerrit), jam, brettw-cc_chromium.org
Visibility:
Public.

Description

Drop url_constants dependency in content/. BUG=76697 TEST=compiles, about:urls still work Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=85907

Patch Set 1 #

Patch Set 2 : moar fix #

Patch Set 3 : forgot null check #

Total comments: 4

Patch Set 4 : better! #

Patch Set 5 : moar better! #

Patch Set 6 : reinsert #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -27 lines) Patch
M chrome/browser/chrome_content_browser_client.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M content/browser/DEPS View 1 2 3 4 5 2 chunks +1 line, -1 line 0 comments Download
M content/browser/browsing_instance.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/child_process_security_policy.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/content_browser_client.h View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/content_browser_client.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/buffered_resource_handler.cc View 1 chunk +0 lines, -1 line 0 comments Download
content/browser/renderer_host/render_message_filter.cc View 2 chunks +1 line, -1 line 0 comments Download
M content/browser/renderer_host/resource_dispatcher_host.cc View 2 chunks +1 line, -1 line 0 comments Download
M content/browser/renderer_host/x509_user_cert_resource_handler.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/site_instance.cc View 1 2 3 4 1 chunk +12 lines, -9 lines 0 comments Download
M content/browser/site_instance_unittest.cc View 1 2 3 4 6 chunks +11 lines, -6 lines 0 comments Download
M content/browser/tab_contents/navigation_controller.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/tab_contents/navigation_entry.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/tab_contents/render_view_host_manager.cc View 2 chunks +1 line, -1 line 0 comments Download
M content/browser/tab_contents/tab_contents.cc View 1 2 chunks +1 line, -1 line 0 comments Download
M content/browser/tab_contents/tab_contents_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
Avi (use Gerrit)
9 years, 7 months ago (2011-05-18 21:48:55 UTC) #1
jam
http://codereview.chromium.org/7044013/diff/5001/content/browser/DEPS File content/browser/DEPS (right): http://codereview.chromium.org/7044013/diff/5001/content/browser/DEPS#newcode123 content/browser/DEPS:123: "+chrome/common/url_constants.h", are there many tests that use this? can ...
9 years, 7 months ago (2011-05-18 22:12:28 UTC) #2
Avi (use Gerrit)
http://codereview.chromium.org/7044013/diff/5001/content/browser/DEPS File content/browser/DEPS (right): http://codereview.chromium.org/7044013/diff/5001/content/browser/DEPS#newcode123 content/browser/DEPS:123: "+chrome/common/url_constants.h", Already fixed. http://codereview.chromium.org/7044013/diff/5001/content/browser/site_instance.cc File content/browser/site_instance.cc (right): http://codereview.chromium.org/7044013/diff/5001/content/browser/site_instance.cc#newcode28 content/browser/site_instance.cc:28: ...
9 years, 7 months ago (2011-05-18 22:13:59 UTC) #3
Avi (use Gerrit)
Much better. Running through try bots again.
9 years, 7 months ago (2011-05-18 22:34:56 UTC) #4
jam
lgtm
9 years, 7 months ago (2011-05-18 22:40:57 UTC) #5
Avi (use Gerrit)
Turns out that no, you can't pull that line from DEPS; there are several tests ...
9 years, 7 months ago (2011-05-19 00:36:34 UTC) #6
jam
9 years, 7 months ago (2011-05-19 00:43:50 UTC) #7 
that's fine for now, we can just move the tests out later to chrome

On Wed, May 18, 2011 at 5:36 PM, <avi@chromium.org> wrote:

> Turns out that no, you can't pull that line from DEPS; there are several
> tests I
> missed that require it and aren't easily re-written :(
>
> Trybots pass now.
>
>
> http://codereview.chromium.org/7044013/
>

Powered by Google App Engine
This is Rietveld 408576698