Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(572)

Issue 196423023: Revert of Include sync.pb.h in sync_data.h. (Closed)

Created:
6 years, 9 months ago by maniscalco
Modified:
6 years, 9 months ago
CC:
chromium-reviews, tim+watch_chromium.org, chromium-apps-reviews_chromium.org, maniscalco+watch_chromium.org, haitaol+watch_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Revert of Include sync.pb.h in sync_data.h. (https://chromiumcodereview.appspot.com/182313002/) Reason for revert: Broke win_x86_rel trybots about two thirds of the time (can't find sync.pb.h). Appears that the build is non-deterministic, gonna need more testing. Original issue's description: > Include sync.pb.h in sync_data.h. > > This change has two parts, including sync.pb.h in sync_data.h, and fixing > gyp dependencies and header includes so that everything still builds. > > Include sync.pb.h in sync_data.h in preparation for using AttachmentId > (a protobuf in sync.pb.h) in sync_data.h. A forward declaration is > insufficient because sync_data.h will be getting a typedef for vector<> > of AttachmentId. > > Several targets (transitively) depend on sync, however, their dependence > is not properly modeled in gyp. Update dependencies and header includes > so they can "see" sync.pb.h (a generated header) and any headers it > includes. > > Without these changes, four targets fail to build: > > 1. extensions_browser - See extensions/extensions.gyp for the fix. Here's > the compiler error: > > In file included from ../../extensions/browser/extension_function.cc:10: > In file included from ../../chrome/browser/extensions/extension_service.h:23: > In file included from ../../chrome/browser/extensions/extension_sync_service.h:12: > In file included from ../../chrome/browser/extensions/app_sync_bundle.h:15: > In file included from ../../chrome/browser/extensions/app_sync_data.h:8: > In file included from ../../chrome/browser/extensions/extension_sync_data.h:11: > In file included from ../../sync/api/sync_change.h:13: > ../../sync/api/sync_data.h:17:10: fatal error: 'sync/protocol/sync.pb.h' file not found > > > 2. debugger - See chrome/chrome.gyp for the fix. Here's the compiler > error: > > In file included from ../../chrome/browser/devtools/browser_list_tabcontents_provider.cc:10: > In file included from ../../chrome/browser/history/top_sites.h:13: > In file included from ../../chrome/browser/history/history_service.h:26: > In file included from ../../chrome/browser/history/delete_directive_handler.h:12: > In file included from ../../sync/api/sync_change_processor.h:10: > ../../sync/api/sync_data.h:17:10: fatal error: 'sync/protocol/sync.pb.h' file not found > > > 3. apps - See chrome/chrome_browser_extensions.gypi for the fix. Here's > the compiler error: > > In file included from ../../apps/app_load_service.cc:12: > In file included from ../../chrome/browser/extensions/extension_service.h:23: > In file included from ../../chrome/browser/extensions/extension_sync_service.h:12: > In file included from ../../chrome/browser/extensions/app_sync_bundle.h:15: > In file included from ../../chrome/browser/extensions/app_sync_data.h:8: > In file included from ../../chrome/browser/extensions/extension_sync_data.h:11: > In file included from ../../sync/api/sync_change.h:13: > ../../sync/api/sync_data.h:17:10: fatal error: 'sync/protocol/sync.pb.h' file not found > > > 4. performance_ui_tests - See chrome/chrome_tests.gypi for the fix. > Here's the compiler error: > > In file included from ../../chrome/test/perf/generate_profile.cc:19: > In file included from ../../chrome/browser/history/history_service.h:26: > In file included from ../../chrome/browser/history/delete_directive_handler.h:12: > In file included from ../../sync/api/sync_change_processor.h:10: > In file included from ../../sync/api/sync_data.h:17: > gen/protoc_out/sync/protocol/sync.pb.h:9:10: fatal error: 'google/protobuf/stubs/common.h' file not found > > BUG= > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257666 TBR=tim@chromium.org,asargent@chromium.org,thakis@chromium.org,rsleevi@chromium.org NOTREECHECKS=true NOTRY=true BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257833

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -18 lines) Patch
M chrome/browser/devtools/adb/android_rsa.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/devtools/devtools_window.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/extension_disabled_ui.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_service.h View 2 chunks +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_service_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/extensions/launch_util.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/history/top_sites.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/sync/test/integration/two_client_apps_sync_test.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/ntp/suggestions_source_top_sites.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/chrome.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M components/password_manager.gypi View 2 chunks +0 lines, -4 lines 0 comments Download
M sync/api/sync_data.h View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
maniscalco
Created Revert of Include sync.pb.h in sync_data.h.
6 years, 9 months ago (2014-03-19 00:28:17 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maniscalco@chromium.org/196423023/1
6 years, 9 months ago (2014-03-19 00:30:54 UTC) #2
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-19 00:30:55 UTC) #3
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 9 months ago (2014-03-19 00:30:56 UTC) #4
Lei Zhang
lgtm stamp
6 years, 9 months ago (2014-03-19 00:33:32 UTC) #5
maniscalco
The CQ bit was checked by maniscalco@chromium.org
6 years, 9 months ago (2014-03-19 00:34:16 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/maniscalco@chromium.org/196423023/1
6 years, 9 months ago (2014-03-19 00:35:07 UTC) #7
commit-bot: I haz the power
6 years, 9 months ago (2014-03-19 00:39:39 UTC) #8
Message was sent while issue was closed.
Change committed as 257833

Powered by Google App Engine
This is Rietveld 408576698