New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 665489 link

Starred by 1 user
Status: Assigned
Owner:
█ alokp@chromium.org
Last visit > 30 days ago
Cc:
█ lcwu@chromium.org
sanfin@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 663067



Sign in to add a comment

Cleanup cyclic dependency between chromecast components

Project Member Reported by alokp@chromium.org, Nov 15 2016 
Currently there are multiple instances of cyclic dependency between various chromecast components. This may be because for a long time anything under chromecast was allowed to depend on anything else. We did not enforce DEPS properly. The result is a big ball of mud.

We clarified and started enforcing DEPS starting this patch:
https://codereview.chromium.org/2206813002

More needs to be done to eliminate bad dependencies:

1. No chromecast module should depend back on content layer. The content layer consists of code in the following directories:
- content/
- chromecast/app/
- chromecast/browser/
- chromecast/renderer/
- chromecast/common/
- chromecast/internal/shell/

2. There should be no cyclic dependencies between modules.

Comment 1 by alokp@chromium.org, Nov 15 2016

Cc: lcwu@chromium.org

Comment 2 by alokp@chromium.org, Nov 15 2016 

chromecast/utility also belongs in the content layer.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98afc7db00a7bfe749914e4d618ccb9cc76cbb97

commit 98afc7db00a7bfe749914e4d618ccb9cc76cbb97
Author: alokp <alokp@chromium.org>
Date: Tue Nov 15 18:51:42 2016

[chromecast] Make testnames consistent.

Note that cast_shell_tests were not being run on the bots. This patch
moves it into a top-level cast_shell_unittests target.

BUG=665489
TBR=dpranke,sky

