Moved ipc-specific files from autofill/core to autofill/content.

Moved ipc-specific files from autofill/core to autofill/content.

BUG=302468
TBR=jschuh@chromium.org, sky@chromium.org, aedla@chromium.org

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

[jeanfrancois, 2013-12-11 13:04:46]

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:38: ],
I haven't included dependencies to 'autofill_jni_headers', but I'm not sure it's
the correct thing to do.

[jif-google, 2013-12-11 13:23:55]

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:60: '../url/url.gyp:url_lib',
forgot to add the dependency to autofill_content_common

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
File components/autofill/content/browser/autofill_driver_impl_unittest.cc
(right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/browser/autofill_driver_impl_unittest.cc:13:
#include "components/autofill/content/browser/autofill_manager.h"
what the... will revert this change.

[blundell, 2013-12-11 13:32:52]

https://codereview.chromium.org/108283005/diff/1/chrome/common/importer/profi...
File chrome/common/importer/profile_import_process_messages.h (right):

https://codereview.chromium.org/108283005/diff/1/chrome/common/importer/profi...
chrome/common/importer/profile_import_process_messages.h:17: #include
"components/autofill/content/common/autofill_param_traits_macros.h"
chrome_common.gypi needs a dependency on autofill_content_common in the !iOS
dependencies section.

https://codereview.chromium.org/108283005/diff/1/chrome/renderer/DEPS
File chrome/renderer/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/chrome/renderer/DEPS#newcode3
chrome/renderer/DEPS:3: "+components/autofill/content/common",
nit: order with above

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:30: 'target_name': 'autofill_content_common',
nit: This target would be better-placed directly above autofill_content_browser.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:33: '../base/base.gyp:base',
This needs a dependency on autofill_core_common.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:56: '../content/content.gyp:content_common',
This dependency can go.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:57:
'../third_party/WebKit/public/blink.gyp:blink_minimal',
This dependency can go.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
File components/autofill/content/browser/autofill_driver_impl_unittest.cc
(right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/browser/autofill_driver_impl_unittest.cc:13:
#include "components/autofill/content/browser/autofill_manager.h"
This is a mistake.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
File components/autofill/content/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/common/DEPS:2:
"!content/public/common/common_param_traits.h",
These can now be +'s instead of !'s and the TODO(blundell) can go away: this is
//content-based code.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/common/DEPS:8:
"!content/public/common/common_param_traits_macros.h",
Could you please add a "-ipc" to //components/autofill/DEPS and add it in
//components/autofill/content/DEPS?

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
components/autofill/core/browser/DEPS:2: "+components/autofill/content/common",
I think that rather than doing this you should be able to just eliminate the
includes of autofill_messages.h in this directory: Nothing should actually be
using IPC anymore (since you've removed all of it).

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_manager_unittest.cc (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_manager_unittest.cc:48: #include
"ipc/ipc_test_sink.h"
You should be able to kill this include I believe.

[jeanfrancois, 2013-12-11 15:01:18]

https://codereview.chromium.org/108283005/diff/1/chrome/common/importer/profi...
File chrome/common/importer/profile_import_process_messages.h (right):

https://codereview.chromium.org/108283005/diff/1/chrome/common/importer/profi...
chrome/common/importer/profile_import_process_messages.h:17: #include
"components/autofill/content/common/autofill_param_traits_macros.h"
On 2013/12/11 13:32:53, blundell wrote:
> chrome_common.gypi needs a dependency on autofill_content_common in the !iOS
> dependencies section.

Done.

https://codereview.chromium.org/108283005/diff/1/chrome/renderer/DEPS
File chrome/renderer/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/chrome/renderer/DEPS#newcode3
chrome/renderer/DEPS:3: "+components/autofill/content/common",
On 2013/12/11 13:32:53, blundell wrote:
> nit: order with above

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:30: 'target_name': 'autofill_content_common',
On 2013/12/11 13:32:53, blundell wrote:
> nit: This target would be better-placed directly above
autofill_content_browser.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:33: '../base/base.gyp:base',
On 2013/12/11 13:32:53, blundell wrote:
> This needs a dependency on autofill_core_common.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:56: '../content/content.gyp:content_common',
On 2013/12/11 13:32:53, blundell wrote:
> This dependency can go.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill.gypi#new...
components/autofill.gypi:57:
'../third_party/WebKit/public/blink.gyp:blink_minimal',
On 2013/12/11 13:32:53, blundell wrote:
> This dependency can go.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
File components/autofill/content/browser/autofill_driver_impl_unittest.cc
(right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/browser/autofill_driver_impl_unittest.cc:13:
#include "components/autofill/content/browser/autofill_manager.h"
On 2013/12/11 13:32:53, blundell wrote:
> This is a mistake.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
File components/autofill/content/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/common/DEPS:2:
"!content/public/common/common_param_traits.h",
On 2013/12/11 13:32:53, blundell wrote:
> These can now be +'s instead of !'s and the TODO(blundell) can go away: this
is
> //content-based code.

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill/content/...
components/autofill/content/common/DEPS:8:
"!content/public/common/common_param_traits_macros.h",
On 2013/12/11 13:32:53, blundell wrote:
> Could you please add a "-ipc" to //components/autofill/DEPS and add it in
> //components/autofill/content/DEPS?

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/DEPS (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
components/autofill/core/browser/DEPS:2: "+components/autofill/content/common",
On 2013/12/11 13:32:53, blundell wrote:
> I think that rather than doing this you should be able to just eliminate the
> includes of autofill_messages.h in this directory: Nothing should actually be
> using IPC anymore (since you've removed all of it).

Done.

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_manager_unittest.cc (right):

https://codereview.chromium.org/108283005/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_manager_unittest.cc:48: #include
"ipc/ipc_test_sink.h"
On 2013/12/11 13:32:53, blundell wrote:
> You should be able to kill this include I believe.

Done.

[blundell, 2013-12-11 15:42:05]

LGTM with fixes

+isherman@ (You can TBR the other required OWNERs when the time comes because
the changes outside of //components/autofill are super-trivial)

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:33: 'autofill_content_common',
The dependency needs to be the other way around: autofill_content_common depends
on autofill_core_common.

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:48: '<(SHARED_INTERMEDIATE_DIR)/autofill'
Was this just for the IPC code? If so it's not necessary anymore.

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:95: '<(SHARED_INTERMEDIATE_DIR)/autofill'
Needs a ','.

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS
File components/autofill/DEPS (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS...
components/autofill/DEPS:7: "-ipc",
Put this under the "layered component" comment (in alphabetical order with the
other subtractions there).

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS...
components/autofill/DEPS:11: # TODO(blundell): This subtraction can be
eliminated once crbug.com/314560 is
While you're here, could you remove these three lines? That bug is fixed now.

https://codereview.chromium.org/108283005/diff/20001/components/autofill/cont...
File components/autofill/content/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill/cont...
components/autofill/content/common/DEPS:2:
"+content/public/common/common_param_traits.h",
nit: You can cut these off at the level of the innermost directory.

[Ilya Sherman, 2013-12-11 22:32:56]

RSLGTM

[jif-google, 2013-12-12 09:53:59]

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:33: 'autofill_content_common',
On 2013/12/11 15:42:06, blundell wrote:
> The dependency needs to be the other way around: autofill_content_common
depends
> on autofill_core_common.

Done.

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:48: '<(SHARED_INTERMEDIATE_DIR)/autofill'
On 2013/12/11 15:42:06, blundell wrote:
> Was this just for the IPC code? If so it's not necessary anymore.

It's not necessary anymore.

https://codereview.chromium.org/108283005/diff/20001/components/autofill.gypi...
components/autofill.gypi:95: '<(SHARED_INTERMEDIATE_DIR)/autofill'
On 2013/12/11 15:42:06, blundell wrote:
> Needs a ','.
> 

Removed the '<(SHARED_INTERMEDIATE_DIR)/autofill' part instead as it builds fine
with out it.

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS
File components/autofill/DEPS (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS...
components/autofill/DEPS:7: "-ipc",
On 2013/12/11 15:42:06, blundell wrote:
> Put this under the "layered component" comment (in alphabetical order with the
> other subtractions there).

Done.

https://codereview.chromium.org/108283005/diff/20001/components/autofill/DEPS...
components/autofill/DEPS:11: # TODO(blundell): This subtraction can be
eliminated once crbug.com/314560 is
On 2013/12/11 15:42:06, blundell wrote:
> While you're here, could you remove these three lines? That bug is fixed now.

Done.

https://codereview.chromium.org/108283005/diff/20001/components/autofill/cont...
File components/autofill/content/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/20001/components/autofill/cont...
components/autofill/content/common/DEPS:2:
"+content/public/common/common_param_traits.h",
On 2013/12/11 15:42:06, blundell wrote:
> nit: You can cut these off at the level of the innermost directory.

Done.

[commit-bot: I haz the power, 2013-12-12 09:56:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeanfrancoisg@chromium.org/108283005/4...

[commit-bot: I haz the power, 2013-12-12 10:33:13]

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, 2013-12-12 11:01:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeanfrancoisg@chromium.org/108283005/7...

[commit-bot: I haz the power, 2013-12-12 11:45:12]

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

[commit-bot: I haz the power, 2013-12-12 13:33:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeanfrancoisg@chromium.org/108283005/1...

[jeanfrancois, 2013-12-12 14:39:29]

On 2013/12/12 13:33:59, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>
https://chromium-status.appspot.com/cq/jeanfrancoisg%40chromium.org/108283005...

For OWNERS:
+jschuh@chromium.org
+sky@chromium.org

[jeanfrancois, 2013-12-12 14:52:25]

On 2013/12/12 14:39:29, jeanfrancois wrote:
> On 2013/12/12 13:33:59, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >
>
https://chromium-status.appspot.com/cq/jeanfrancoisg%2540chromium.org/1082830...
> 
> For OWNERS:
> mailto:+jschuh@chromium.org
> mailto:+sky@chromium.org

jschuh OWNER of:
  chrome/common/importer/profile_import_process_messages.h
  components/autofill/content/common/autofill_messages.h

sky OWNER of:
  chrome/browser/DEPS [5]
  chrome/browser/password_manager/password_form_manager.cc [9]
  chrome/browser/password_manager/password_generation_manager.cc [9]
  chrome/browser/password_manager/password_manager.cc [9]
  chrome/browser/password_manager/password_manager_delegate_impl.cc [9]
  chrome/browser/ui/autofill/tab_autofill_manager_delegate.cc [9]
  chrome/browser/ui/cocoa/browser/password_generation_bubble_controller.mm [14]
  chrome/browser/ui/gtk/password_generation_bubble_gtk.cc [9]
  chrome/browser/ui/views/password_generation_bubble_view.cc [7]
  chrome/common/DEPS [5]
  chrome/renderer/DEPS [5]
  chrome/renderer/autofill/autofill_renderer_browsertest.cc [6]
  chrome/renderer/autofill/form_autocomplete_browsertest.cc [6]
  chrome/renderer/autofill/password_autofill_agent_browsertest.cc [6]
  chrome/renderer/autofill/password_generation_agent_browsertest.cc [6]

aedla OWNER of:
  tools/ipc_fuzzer

[aedla, 2013-12-12 15:20:40]

LGTM for tools/ipc_fuzzer.

[sky, 2013-12-12 17:17:29]

https://codereview.chromium.org/108283005/diff/110001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/110001/chrome/common/DEPS#newc...
chrome/common/DEPS:6: "+components/autofill/core/common",
Can this DEP be removed now?

[blundell, 2013-12-12 17:46:52]

https://codereview.chromium.org/108283005/diff/110001/chrome/common/DEPS
File chrome/common/DEPS (right):

https://codereview.chromium.org/108283005/diff/110001/chrome/common/DEPS#newc...
chrome/common/DEPS:6: "+components/autofill/core/common",
No; there are usages of autofill_switches.h and autofill_pref_names.h.

On 2013/12/12 17:17:29, sky wrote:
> Can this DEP be removed now?

[commit-bot: I haz the power, 2013-12-12 18:22:07]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2013-12-12 19:24:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeanfrancoisg@chromium.org/108283005/1...

[commit-bot: I haz the power, 2013-12-12 19:28:11]

Change committed as 240382