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

Issue 10837238: Move browser_test_base.h to content/public/test. I originally thought that content_browsertests wou… (Closed)

Created:
8 years, 4 months ago by jam
Modified:
8 years, 4 months ago
Reviewers:
Jói
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Move browser_test_base.h to content/public/test. I originally thought that content_browsertests wouldn't derive from this. But since it ended up being shared, move to content/public/test. BUG=90448 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151498

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -103 lines) Patch
M chrome/DEPS View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/test/base/in_process_browser_test.h View 2 chunks +2 lines, -2 lines 0 comments Download
M content/content_tests.gypi View 2 chunks +1 line, -1 line 0 comments Download
M content/public/test/DEPS View 1 1 chunk +0 lines, -3 lines 0 comments Download
A + content/public/test/browser_test_base.h View 3 chunks +7 lines, -3 lines 0 comments Download
D content/test/browser_test_base.h View 1 chunk +0 lines, -90 lines 0 comments Download
M content/test/browser_test_base.cc View 4 chunks +6 lines, -2 lines 0 comments Download
M content/test/content_browser_test.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
jam
8 years, 4 months ago (2012-08-14 16:24:37 UTC) #1
Jói
http://codereview.chromium.org/10837238/diff/1/content/test/browser_test_base.cc File content/test/browser_test_base.cc (right): http://codereview.chromium.org/10837238/diff/1/content/test/browser_test_base.cc#newcode5 content/test/browser_test_base.cc:5: #include "content/public/test/browser_test_base.h" If I understand correctly, the rule is ...
8 years, 4 months ago (2012-08-14 16:29:07 UTC) #2
jam
8 years, 4 months ago (2012-08-14 16:42:22 UTC) #3
http://codereview.chromium.org/10837238/diff/1/content/test/browser_test_base.cc
File content/test/browser_test_base.cc (right):

http://codereview.chromium.org/10837238/diff/1/content/test/browser_test_base...
content/test/browser_test_base.cc:5: #include
"content/public/test/browser_test_base.h"
On 2012/08/14 16:29:07, Jói wrote:
> If I understand correctly, the rule is to move the .cc file with the header. 
> Shouldn't this file move then?

content/test didn't obey this. the history is that originally chrome code could
include files from content/test. but then some internal content headers were
leaking through public test headers, so I moved the public ones to
content/public/test.

with your deps improvements, I can now move the cc files to be alongside their
headers,  and enforce that only cc files are including internal headers :) i'll
do that in a followup.

Powered by Google App Engine
This is Rietveld 408576698