Review-Url: https://codereview.chromium.org/2499563002
Cr-Commit-Position: refs/heads/master@{#432225}

[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/BUILD.gn
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/app/BUILD.gn
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/app/DEPS
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/app/cast_test_launcher.cc
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/BUILD.gn
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/cast_media_blocker_browsertest.cc
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/cast_media_blocker_unittest.cc
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/devtools/cast_devtools_manager_delegate_unittest.cc
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/test/DEPS
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/test/cast_browser_test.cc
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/test/cast_browser_test.h
[rename] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/chromecast/browser/test/cast_navigation_browsertest.cc
[delete] https://crrev.com/3c50f35f90524844e9f69bc19bd05a0dd34351d5/chromecast/browser/test/chromecast_browser_test_runner.cc
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97/testing/buildbot/gn_isolate_map.pyl
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f25f76c28650c6d27dbdc972cf0027516066a2a

commit 9f25f76c28650c6d27dbdc972cf0027516066a2a
Author: dpranke <dpranke@chromium.org>
Date: Tue Nov 15 23:40:36 2016

Revert of [chromecast] Make testnames consistent. (patchset #6 id:100001 of https://codereview.chromium.org/2499563002/ )

Reason for revert:
This broke the cast_shell_linux builder, because gn_isolate_map.pyl was not updated correctly, causing the analyze step to fail, e.g.:

https://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/261305/steps/analyze/logs/stdio

Unfortunately, because the //testing files (and some of the BUILD.gn files) were changed, we bypassed the actual analyze invocation, so we didn't notice this in the try jobs.

Original issue's description:
> [chromecast] Make testnames consistent.
>
> Note that cast_shell_tests were not being run on the bots. This patch
> moves it into a top-level cast_shell_unittests target.
>
> BUG=665489
> TBR=dpranke,sky
>
> Committed: https://crrev.com/98afc7db00a7bfe749914e4d618ccb9cc76cbb97
> Cr-Commit-Position: refs/heads/master@{#432225}

TBR=derekjchow@chromium.org,halliwell@chromium.org,alokp@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=665489

Review-Url: https://codereview.chromium.org/2502143002
Cr-Commit-Position: refs/heads/master@{#432290}

[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/BUILD.gn
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/app/BUILD.gn
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/app/DEPS
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/BUILD.gn
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/DEPS
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/cast_devtools_delegate_test.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/cast_media_blocker_test.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/cast_test_launcher.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/chromecast_browser_test.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/chromecast_browser_test.h
[add] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/chromecast_browser_test_runner.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/chromecast_shell_browser_test.cc
[rename] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/chromecast/browser/test/chromecast_shell_media_blocking_browser_test.cc
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/9f25f76c28650c6d27dbdc972cf0027516066a2a/testing/buildbot/gn_isolate_map.pyl
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36

commit e3bd5edddfc78fba54b08bd35857dde2f8fa6c36
Author: alokp <alokp@chromium.org>
Date: Wed Nov 16 15:57:10 2016

Reland: [chromecast] Make testnames consistent.

Note that cast_shell_tests were not being run on the bots. This patch
moves it into a top-level cast_shell_unittests target.

BUG=665489

Review-Url: https://codereview.chromium.org/2510473002
Cr-Commit-Position: refs/heads/master@{#432501}

[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/BUILD.gn
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/app/BUILD.gn
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/app/DEPS
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/app/cast_test_launcher.cc
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/BUILD.gn
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/cast_media_blocker_browsertest.cc
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/cast_media_blocker_unittest.cc
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/devtools/cast_devtools_manager_delegate_unittest.cc
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/test/DEPS
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/test/cast_browser_test.cc
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/test/cast_browser_test.h
[rename] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/chromecast/browser/test/cast_navigation_browsertest.cc
[delete] https://crrev.com/cb533977c28a294df056853e17581896b73b14e5/chromecast/browser/test/chromecast_browser_test_runner.cc
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/e3bd5edddfc78fba54b08bd35857dde2f8fa6c36/testing/buildbot/gn_isolate_map.pyl

Comment 6 Deleted

Comment 7 by amineer@chromium.org, Nov 16 2016 

[Bulk edit]

Branch issues caused these items to be erroneously tagged as merged to branch 2922, please ignore.

Comment 8 by mmoss@chromium.org, Nov 16 2016

Labels: -merge-merged-2922

Comment 9 by sanfin@chromium.org, Nov 16 2016

Cc: sanfin@chromium.org
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd832a26cda1d5393a973e16c833d43320168270

commit cd832a26cda1d5393a973e16c833d43320168270
Author: alokp <alokp@chromium.org>
Date: Wed Nov 16 19:54:28 2016

[chromecast] Cleanup android DEPS.

BUG=665489

Review-Url: https://codereview.chromium.org/2498423003
Cr-Commit-Position: refs/heads/master@{#432601}

[modify] https://crrev.com/cd832a26cda1d5393a973e16c833d43320168270/chromecast/android/DEPS
[modify] https://crrev.com/cd832a26cda1d5393a973e16c833d43320168270/chromecast/utility/DEPS
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b0e3523e5aa57d6e9ffba7af083835c0ebc5fdb7

commit b0e3523e5aa57d6e9ffba7af083835c0ebc5fdb7
Author: alokp <alokp@chromium.org>
Date: Mon Dec 05 18:24:44 2016

Moves cast-specific prefs into chromecast namespace.

BUG=665489

Review-Url: https://codereview.chromium.org/2555493002
Cr-Commit-Position: refs/heads/master@{#436357}

[modify] https://crrev.com/b0e3523e5aa57d6e9ffba7af083835c0ebc5fdb7/chromecast/base/pref_names.cc
[modify] https://crrev.com/b0e3523e5aa57d6e9ffba7af083835c0ebc5fdb7/chromecast/base/pref_names.h
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 17 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ad924db876e1ee8c8cacb3ca03c484ee75cd4ce

commit 4ad924db876e1ee8c8cacb3ca03c484ee75cd4ce
Author: ryanchung <ryanchung@chromium.org>
Date: Sat Dec 17 00:14:40 2016

Remove PlatformClientAuth interface from upstream.

Moving it to the internal certificate code.
This allows internal certificate code to break dependency on cc/common.

BUG=665489

Review-Url: https://codereview.chromium.org/2553173004
Cr-Commit-Position: refs/heads/master@{#439245}

[modify] https://crrev.com/4ad924db876e1ee8c8cacb3ca03c484ee75cd4ce/chromecast/browser/cast_browser_main_parts.cc
[modify] https://crrev.com/4ad924db876e1ee8c8cacb3ca03c484ee75cd4ce/chromecast/common/BUILD.gn
[delete] https://crrev.com/810fa54c3422d6f7e3666506ee8cd3b84b9b2c82/chromecast/common/platform_client_auth.h
[delete] https://crrev.com/810fa54c3422d6f7e3666506ee8cd3b84b9b2c82/chromecast/common/platform_client_auth_simple.cc
►

Sign in to add a comment