MD History: Delete Grouped History

MD History: Delete Grouped History

The current implementation of grouped mode for MD History is overly
complex and doesn't provide a great user experience. We have decided
not to go ahead with launching this feature, instead focusing on other
WebUI projects.

This deletes grouped history mode from the page, including the router,
the toolbar and the main list. Further simplification (removing
list-container and list-behavior) will come in a follow-up CL.

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

Review-Url: https://codereview.chromium.org/2684493002
Cr-Commit-Position: refs/heads/master@{#450114}
Committed: https://chromium.googlesource.com/chromium/src/+/13af982ac673d665cb43a8a408d42256afece0c2

[tsergeant, 2017-02-07 01:50:21]

Description was changed from

==========
MD History: Delete Grouped History

The current implementation of grouped mode for MD History is overly
complex and doesn't provide a great user experience. We have decided
not to go ahead with launching this feature, instead focusing on other
WebUI projects.

This deletes grouped history mode from the page, including the router,
the toolbar and the main list. Further simplification (removing
list-container and list-behavior) will come in a follow-up CL.

BUG=589357
==========

to

==========
MD History: Delete Grouped History

The current implementation of grouped mode for MD History is overly
complex and doesn't provide a great user experience. We have decided
not to go ahead with launching this feature, instead focusing on other
WebUI projects.

This deletes grouped history mode from the page, including the router,
the toolbar and the main list. Further simplification (removing
list-container and list-behavior) will come in a follow-up CL.

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

[tsergeant, 2017-02-07 01:50:58]

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

[commit-bot: I haz the power, 2017-02-07 01:51: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-02-07 02:59:47]

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

[commit-bot: I haz the power, 2017-02-07 02:59:48]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-02-07 03:08:26]

tsergeant@chromium.org changed reviewers:
+ calamity@chromium.org

[tsergeant, 2017-02-07 03:08:26]

🙅

[Dan Beam, 2017-02-07 18:14:12]

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

[Dan Beam, 2017-02-07 18:14:13]

lgtm but obviously calamity's the master here

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/app.js:245: if (e.command.id ==
'delete-command')
nit: else if or switch()

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/externs.js (left):

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/externs.js:15: *            range:
HistoryRange,
where were these actually removed from C++?

[Dan Beam, 2017-02-07 18:15:08]

https://codereview.chromium.org/2684493002/diff/1/chrome/test/data/webui/md_h...
File chrome/test/data/webui/md_history/history_grouped_list_test.js (left):

https://codereview.chromium.org/2684493002/diff/1/chrome/test/data/webui/md_h...
chrome/test/data/webui/md_history/history_grouped_list_test.js:22: '2016-03-16',
'https://www.google.com/?q=yoloswaggins'),
RIP yolo swaggins 😭

[tsergeant, 2017-02-08 00:12:36]

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/externs.js (left):

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/externs.js:15: *            range:
HistoryRange,
On 2017/02/07 18:14:13, Dan Beam wrote:
> where were these actually removed from C++?

This typedef is purely for JS -- it doesn't have a C++ equivalent. Perhaps
externs is not the right place for it? (I'm not sure where would be better,
though)

https://codereview.chromium.org/2684493002/diff/1/chrome/test/data/webui/md_h...
File chrome/test/data/webui/md_history/history_grouped_list_test.js (left):

https://codereview.chromium.org/2684493002/diff/1/chrome/test/data/webui/md_h...
chrome/test/data/webui/md_history/history_grouped_list_test.js:22: '2016-03-16',
'https://www.google.com/?q=yoloswaggins'),
On 2017/02/07 18:15:08, Dan Beam wrote:
> RIP yolo swaggins 😭

😞

[Dan Beam, 2017-02-08 00:38:53]

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_history/externs.js (left):

https://codereview.chromium.org/2684493002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_history/externs.js:15: *            range:
HistoryRange,
On 2017/02/08 00:12:36, tsergeant wrote:
> On 2017/02/07 18:14:13, Dan Beam wrote:
> > where were these actually removed from C++?
> 
> This typedef is purely for JS -- it doesn't have a C++ equivalent. Perhaps
> externs is not the right place for it? (I'm not sure where would be better,
> though)

