Move OmahaQueryParams to a component and add a delegate interface.

Move OmahaQueryParams to a component and add a delegate interface.

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

[Yoyo Zhou, 2014-06-21 00:04:30]

asargent: please review
brettw: please review new DEPS inclusions of base and content

[brettw, 2014-06-21 00:28:52]

DEPS LGTM, didn't look at the rest

[asargent_no_longer_on_chrome, 2014-06-23 18:21:00]

lgtm

https://codereview.chromium.org/333353005/diff/1/components/omaha_query_param...
File components/omaha_query_params/omaha_query_params.cc (right):

https://codereview.chromium.org/333353005/diff/1/components/omaha_query_param...
components/omaha_query_params/omaha_query_params.cc:132: g_delegate = delegate;
should it be an error to replace a non-null one with another non-null one? 

E.g. do we want:

DCHECK(!g_delegate || !delegate)

here?

https://codereview.chromium.org/333353005/diff/1/content/shell/browser/shell_...
File content/shell/browser/shell_content_browser_client.cc (right):

https://codereview.chromium.org/333353005/diff/1/content/shell/browser/shell_...
content/shell/browser/shell_content_browser_client.cc:13: 
spurious newline?

[Yoyo Zhou, 2014-06-23 23:48:33]

https://codereview.chromium.org/333353005/diff/1/components/omaha_query_param...
File components/omaha_query_params/omaha_query_params.cc (right):

https://codereview.chromium.org/333353005/diff/1/components/omaha_query_param...
components/omaha_query_params/omaha_query_params.cc:132: g_delegate = delegate;
On 2014/06/23 18:20:59, Antony Sargent wrote:
> should it be an error to replace a non-null one with another non-null one? 
> 
> E.g. do we want:
> 
> DCHECK(!g_delegate || !delegate)
> 
> here?

Yes, I think so.

After discussion with the webstore team, it turns out that you can't omit the
prodversion, because pre-M31 chrome is now desupported. I'll add a delegate for
AppShell with a fake but sufficiently high version number.

https://codereview.chromium.org/333353005/diff/1/content/shell/browser/shell_...
File content/shell/browser/shell_content_browser_client.cc (right):

https://codereview.chromium.org/333353005/diff/1/content/shell/browser/shell_...
content/shell/browser/shell_content_browser_client.cc:13: 
On 2014/06/23 18:20:59, Antony Sargent wrote:
> spurious newline?

Done.

[Yoyo Zhou, 2014-06-23 23:52:25]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-06-23 23:54:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yoz@chromium.org/333353005/60001

[Yoyo Zhou, 2014-06-24 00:04:40]

The CQ bit was unchecked by yoz@chromium.org

[Yoyo Zhou, 2014-06-24 00:20:44]

jochen: can you review chrome/ (not extensions) and components/ (top-level)?

[jochen (gone - plz use gerrit), 2014-06-24 08:52:32]

https://codereview.chromium.org/333353005/diff/80001/apps/shell/browser/shell...
File apps/shell/browser/shell_omaha_query_params_delegate.h (right):

https://codereview.chromium.org/333353005/diff/80001/apps/shell/browser/shell...
apps/shell/browser/shell_omaha_query_params_delegate.h:19:
DISALLOW_COPY_AND_ASSIGN(ShellOmahaQueryParamsDelegate);
should be private

https://codereview.chromium.org/333353005/diff/80001/chrome/browser/omaha_que...
File chrome/browser/omaha_query_params/chrome_omaha_query_params_delegate.h
(right):

https://codereview.chromium.org/333353005/diff/80001/chrome/browser/omaha_que...
chrome/browser/omaha_query_params/chrome_omaha_query_params_delegate.h:30:
DISALLOW_COPY_AND_ASSIGN(ChromeOmahaQueryParamsDelegate);
should be private

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
File components/omaha_query_params.gypi (right):

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
components/omaha_query_params.gypi:15: '../content/content.gyp:content_browser',
why do you depend on content_browser but the DEPS says content/public/common?

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
File components/omaha_query_params/omaha_query_params.cc (right):

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
components/omaha_query_params/omaha_query_params.cc:10: #include
"content/public/common/content_client.h"
what are you using this header for?

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
File components/omaha_query_params/omaha_query_params.h (right):

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
components/omaha_query_params/omaha_query_params.h:17: class OmahaQueryParams {
please put components into their own namespace

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
components/omaha_query_params/omaha_query_params.h:53: class
OmahaQueryParamsDelegate {
shouldn't this class be in a separate header?

https://codereview.chromium.org/333353005/diff/80001/components/omaha_query_p...
components/omaha_query_params/omaha_query_params.h:65:
DISALLOW_COPY_AND_ASSIGN(OmahaQueryParamsDelegate);
should be private

[Yoyo Zhou, 2014-06-25 01:05:10]

Jochen, I addressed all comments. Please take another look.

[jochen (gone - plz use gerrit), 2014-06-25 09:44:30]

lgtm

[Yoyo Zhou, 2014-06-25 15:27:11]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-06-25 15:28:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yoz@chromium.org/333353005/120001

[commit-bot: I haz the power, 2014-06-25 15:39:11]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/21420)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/25514)

[commit-bot: I haz the power, 2014-06-25 15:40:48]

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

[commit-bot: I haz the power, 2014-06-25 15:40:49]

Try jobs failed on following builders:
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/21424)

[Yoyo Zhou, 2014-06-25 19:04:41]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2014-06-25 19:06:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yoz@chromium.org/333353005/140001

[commit-bot: I haz the power, 2014-06-25 22:07:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)

[commit-bot: I haz the power, 2014-06-26 00:01:35]

Change committed as 279867