[MemoryPressure] Move chrome/browser/chromeos/* to chrome/browser/memory/*.

[MemoryPressure] Move chrome/browser/chromeos/* to chrome/browser/memory/*.

Also use namespace memory for those files.

Note: This is the first step of expanding Chromeos tab killing to other platforms.

BUG=463597
Committed: https://crrev.com/97de162a0bd8dc7dc012c31e9c7b1a981ef23acb
Cr-Commit-Position: refs/heads/master@{#335260}

[Georges Khalil, 2015-06-16 14:59:17]

Patchset #1 (id:1) has been deleted

[Georges Khalil, 2015-06-16 15:21:47]

Patchset #1 (id:20001) has been deleted

[Georges Khalil, 2015-06-16 17:09:10]

Patchset #1 (id:40001) has been deleted

[Georges Khalil, 2015-06-16 17:20:32]

Patchset #1 (id:60001) has been deleted

[Georges Khalil, 2015-06-16 17:37:05]

Patchset #1 (id:80001) has been deleted

[Georges Khalil, 2015-06-16 17:54:05]

Patchset #1 (id:100001) has been deleted

[Georges Khalil, 2015-06-16 18:16:16]

Patchset #1 (id:120001) has been deleted

[Georges Khalil, 2015-06-16 18:16:26]

Patchset #1 (id:120001) has been deleted

[Georges Khalil, 2015-06-16 18:40:02]

Patchset #1 (id:140001) has been deleted

[Georges Khalil, 2015-06-16 19:00:08]

Patchset #1 (id:160001) has been deleted

[Georges Khalil, 2015-06-16 21:49:49]

Patchset #1 (id:180001) has been deleted

[Georges Khalil, 2015-06-17 14:47:24]

Patchset #1 (id:200001) has been deleted

[Georges Khalil, 2015-06-17 14:48:51]

Patchset #1 (id:210001) has been deleted

[Georges Khalil, 2015-06-17 14:57:42]

Patchset #1 (id:230001) has been deleted

[Georges Khalil, 2015-06-17 15:42:57]

Patchset #1 (id:250001) has been deleted

[Georges Khalil, 2015-06-17 15:46:38]

georgesak@chromium.org changed reviewers:
+ jamescook@chromium.org

[Georges Khalil, 2015-06-17 15:46:39]

PTAL.

This is a first step towards having tab killing on other platforms.

[James Cook, 2015-06-17 21:20:23]

LGTM with nit, assuming you're OK renaming the browertest file.

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/browser...
File chrome/browser/browser_process_platform_part_chromeos.h (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/browser...
chrome/browser/browser_process_platform_part_chromeos.h:77: virtual
memory::OomPriorityManager* oom_priority_manager();
Not for this CL, but it would be good to rename this function to
GetOomPriorityManager().

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos.cc (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos.cc:48: #ifdef OS_CHROMEOS
#if defined(OS_CHROMEOS)

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_browser....
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_browser....
chrome/chrome_browser.gypi:589: 'browser/memory_details.cc',
Not for this CL, but I wonder if these files should move into c/b/memory as
well.

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:343:
'browser/memory/oom_priority_manager_browsertest_chromeos.cc',
I would prefer this file be named oom_priority_manager_chromeos_browsertest.cc,
as this is similar to the unittest naming (and it seems like we support both
naming conventions).

[Georges Khalil, 2015-06-18 15:25:48]

On 2015/06/17 21:20:23, James Cook (OOO until June 17) wrote:
> LGTM with nit, assuming you're OK renaming the browertest file.

That was the original name that I had given this file. However, it was being
compiled on other platforms too. Looking at filename_rules.gypi, Chromeos
specific files end with either chromeos or chromeos_unittest (only Windows has a
rule for win_browsertest).

So we either keep the filename as is, or we add a new rule in
filename_rules.gypi (not sure if that's a viable option).

WDYT?

[gab, 2015-06-18 15:44:35]

drive-by note on naming scheme below

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_tests.gy...
chrome/chrome_tests.gypi:343:
'browser/memory/oom_priority_manager_browsertest_chromeos.cc',
On 2015/06/17 21:20:23, James Cook (OOO until June 17) wrote:
> I would prefer this file be named
oom_priority_manager_chromeos_browsertest.cc,
> as this is similar to the unittest naming (and it seems like we support both
> naming conventions).

Just to clarify (I suggested this naming scheme to Georges).

I think there is a semantical difference between the two forms.

foo_platform_test.cc means "this is the direct test of foo_platform.cc" where as
foo_test_platform.cc means "this the platform specific implementation of foo's
test".

Therefore for unittests it often makes sense to have foo_platform_unittest.cc
when it directly tests a platform specific implementation and
foo_unittest_platform.cc when it's testing a generic API in a platform specific
way.

Since browser tests are mostly always integration tests,
foo_browsertest_platform.cc almost always makes more sense IMO.

[James Cook, 2015-06-18 15:48:16]

You have convinced me the existing filename is OK.  LGTM with the ifdef nit.

[Georges Khalil, 2015-06-18 16:43:00]

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

[Georges Khalil, 2015-06-18 16:43:02]

sky@chromium.org: Please review changes in 

- New files in chrome/browser/memory (moved from chrome/browser/chromeos/memory)
- All other files in chrome/browser and chrome/test

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/browser...
File chrome/browser/browser_process_platform_part_chromeos.h (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/browser...
chrome/browser/browser_process_platform_part_chromeos.h:77: virtual
memory::OomPriorityManager* oom_priority_manager();
On 2015/06/17 21:20:23, James Cook (OOO until June 17) wrote:
> Not for this CL, but it would be good to rename this function to
> GetOomPriorityManager().

Acknowledged.

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos.cc (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos.cc:48: #ifdef OS_CHROMEOS
On 2015/06/17 21:20:23, James Cook (OOO until June 17) wrote:
> #if defined(OS_CHROMEOS)

Done.

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_browser....
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/1188823002/diff/270001/chrome/chrome_browser....
chrome/chrome_browser.gypi:589: 'browser/memory_details.cc',
On 2015/06/17 21:20:23, James Cook (OOO until June 17) wrote:
> Not for this CL, but I wonder if these files should move into c/b/memory as
> well.

Agreed. I'll look into it.

[sky, 2015-06-18 19:56:56]

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager.h (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager.h:78: bool is_app;            //
browser window is an app
nit: I wouldn't bother with trying to align like this. It's too painful to keep
up.

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos.cc (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos.cc:48: #if
defined(OS_CHROMEOS)
If this file is chromeos only, why the ifdefs?

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos_unittest.cc (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos_unittest.cc:11: #include
"chrome/browser/memory/oom_priority_manager.h"
this should be the first include. Also, how come this isn't including the
_chromeos file?

[Georges Khalil, 2015-06-18 20:27:47]

sky@, PTAnL.

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager.h (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager.h:78: bool is_app;            //
browser window is an app
On 2015/06/18 19:56:56, sky wrote:
> nit: I wouldn't bother with trying to align like this. It's too painful to
keep
> up.

I think that was git cl format doing its magic.

Reverted it.

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos.cc (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos.cc:48: #if
defined(OS_CHROMEOS)
On 2015/06/18 19:56:56, sky wrote:
> If this file is chromeos only, why the ifdefs?

You are right, this is not needed anymore. Removed.

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
File chrome/browser/memory/oom_priority_manager_chromeos_unittest.cc (right):

https://codereview.chromium.org/1188823002/diff/290001/chrome/browser/memory/...
chrome/browser/memory/oom_priority_manager_chromeos_unittest.cc:11: #include
"chrome/browser/memory/oom_priority_manager.h"
On 2015/06/18 19:56:56, sky wrote:
> this should be the first include. Also, how come this isn't including the
> _chromeos file?

Lint complains if I move it at the beginning. I guess the fact that we have
"chromeos" in the name of the test confuses it. So I left it as is.

Another option would be to move it and add // NOLINT. Do you prefer that?

Also, there is no oom_priority_manager_chromeos.h, only a
oom_priority_manager_chromeos.cc. The header is generic, implementation is
platform specific (for now, that might change).

[sky, 2015-06-18 23:26:02]

LGTM

[Georges Khalil, 2015-06-19 00:13:27]

The CQ bit was checked by georgesak@chromium.org

[Georges Khalil, 2015-06-19 00:13:28]

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

[commit-bot: I haz the power, 2015-06-19 00:14:44]

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

[commit-bot: I haz the power, 2015-06-19 00:29:54]

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

[commit-bot: I haz the power, 2015-06-19 00:29:55]

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

[Georges Khalil, 2015-06-19 13:43:01]

The CQ bit was checked by georgesak@chromium.org

[commit-bot: I haz the power, 2015-06-19 13:43:43]

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

[commit-bot: I haz the power, 2015-06-19 15:19:02]

Committed patchset #4 (id:330001)

[commit-bot: I haz the power, 2015-06-19 15:19:57]

Patchset 4 (id:??) landed as
https://crrev.com/97de162a0bd8dc7dc012c31e9c7b1a981ef23acb
Cr-Commit-Position: refs/heads/master@{#335260}