ehhh, in an ideal world (I guess) this would be a types.js or something

but that'd mean more files.  and meh.

[calamity, 2017-02-09 02:45:08]

Can you also add a TODO on me in BrowsingHistoryHandler to clean up the grouped
data fields in OnQueryComplete?

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/query_manager.js (left):

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/query_manager.js:100: /** @type {{range:
?HistoryRange, offset: ?number, search: ?string}} */
Shouldn't need HistoryRange anymore. Also get rid of it from constants.html and
mark the histogram values as deprecated.

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_history_ui.cc (right):

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_history_ui.cc:138:
source->AddBoolean("groupByDomain", group_by_domain);
Nuke this.

[tsergeant, 2017-02-09 03:11:49]

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

[tsergeant, 2017-02-09 03:12:16]

Patchset #4 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-02-09 03:12:30]

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

[tsergeant, 2017-02-09 03:13:42]

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_history/query_manager.js (left):

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_history/query_manager.js:100: /** @type {{range:
?HistoryRange, offset: ?number, search: ?string}} */
On 2017/02/09 02:45:08, calamity wrote:
> Shouldn't need HistoryRange anymore. Also get rid of it from constants.html
and
> mark the histogram values as deprecated.

Done.

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_history_ui.cc (right):

https://codereview.chromium.org/2684493002/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_history_ui.cc:138:
source->AddBoolean("groupByDomain", group_by_domain);
On 2017/02/09 02:45:08, calamity wrote:
> Nuke this.

Done.

[commit-bot: I haz the power, 2017-02-09 06:04:39]

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

[commit-bot: I haz the power, 2017-02-09 06:04:40]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-02-13 01:51:06]

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

[commit-bot: I haz the power, 2017-02-13 01:51:12]

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

[calamity, 2017-02-13 02:52:27]

lgtm

[commit-bot: I haz the power, 2017-02-13 03:04:08]

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

[commit-bot: I haz the power, 2017-02-13 03:04:09]

Dry run: This issue passed the CQ dry run.

[tsergeant, 2017-02-13 03:52:17]

tsergeant@chromium.org changed reviewers:
+ isherman@chromium.org

[tsergeant, 2017-02-13 03:52:17]

+isherman@ for histograms.xml.

[Ilya Sherman, 2017-02-13 22:07:45]

histograms.xml lgtm

[Ilya Sherman, 2017-02-13 22:08:00]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2017-02-13 22:08:01]

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/2684493002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-13 22:08:47]

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-13 22:18:05]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487023680898520,
"parent_rev": "a232b05bbe4456d3e7ac38074a48a35488ea6f8b", "commit_rev":
"13af982ac673d665cb43a8a408d42256afece0c2"}

[commit-bot: I haz the power, 2017-02-13 22:18:46]

Description was changed from

==========
MD History: Delete Grouped History

The current implementation of grouped mode for MD History is overly
complex and doesn't provide a great user experience. We have decided
not to go ahead with launching this feature, instead focusing on other
WebUI projects.

This deletes grouped history mode from the page, including the router,
the toolbar and the main list. Further simplification (removing
list-container and list-behavior) will come in a follow-up CL.

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

to

==========
MD History: Delete Grouped History

The current implementation of grouped mode for MD History is overly
complex and doesn't provide a great user experience. We have decided
not to go ahead with launching this feature, instead focusing on other
WebUI projects.

This deletes grouped history mode from the page, including the router,
the toolbar and the main list. Further simplification (removing
list-container and list-behavior) will come in a follow-up CL.

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

Review-Url: https://codereview.chromium.org/2684493002
Cr-Commit-Position: refs/heads/master@{#450114}
Committed:
https://chromium.googlesource.com/chromium/src/+/13af982ac673d665cb43a8a408d4...
==========

[commit-bot: I haz the power, 2017-02-13 22:18:48]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/13af982ac673d665cb43a8a408d4...