[Device Discovery] Move files from browser/extensions/api/dial to browser/media/router/discovery/di…

[Device Discovery] Move files from browser/extensions/api/dial to browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and DeviceDescriptionFetcher class. Move them from extension to media router folder so DialMediaSinkService does not depend on files in extension folder. Files moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

BUG=687375
Review-Url: https://codereview.chromium.org/2756483007
Cr-Commit-Position: refs/heads/master@{#459253}
Committed: https://chromium.googlesource.com/chromium/src/+/cbc843e7781e48a4a7655ab650e2a841a4ee9bc2

[zhaobin, 2017-03-17 18:55:38]

zhaobin@chromium.org changed reviewers:
+ imcheng@chromium.org, mfoltz@chromium.org

[mark a. foltz, 2017-03-18 18:24:22]

LGTM

Thanks for doing this!

[mark a. foltz, 2017-03-18 18:25:29]

On 2017/03/18 at 18:24:22, mark a. foltz wrote:
> LGTM
> 
> Thanks for doing this!

Nit: can you list the filenames that are being moved in the patch description?

[zhaobin, 2017-03-20 18:34:27]

Description was changed from

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder.

Changed namespace to media_router and updated unit tests.

Follow up: make DialRegistry a KeyedService. Extension API functions and
DialMediaSinkService should reference the same DialRegistry instance.

BUG=687375
==========

to

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder. Files
moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

Follow up: make DialRegistry a KeyedService. Extension API functions and
DialMediaSinkService should reference the same DialRegistry instance.

BUG=687375
==========

[imcheng, 2017-03-20 21:03:41]

lgtm

Nice!

[zhaobin, 2017-03-20 21:12:35]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 21:13:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zhaobin, 2017-03-20 21:14:43]

zhaobin@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[zhaobin, 2017-03-20 21:14:44]

+rdevlin.cronin@ to review:

chrome/browser/extensions/BUILD.gn

[Devlin, 2017-03-20 21:18:48]

Do we need to update the deps of extensions to explicitly include
media/router/discovery?

[commit-bot: I haz the power, 2017-03-20 21:27:33]

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

[commit-bot: I haz the power, 2017-03-20 21:27:36]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[zhaobin, 2017-03-20 22:02:14]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 22:03:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 22:24:41]

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

[commit-bot: I haz the power, 2017-03-20 22:24:42]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[zhaobin, 2017-03-20 22:47:12]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 22:48:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 23:01:11]

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

[commit-bot: I haz the power, 2017-03-20 23:01:12]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[imcheng, 2017-03-20 23:48:10]

Sorry 2 more comments:

1) I noticed there are still some #include to generated extension API .h files
-- can those be removed?
2) Note that the discovery code should only be included in desktop builds - in
our case, Android should be excluded.

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
File chrome/browser/media/router/discovery/dial/dial_device_data.cc (right):

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
chrome/browser/media/router/discovery/dial/dial_device_data.cc:7: #include
"chrome/common/extensions/api/dial.h"
Remove?

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
File chrome/browser/media/router/discovery/dial/dial_registry.cc (right):

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
chrome/browser/media/router/discovery/dial/dial_registry.cc:18: #include
"chrome/common/extensions/api/dial.h"
Can this be removed?

[zhaobin, 2017-03-21 01:59:39]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 02:00:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 02:10:08]

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

[commit-bot: I haz the power, 2017-03-21 02:10:09]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[zhaobin, 2017-03-21 02:38:28]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 02:38:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 02:48:30]

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

[commit-bot: I haz the power, 2017-03-21 02:48:34]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[zhaobin, 2017-03-21 04:11:48]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 04:12:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-21 05:16:12]

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

[commit-bot: I haz the power, 2017-03-21 05:16:13]

Dry run: This issue passed the CQ dry run.

[zhaobin, 2017-03-21 20:26:53]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 20:27:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[zhaobin, 2017-03-21 21:20:50]

Resolve code review comments from Derek and create a seperate target for
media/router/discovery.

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
File chrome/browser/media/router/discovery/dial/dial_device_data.cc (right):

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
chrome/browser/media/router/discovery/dial/dial_device_data.cc:7: #include
"chrome/common/extensions/api/dial.h"
On 2017/03/20 23:48:10, imcheng wrote:
> Remove?

Done.

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
File chrome/browser/media/router/discovery/dial/dial_registry.cc (right):

https://codereview.chromium.org/2756483007/diff/60001/chrome/browser/media/ro...
chrome/browser/media/router/discovery/dial/dial_registry.cc:18: #include
"chrome/common/extensions/api/dial.h"
On 2017/03/20 23:48:10, imcheng wrote:
> Can this be removed?

Done.

[commit-bot: I haz the power, 2017-03-21 22:01:42]

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

[commit-bot: I haz the power, 2017-03-21 22:01:43]

Dry run: This issue passed the CQ dry run.

[mark a. foltz, 2017-03-21 22:59:04]

