Disable SDCH by Default

Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals dumps that need to be examined from earlier versions of Chrome.  

BUG=696815
patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2785493003
Cr-Commit-Position: refs/heads/master@{#461100}
Committed: https://chromium.googlesource.com/chromium/src/+/4152e1751a735f2c0f2cffc30976f9c5ed233f95

[Randy Smith (Not in Mondays), 2017-03-28 21:40:53]

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

[commit-bot: I haz the power, 2017-03-28 21:41:05]

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

[Randy Smith (Not in Mondays), 2017-03-29 14:19:20]

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

[commit-bot: I haz the power, 2017-03-29 14:19:40]

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-29 14:53:05]

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

[commit-bot: I haz the power, 2017-03-29 14:53:05]

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

[Randy Smith (Not in Mondays), 2017-03-29 17:30:32]

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

[commit-bot: I haz the power, 2017-03-29 17:31:09]

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

[Randy Smith (Not in Mondays), 2017-03-29 17:32:27]

Description was changed from

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
==========

to

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals
dumps that need to be examined from earlier versions of Chrome.  

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
==========

[commit-bot: I haz the power, 2017-03-29 20:49:05]

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

[commit-bot: I haz the power, 2017-03-29 20:49:06]

Dry run: This issue passed the CQ dry run.

[Randy Smith (Not in Mondays), 2017-03-29 23:21:05]

rdsmith@chromium.org changed reviewers:
+ eroman@chromium.org, xunjieli@chromium.org

[Randy Smith (Not in Mondays), 2017-03-29 23:21:06]

Helen, could you review this?  It's close to Ryan's original, with some tweaks. 
I'm not intending to address the issue you raised on the original CL in this one
(unless you object--it's messy, and I suspect usually ignored, for all that I
agree with you about it), but I'm trying to follow up on it offline and will at
minimum file a bug for it.

Eric, would you take a look at the net internals test changes?  I chose not to
remove the SDCH tab, since net-internals is often used to look at files
generated by previous versions of Chrome--if you'd like a more nuanced approach,
let me know what it is.

Ryan, FYI.  No review needed, though your review is welcome.

[eroman, 2017-03-29 23:27:53]

for net_internals, overall approach SGTM. Can you also add a TODO comment in
sdch_view.js which:
 * Explains when SDCH was disabled
 * Gives a tentative milestone when the code should be re-visited for deletion
(4-5 milestones in the future is pretty liberal I think).

[Randy Smith (Not in Mondays), 2017-03-30 16:44:13]

Eric: Done.  PTAL.

[xunjieli, 2017-03-30 16:58:28]

Should we also update chrome_browser_state_impl_io_data.mm ?

[eroman, 2017-03-30 17:00:25]

https://codereview.chromium.org/2785493003/diff/70001/chrome/test/data/webui/...
File chrome/test/data/webui/net_internals/sdch_view.js (right):

https://codereview.chromium.org/2785493003/diff/70001/chrome/test/data/webui/...
chrome/test/data/webui/net_internals/sdch_view.js:5: // TODO(rdsmith) Note that
SDCH has been disabled in Chrome as of M59.
Please put this in the implementation file rather than the test file.

Other than that, net_internals LGTM

[Randy Smith (Not in Mondays), 2017-03-30 17:10:23]

Description was changed from

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals
dumps that need to be examined from earlier versions of Chrome.  

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
==========

to

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals
dumps that need to be examined from earlier versions of Chrome.  

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Randy Smith (Not in Mondays), 2017-03-30 17:11:18]

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

[commit-bot: I haz the power, 2017-03-30 17:11:40]

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

[Randy Smith (Not in Mondays), 2017-03-30 17:19:32]

rdsmith@chromium.org changed reviewers:
+ mmenke@chromium.org, sdefresne@chromium.org, sky@chromium.org

[Randy Smith (Not in Mondays), 2017-03-30 17:19:33]

I've responded to comments; PTAL.

Adding in people for OWNERS stamps:

sky@: chrome/common/chrome_constants.* (sorry, didn't find a narrower reviewer
for those files)
mmenke@: chrome/browser/profiles
sdefresne@: ios/

[commit-bot: I haz the power, 2017-03-30 17:21:09]

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

