MD Settings: Add "Auto Open" setting to Downloads page UI.

MD Settings: Add "Auto Open" setting to Downloads page UI.

BUG=692662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2713343003
Cr-Commit-Position: refs/heads/master@{#453707}
Committed: https://chromium.googlesource.com/chromium/src/+/915915168110d5bc831be6bfdae58a106f1a3cfe

[tommycli, 2017-02-25 01:17:53]

Description was changed from

==========
MD Settings: Add "Auto Open" setting to Downloads page UI.

BUG=692662
==========

to

==========
MD Settings: Add "Auto Open" setting to Downloads page UI.

BUG=692662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tommycli, 2017-02-25 01:18:12]

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

[commit-bot: I haz the power, 2017-02-25 01:18:22]

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

[tommycli, 2017-02-25 01:19:12]

tommycli@chromium.org changed reviewers:
+ dbeam@chromium.org

[tommycli, 2017-02-25 01:19:12]

dbeam: PTAL

[tommycli, 2017-02-25 01:20:07]

On 2017/02/25 01:19:12, tommycli wrote:
> dbeam: PTAL

Screenie: http://imgur.com/a/mmAVl
UI approved by bettes@. Mock in Downloads PREVIEW section.

[commit-bot: I haz the power, 2017-02-25 01:21:07]

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

[commit-bot: I haz the power, 2017-02-25 01:21:07]

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...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tommycli, 2017-02-25 01:22:02]

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

[commit-bot: I haz the power, 2017-02-25 01:22:17]

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-02-25 02:22:59]

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

[commit-bot: I haz the power, 2017-02-25 02:23:02]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-02-27 05:50:37]

can you add some basic tests for this UI?

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.html
(right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.html:45: <div
class="settings-box" hidden="[[!autoOpenDownloads_]]">
can you make this a dom-if instead of binding to hidden?

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.js (right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:51:
chrome.send('initializeDownloads');
don't chrome.send() directly

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:79:
chrome.send('resetAutoOpenFileTypes');
use a browser proxy

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/downloads_handler.cc (right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/downloads_handler.cc:36: "initializeDownloads",
where is this already called?  was this just unused code until now?

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:701:
IDS_SETTINGS_OPEN_FILE_TYPES_AUTOMATICALLY}};
is this was clang-format does?

https://codereview.chromium.org/2713343003/diff/20001/components/components_s...
File components/components_strings.grd (right):

https://codereview.chromium.org/2713343003/diff/20001/components/components_s...
components/components_strings.grd:223: <message name="IDS_CLEAR" desc="A generic
term for Clear on buttons and menus.">
why are you adding this here?

[tommycli, 2017-02-27 23:50:17]

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

[commit-bot: I haz the power, 2017-02-27 23:50:47]

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

[tommycli, 2017-02-27 23:55:18]

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

[commit-bot: I haz the power, 2017-02-27 23:55:37]

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

[tommycli, 2017-02-28 00:47:27]

dbeam: Thanks! See below

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.html
(right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.html:45: <div
class="settings-box" hidden="[[!autoOpenDownloads_]]">
On 2017/02/27 05:50:36, Dan Beam wrote:
> can you make this a dom-if instead of binding to hidden?

Done.

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.js (right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:51:
chrome.send('initializeDownloads');
On 2017/02/27 05:50:36, Dan Beam wrote:
> don't chrome.send() directly

Done.

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:79:
chrome.send('resetAutoOpenFileTypes');
On 2017/02/27 05:50:36, Dan Beam wrote:
> use a browser proxy

Done.

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/downloads_handler.cc (right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/downloads_handler.cc:36: "initializeDownloads",
On 2017/02/27 05:50:36, Dan Beam wrote:
> where is this already called?  was this just unused code until now?

Yes, added in the previous CL, used in this CL for the first time.

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/2713343003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:701:
IDS_SETTINGS_OPEN_FILE_TYPES_AUTOMATICALLY}};
On 2017/02/27 05:50:36, Dan Beam wrote:
> is this was clang-format does?

Yep...

https://codereview.chromium.org/2713343003/diff/20001/components/components_s...
File components/components_strings.grd (right):

https://codereview.chromium.org/2713343003/diff/20001/components/components_s...
components/components_strings.grd:223: <message name="IDS_CLEAR" desc="A generic
term for Clear on buttons and menus.">
On 2017/02/27 05:50:36, Dan Beam wrote:
> why are you adding this here?

Done.

[commit-bot: I haz the power, 2017-02-28 01:19:45]

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

[commit-bot: I haz the power, 2017-02-28 01:19:47]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-02-28 02:36:42]

lgtm w/nits

https://codereview.chromium.org/2713343003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.js (right):

https://codereview.chromium.org/2713343003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:82: 
nit: @private

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/downloads_page_test.js (right):

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/downloads_page_test.js:37: var DownloadsPage =
null;
jsVarsLikeThis (start with lower)

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/downloads_page_test.js:62: assert(!!button);
nit: assert -> assertTrue

[tommycli, 2017-02-28 17:06:39]

thanks!

https://codereview.chromium.org/2713343003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/downloads_page/downloads_page.js (right):

https://codereview.chromium.org/2713343003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/downloads_page/downloads_page.js:82: 
On 2017/02/28 02:36:42, Dan Beam wrote:
> nit: @private

Done.

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/downloads_page_test.js (right):

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/downloads_page_test.js:37: var DownloadsPage =
null;
On 2017/02/28 02:36:42, Dan Beam wrote:
> jsVarsLikeThis (start with lower)

Done.

https://codereview.chromium.org/2713343003/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/downloads_page_test.js:62: assert(!!button);
On 2017/02/28 02:36:42, Dan Beam wrote:
> nit: assert -> assertTrue

Done.

[tommycli, 2017-02-28 17:06:44]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2017-02-28 17:06:45]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2713343003/#ps80001
(title: "Address nits")

[commit-bot: I haz the power, 2017-02-28 17:06:58]

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-02-28 17:08:56]

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

[commit-bot: I haz the power, 2017-02-28 17:08:58]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tommycli, 2017-02-28 20:09:46]

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

[tommycli, 2017-02-28 20:10:22]

The CQ bit was unchecked by tommycli@chromium.org

[tommycli, 2017-02-28 20:10:22]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2017-02-28 20:10:24]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2713343003/#ps120001
(title: "format")

[commit-bot: I haz the power, 2017-02-28 20:10:55]

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-02-28 21:31:46]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1488312622561790,
"parent_rev": "459f5a96a07236e94cae14eba657fc68498afe1b", "commit_rev":
"915915168110d5bc831be6bfdae58a106f1a3cfe"}

[commit-bot: I haz the power, 2017-02-28 21:34:09]

Description was changed from

==========
MD Settings: Add "Auto Open" setting to Downloads page UI.

BUG=692662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Add "Auto Open" setting to Downloads page UI.

BUG=692662
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2713343003
Cr-Commit-Position: refs/heads/master@{#453707}
Committed:
https://chromium.googlesource.com/chromium/src/+/915915168110d5bc831be6bfdae5...
==========

[commit-bot: I haz the power, 2017-02-28 21:34:11]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/915915168110d5bc831be6bfdae5...