Still LGTM.  Thanks for splitting the targets; even if not necessary for
chromecast build, it will help enforce layering in the future.

Please update the comment in the patch description w.r.t. DialRegistry (or just
remove it if unsure).

[zhaobin, 2017-03-21 23:00:38]

Description was changed from

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder. Files
moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

Follow up: make DialRegistry a KeyedService. Extension API functions and
DialMediaSinkService should reference the same DialRegistry instance.

BUG=687375
==========

to

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder. Files
moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

BUG=687375
==========

[imcheng, 2017-03-21 23:15:16]

lgtm

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/media/r...
File chrome/browser/media/router/discovery/BUILD.gn (right):

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/media/r...
chrome/browser/media/router/discovery/BUILD.gn:4: #
Remove extra #

[Devlin, 2017-03-23 17:39:34]

chrome/browser/extensions/BUILD.gn lgtm % nit

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/BUILD.gn (right):

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/BUILD.gn:924: if (!is_android) {
nit: none of the extensions module is ever compiled on android.  You can remove
this if and add this to the main group.

Separately, if media router is the only consumer of dial, it would be great to
move this and the full dial api into the if (enable_media_router) block, but
that can be another change.

[zhaobin, 2017-03-23 18:05:52]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[zhaobin, 2017-03-23 18:06:03]

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/extensi...
File chrome/browser/extensions/BUILD.gn (right):

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/BUILD.gn:924: if (!is_android) {
On 2017/03/23 17:39:34, Devlin wrote:
> nit: none of the extensions module is ever compiled on android.  You can
remove
> this if and add this to the main group.
> 
> Separately, if media router is the only consumer of dial, it would be great to
> move this and the full dial api into the if (enable_media_router) block, but
> that can be another change.

Created crbug.com/704617

https://codereview.chromium.org/2756483007/diff/140001/chrome/browser/extensi...
chrome/browser/extensions/BUILD.gn:924: if (!is_android) {
On 2017/03/23 17:39:34, Devlin wrote:
> nit: none of the extensions module is ever compiled on android.  You can
remove
> this if and add this to the main group.
> 
> Separately, if media router is the only consumer of dial, it would be great to
> move this and the full dial api into the if (enable_media_router) block, but
> that can be another change.

Done.

[commit-bot: I haz the power, 2017-03-23 18:06:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-23 18:50:39]

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

[commit-bot: I haz the power, 2017-03-23 18:50:41]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[zhaobin, 2017-03-23 21:06:39]

The CQ bit was checked by zhaobin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-23 21:07:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-23 22:11:39]

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

[commit-bot: I haz the power, 2017-03-23 22:11:40]

Dry run: This issue passed the CQ dry run.

[zhaobin, 2017-03-23 22:12:12]

The CQ bit was checked by zhaobin@chromium.org

[zhaobin, 2017-03-23 22:12:13]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org,
rdevlin.cronin@chromium.org, imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2756483007/#ps160001
(title: "resolve code review comments from Devlin")

[commit-bot: I haz the power, 2017-03-23 22:12:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-23 22:22:01]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1490307132861270,
"parent_rev": "bedc60e74ddc887d5599b468c83191f992311458", "commit_rev":
"cbc843e7781e48a4a7655ab650e2a841a4ee9bc2"}

[commit-bot: I haz the power, 2017-03-23 22:22:47]

Description was changed from

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder. Files
moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

BUG=687375
==========

to

==========
[Device Discovery] Move files from browser/extensions/api/dial to
browser/media/router/discovery/dial

DialMediaSinkService needs to reference DialRegistry and
DeviceDescriptionFetcher class. Move them from extension to media router folder
so DialMediaSinkService does not depend on files in extension folder. Files
moved to media router folder:

chrome/browser/extensions/api/dial/device_description_fetcher.h
chrome/browser/extensions/api/dial/device_description_fetcher.cc
chrome/browser/extensions/api/dial/device_description_fetcher_unittest.cc
chrome/browser/extensions/api/dial/dial_device_data.h
chrome/browser/extensions/api/dial/dial_device_data.cc
chrome/browser/extensions/api/dial/dial_device_data_unittest.cc
chrome/browser/extensions/api/dial/dial_registry.h
chrome/browser/extensions/api/dial/dial_registry.cc
chrome/browser/extensions/api/dial/dial_registry_unittest.cc
chrome/browser/extensions/api/dial/dial_service.h
chrome/browser/extensions/api/dial/dial_service.cc
chrome/browser/extensions/api/dial/dial_service_unittest.cc

Changed namespace to media_router and updated unit tests.

BUG=687375

Review-Url: https://codereview.chromium.org/2756483007
Cr-Commit-Position: refs/heads/master@{#459253}
Committed:
https://chromium.googlesource.com/chromium/src/+/cbc843e7781e48a4a7655ab650e2...
==========

[commit-bot: I haz the power, 2017-03-23 22:22:49]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/cbc843e7781e48a4a7655ab650e2...