[commit-bot: I haz the power, 2017-03-30 17:21:10]

Dry run: 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...)

[mmenke, 2017-03-30 17:22:20]

https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
File chrome/browser/profiles/profile_impl_io_data.cc (left):

https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
chrome/browser/profiles/profile_impl_io_data.cc:453:
network_json_store_->ReadPrefsAsync(nullptr);
Should we do something about cleaning up the old file?

[Randy Smith (Not in Mondays), 2017-03-30 17:33:25]

https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
File chrome/browser/profiles/profile_impl_io_data.cc (left):

https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
chrome/browser/profiles/profile_impl_io_data.cc:453:
network_json_store_->ReadPrefsAsync(nullptr);
On 2017/03/30 17:22:20, mmenke wrote:
> Should we do something about cleaning up the old file?

Helen raised that issue on the original CL, and I have a question out to a
profiles OWNER about it, but I wasn't planning to do anything about that in this
CL.

[mmenke, 2017-03-30 17:35:59]

On 2017/03/30 17:33:25, Randy Smith (Not in Mondays) wrote:
>
https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
> File chrome/browser/profiles/profile_impl_io_data.cc (left):
> 
>
https://codereview.chromium.org/2785493003/diff/90001/chrome/browser/profiles...
> chrome/browser/profiles/profile_impl_io_data.cc:453:
> network_json_store_->ReadPrefsAsync(nullptr);
> On 2017/03/30 17:22:20, mmenke wrote:
> > Should we do something about cleaning up the old file?
> 
> Helen raised that issue on the original CL, and I have a question out to a
> profiles OWNER about it, but I wasn't planning to do anything about that in
this
> CL.

OK, profiles/ LGTM

[xunjieli, 2017-03-30 17:51:03]

LGTM % off_the_record_chrome_browser_state_io_data.mm needs the same change.

[Randy Smith (Not in Mondays), 2017-03-30 18:01:02]

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

[Randy Smith (Not in Mondays), 2017-03-30 18:01:10]

On 2017/03/30 17:51:03, xunjieli wrote:
> LGTM % off_the_record_chrome_browser_state_io_data.mm needs the same change.

Done; thanks!

[commit-bot: I haz the power, 2017-03-30 18:01:37]

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-30 19:07:25]

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

[commit-bot: I haz the power, 2017-03-30 19:07:25]

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...)

[sky, 2017-03-30 20:34:17]

LGTM

[droger, 2017-03-31 11:50:35]

droger@chromium.org changed reviewers:
+ droger@chromium.org

[droger, 2017-03-31 11:50:36]

lgtm

[Randy Smith (Not in Mondays), 2017-03-31 11:51:08]

The CQ bit was checked by rdsmith@chromium.org

[Randy Smith (Not in Mondays), 2017-03-31 11:51:08]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org,
xunjieli@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2785493003/#ps110001
(title: "Clean up IOS compilation errors and remove from IOS OTR profile.")

[commit-bot: I haz the power, 2017-03-31 11:51:33]

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

[sdefresne, 2017-03-31 12:07:31]

ios/ lgtm

[commit-bot: I haz the power, 2017-03-31 12:29:49]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1490961068094420,
"parent_rev": "190797353dea5c2a1faae22e60c528c3f11d983e", "commit_rev":
"4152e1751a735f2c0f2cffc30976f9c5ed233f95"}

[commit-bot: I haz the power, 2017-03-31 12:30:38]

Description was changed from

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals
dumps that need to be examined from earlier versions of Chrome.  

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Disable SDCH by Default

Disable SDCH in Chrome. It remains accessible via either explicit
initialization of an SdchManager/SdchOwner or through the use of
URLRequestContextBuilder's set_sdch_enabled(true)

This does not disable the SDCH netlog panel, as there may be SDCH net-internals
dumps that need to be examined from earlier versions of Chrome.  

BUG=696815

patch from issue 2720613006 at patchset 1 (http://crrev.com/2720613006#ps1)
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2785493003
Cr-Commit-Position: refs/heads/master@{#461100}
Committed:
https://chromium.googlesource.com/chromium/src/+/4152e1751a735f2c0f2cffc30976...
==========

[commit-bot: I haz the power, 2017-03-31 12:30:39]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/4152e1751a735f2c0f2cffc30976...