Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2940403002:
Move SafeBrowsing files from //chrome/browser to a separate target. (Closed)
DescriptionMove SafeBrowsing files from //chrome/browser to a separate target.
BUG=
Review-Url: https://codereview.chromium.org/2940403002
Cr-Original-Commit-Position: refs/heads/master@{#481019}
Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad06659e66708e
Review-Url: https://codereview.chromium.org/2940403002
Cr-Commit-Position: refs/heads/master@{#481081}
Committed: https://chromium.googlesource.com/chromium/src/+/a347106e6fb949ca7c459e94ec1ae3f71e96d95a
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : . #
Total comments: 3
Patch Set 4 : . #Patch Set 5 : . #Patch Set 6 : . #
Messages
Total messages: 35 (20 generated)
The CQ bit was checked by yzshen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by yzshen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
yzshen@chromium.org changed reviewers: + rockot@chromium.org, vakh@chromium.org
Hi, PTAL. Thanks! Ken: chrome/browser/extensions/BUILD.gn Varun: the rest
https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD.gn File chrome/browser/ui/BUILD.gn (right): https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD... chrome/browser/ui/BUILD.gn:478: "//chrome/browser/ui/webui/omnibox:mojo_bindings", Do you know why this dep is needed now when it wasn't needed earlier?
https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD.gn File chrome/browser/ui/BUILD.gn (right): https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD... chrome/browser/ui/BUILD.gn:478: "//chrome/browser/ui/webui/omnibox:mojo_bindings", On 2017/06/19 23:57:25, vakh (Varun Khaneja) wrote: > Do you know why this dep is needed now when it wasn't needed earlier? Do you mean the omnibox:mojo_bindings or safe_browsing? "omnibox:mojo_bindings" is always there. "safe_browsing": I think that is because "ui" includes some safe_browsing headers. We have the following line in //chrome/browser: allow_circular_includes_from = [ "//chrome/browser/ui" ] So it is not an issue before. Now that I moved SB files out of that target, gn check complained unless I added this dependency.
lgtm https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD.gn File chrome/browser/ui/BUILD.gn (right): https://codereview.chromium.org/2940403002/diff/40001/chrome/browser/ui/BUILD... chrome/browser/ui/BUILD.gn:478: "//chrome/browser/ui/webui/omnibox:mojo_bindings", On 2017/06/20 20:48:20, yzshen1 wrote: > On 2017/06/19 23:57:25, vakh (Varun Khaneja) wrote: > > Do you know why this dep is needed now when it wasn't needed earlier? > > Do you mean the omnibox:mojo_bindings or safe_browsing? > > "omnibox:mojo_bindings" is always there. > > "safe_browsing": I think that is because "ui" includes some safe_browsing > headers. We have the following line in //chrome/browser: > allow_circular_includes_from = [ "//chrome/browser/ui" ] > So it is not an issue before. > > Now that I moved SB files out of that target, gn check complained unless I added > this dependency. > Yes, I meant safe_browsing. Thanks for the clarification.
Friendly ping, Ken. Please take a look at the extensions BUILD.gn change. On Tue, Jun 20, 2017 at 2:07 PM, <vakh@chromium.org> wrote: > lgtm > > > > > https://codereview.chromium.org/2940403002/diff/40001/ > chrome/browser/ui/BUILD.gn > File chrome/browser/ui/BUILD.gn (right): > > https://codereview.chromium.org/2940403002/diff/40001/ > chrome/browser/ui/BUILD.gn#newcode478 > chrome/browser/ui/BUILD.gn:478: > "//chrome/browser/ui/webui/omnibox:mojo_bindings", > On 2017/06/20 20:48:20, yzshen1 wrote: > > On 2017/06/19 23:57:25, vakh (Varun Khaneja) wrote: > > > Do you know why this dep is needed now when it wasn't needed > earlier? > > > > Do you mean the omnibox:mojo_bindings or safe_browsing? > > > > "omnibox:mojo_bindings" is always there. > > > > "safe_browsing": I think that is because "ui" includes some > safe_browsing > > headers. We have the following line in //chrome/browser: > > allow_circular_includes_from = [ "//chrome/browser/ui" ] > > So it is not an issue before. > > > > Now that I moved SB files out of that target, gn check complained > unless I added > > this dependency. > > > > Yes, I meant safe_browsing. Thanks for the clarification. > > https://codereview.chromium.org/2940403002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
lgtm
The CQ bit was checked by yzshen@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by yzshen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org, vakh@chromium.org Link to the patchset: https://codereview.chromium.org/2940403002/#ps80001 (title: ".")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1497995198145270,
"parent_rev": "fa7b3da220ade97ad23b41c56be59fb76c96691b", "commit_rev":
"e4da0e76b5b75d453fec094f62ad06659e66708e"}
Message was sent while issue was closed.
Description was changed from ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= ========== to ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= Review-Url: https://codereview.chromium.org/2940403002 Cr-Commit-Position: refs/heads/master@{#481019} Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad...
Message was sent while issue was closed.
On 2017/06/20 at 23:15:10, commit-bot wrote: > Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... I think this has caused compile failures: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.gpu%2FGPU_Linux...
Message was sent while issue was closed.
On 2017/06/20 at 23:41:01, pdr. wrote: > On 2017/06/20 at 23:15:10, commit-bot wrote: > > Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... > > I think this has caused compile failures: > https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.gpu%2FGPU_Linux... More failures: https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac%20Builder/bu... https://uberchromegw.corp.google.com/i/chromium.win/builders/Win%20x64%20Buil...
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2951883002/ by pdr@chromium.org. The reason for reverting is: This caused compile failures (https://uberchromegw.corp.google.com/i/chromium.win/builders/Win%20x64%20Buil...).
Message was sent while issue was closed.
Description was changed from ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= Review-Url: https://codereview.chromium.org/2940403002 Cr-Commit-Position: refs/heads/master@{#481019} Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... ========== to ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= Review-Url: https://codereview.chromium.org/2940403002 Cr-Commit-Position: refs/heads/master@{#481019} Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... ==========
The CQ bit was checked by yzshen@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org, vakh@chromium.org Link to the patchset: https://codereview.chromium.org/2940403002/#ps100001 (title: ".")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1498004602328490,
"parent_rev": "3cfd4f26430b0625455aa5abf90b2ed4ec129070", "commit_rev":
"a347106e6fb949ca7c459e94ec1ae3f71e96d95a"}
Message was sent while issue was closed.
Description was changed from ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= Review-Url: https://codereview.chromium.org/2940403002 Cr-Commit-Position: refs/heads/master@{#481019} Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... ========== to ========== Move SafeBrowsing files from //chrome/browser to a separate target. BUG= Review-Url: https://codereview.chromium.org/2940403002 Cr-Original-Commit-Position: refs/heads/master@{#481019} Committed: https://chromium.googlesource.com/chromium/src/+/e4da0e76b5b75d453fec094f62ad... Review-Url: https://codereview.chromium.org/2940403002 Cr-Commit-Position: refs/heads/master@{#481081} Committed: https://chromium.googlesource.com/chromium/src/+/a347106e6fb949ca7c459e94ec1a... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://chromium.googlesource.com/chromium/src/+/a347106e6fb949ca7c459e94ec1a... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
