Add a notice about other forms of browsing history to the History WebUI.

Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC1cuEjM/

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332
Committed: https://crrev.com/5872007bbf986d490d45212060d31b101f2cc281
Cr-Commit-Position: refs/heads/master@{#384903}

[msramek, 2016-03-30 13:45:35]

Description was changed from

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

BUG=595332
==========

to

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332
==========

[msramek, 2016-03-30 13:54:16]

msramek@chromium.org changed reviewers:
+ dbeam@chromium.org, jyquinn@chromium.org

[msramek, 2016-03-30 13:54:16]

Hi Dan and Jackie,

please have a look! This is similar to
https://codereview.chromium.org/1813023002/, but on the history page.

Dan - as a WebUI owner.
Jackie - for compatibility with iOS.

I tested this manually on Desktop but not on Android, so I might get back yet
with another patchset with some visual tweaks.

Thanks,
Martin

[msramek, 2016-03-30 13:57:25]

Patchset #1 (id:1) has been deleted

[msramek, 2016-03-30 15:14:32]

Ah, I forgot why I didn't upload this CL earlier, and that was because I was
waiting for https://codereview.chromium.org/1813023002/ to land first, as that
one also contains build files. That's why the tests here were red.

Uploaded changes to .GN and .gyp build files in Patchset #2 as well. I ignored
the comment in both files that "new dependencies should be generally added to
OS!=iOS", because we want to reuse the WebUI history page on iOS. Is that
correct, Jackie?

That means that the build file changes in
https://codereview.chromium.org/1813023002/ and here differ. I'll fix that when
one of the CLs lands.

[Jackie Quinn, 2016-03-30 15:18:04]

On 2016/03/30 15:14:32, msramek wrote:
> Ah, I forgot why I didn't upload this CL earlier, and that was because I was
> waiting for https://codereview.chromium.org/1813023002/ to land first, as that
> one also contains build files. That's why the tests here were red.
> 
> Uploaded changes to .GN and .gyp build files in Patchset #2 as well. I ignored
> the comment in both files that "new dependencies should be generally added to
> OS!=iOS", because we want to reuse the WebUI history page on iOS. Is that
> correct, Jackie?
> 
> That means that the build file changes in
> https://codereview.chromium.org/1813023002/ and here differ. I'll fix that
when
> one of the CLs lands.

We have a fork of all the WebUI history files, so go ahead and move the
dependencies into OS!=iOS. I'm going to have to mirror all the changes into the
ios/ version of the files anyways.

[msramek, 2016-03-30 15:47:07]

On 2016/03/30 15:18:04, Jackie Quinn wrote:
> On 2016/03/30 15:14:32, msramek wrote:
> > Ah, I forgot why I didn't upload this CL earlier, and that was because I was
> > waiting for https://codereview.chromium.org/1813023002/ to land first, as
that
> > one also contains build files. That's why the tests here were red.
> > 
> > Uploaded changes to .GN and .gyp build files in Patchset #2 as well. I
ignored
> > the comment in both files that "new dependencies should be generally added
to
> > OS!=iOS", because we want to reuse the WebUI history page on iOS. Is that
> > correct, Jackie?
> > 
> > That means that the build file changes in
> > https://codereview.chromium.org/1813023002/ and here differ. I'll fix that
> when
> > one of the CLs lands.
> 
> We have a fork of all the WebUI history files, so go ahead and move the
> dependencies into OS!=iOS. I'm going to have to mirror all the changes into
the
> ios/ version of the files anyways.

Got it. Done.

[Dan Beam, 2016-03-31 01:05:44]

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.css:51: margin: 1em 0 1em 0;
margin: 1em 0;

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:657:
HistoryModel.prototype.showNotification = function(
this belongs on the view instead

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:671: if (message != '')
if (message)

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:1305:
container.classList.remove('overflow');
var shouldOverflow = ...;
$('container').classList.toggle('overflow', shouldOverflow);

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:2378: historyModel.showNotification(
historyView.showNotification

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/browsing_history_handler.cc (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:314: weak_factory_(this) {
your newly added booleans should also be initialized here or in the header file

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:847: has_synced_results_ =
(results_value != NULL);
nit: nullptr

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:847: has_synced_results_ =
(results_value != NULL);
nit: remove extra parens

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/browsing_history_handler.h (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.h:168: bool
foundOtherFormsOfBrowsingHistory);
cpp_vars_like_this

[msramek, 2016-03-31 19:20:02]

Thanks, Dan! PTAL!

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.css:51: margin: 1em 0 1em 0;
On 2016/03/31 01:05:43, Dan Beam wrote:
> margin: 1em 0;

Done. Forgot about this :)

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/history/history.js (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:657:
HistoryModel.prototype.showNotification = function(
On 2016/03/31 01:05:43, Dan Beam wrote:
> this belongs on the view instead

Done. I originally put it here, since I extracted it from
HistoryModel.prototype.addResults(), which would in turn call
this.view_.showNotification().

This means that there already is a method HistoryModel.showNotification() which
is supposed to be generic. I will define HistoryModel.showWebHistoryNotification
as its specialization that shows a notification specifically about web history
results and other forms of history.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:671: if (message != '')
On 2016/03/31 01:05:43, Dan Beam wrote:
> if (message)

Done.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:1305:
container.classList.remove('overflow');
On 2016/03/31 01:05:43, Dan Beam wrote:
> var shouldOverflow = ...;
> $('container').classList.toggle('overflow', shouldOverflow);

Done.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/resource...
chrome/browser/resources/history/history.js:2378: historyModel.showNotification(
On 2016/03/31 01:05:43, Dan Beam wrote:
> historyView.showNotification

Done. As mentioned above, let's rename it to showWebHistoryNotification() and
use as a layer above the already existing showNotification().

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/browsing_history_handler.cc (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:314: weak_factory_(this) {
On 2016/03/31 01:05:44, Dan Beam wrote:
> your newly added booleans should also be initialized here or in the header
file

Done.

I originally had them here, and then removed them. They are initialized
explicitly in QueryHistory(), together with several other variables, that are
also not mentioned in constructor. There is currently no code path that would
use them uninitialized, but we can initialize them just to be sure.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:847: has_synced_results_ =
(results_value != NULL);
On 2016/03/31 01:05:44, Dan Beam wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.cc:847: has_synced_results_ =
(results_value != NULL);
On 2016/03/31 01:05:44, Dan Beam wrote:
> nit: remove extra parens

Done.

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/browsing_history_handler.h (right):

https://codereview.chromium.org/1838333004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/browsing_history_handler.h:168: bool
foundOtherFormsOfBrowsingHistory);
On 2016/03/31 01:05:44, Dan Beam wrote:
> cpp_vars_like_this

Done. Yep, switching between C++ and Java...

[msramek, 2016-04-01 15:05:57]

We're now matching mocks on both Desktop and Android, and the tests are green.
PTAL!

-------------------------------

Changes in the last Patchset:

1. Fixed the failing tests. BrowsingHistoryHandler also talks to the new MD
history page, so I added a dummy showNotification() method to md_history.js. The
material design page is still able to generate and show a notification from the
historyResult() callback (which contains the information about synced results in
the "info" parameter), so the MD history page is not broken with this CL.

2. Tested on Android and updated history_mobile.css. Most notably:
    2. a) Wrapping the two sentences of the notification into <span>s instead of
<divs>. On desktop, we use "display: block" to make them appear as two
paragraphs, on mobile we set "display: inline" to display them as a single
paragraph (see mocks). This also means we need to change the strings and add a
period after "Learn more" (this is also captured in the mocks).
    2. b) I had to add a light grey color to the top part of the page, including
the empty space between elements. So I changed the background color of the
entire page, and then explicitly set history items to a white color which are
the only ones that did not have a color defined and should not be affected.

3. Updated the selector in the disabled accessibility test (the test doesn't
like "Learn more" links, but they are standard in our WebUI).

[Dan Beam, 2016-04-01 15:32:18]

lgtm but I wish we weren't adding more uses of innerHTML

https://codereview.chromium.org/1838333004/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/history/history.css (right):

https://codereview.chromium.org/1838333004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/history/history.css:41: #top-container.overflow
#notification-bar {
why did you need to change this?

https://codereview.chromium.org/1838333004/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/history/history_mobile.css (right):

https://codereview.chromium.org/1838333004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/history/history_mobile.css:100: On mobile, join them
into a single paragraph. */
/* ...
 * ... */
 ^

[msramek, 2016-04-04 10:03:46]

msramek@chromium.org changed reviewers:
+ blundell@chromium.org

[msramek, 2016-04-04 10:03:46]

In the last patchset, I just updated the string in history_strings.grdp, as the
specification changed since I started writing this CL. Now it matches mocks.

Given that this file is directly in components/, +blundell@, could you please
rubber-stamp it? Thanks!

[blundell, 2016-04-04 10:04:31]

lgtm

[msramek, 2016-04-04 13:22:59]

The CQ bit was checked by msramek@chromium.org

[msramek, 2016-04-04 13:23:00]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1838333004/#ps160001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-04-04 13:23:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1838333004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1838333004/160001

[commit-bot: I haz the power, 2016-04-04 14:34:59]

Description was changed from

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332
==========

to

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332
==========

[commit-bot: I haz the power, 2016-04-04 14:35:00]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2016-04-04 14:36:22]

Description was changed from

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332
==========

to

==========
Add a notice about other forms of browsing history to the History WebUI.

Background:
Design doc:
https://docs.google.com/document/d/1ZMDSAd44KmzKhqXPjobZOf9rZezs6VqiBnqpDD0auCU/
Mocks:
https://docs.google.com/presentation/d/1rpR3xB3aYFzXD0U--piuMq3y4XAmoYdawA2HC...

We expand the existing notification that shows whether there are synced
history results. For users who have other forms of browsing history, we
add one more sentence informing them about this fact.

The call to find out about the existence of other forms of browsing history
is asynchronous, just like the call to get web history results. There are
two outcomes:

1. Web history results arrive first. We have to pass the results to
   the frontend immediately, which also shows the first sentence of the
   notification about synced results. When the ping for other forms of
   browsing history returns, we may or may not show the second sentence
   according to the response.

2. The ping for other forms of history returns first. We show the notification
   saying that there are no synced results, but there are other forms
   of browsing history. When the web history results arrive, we update the
   first sentence of the notification accordingly.

To make this simple, we keep the the booleans (one for each sentence)
in BrowsingHistoryHandler, and then call a single JS method that refreshes
the notification according to their state.

Furthermore, to match the mocks, we update the logic on whether or not the
notification should float beside the editing controls. If the notification
contains only a single sentence and it fits, it can float; otherwise, it must
be moved under the editing controls. The current logic in CSS only changes
the notification bar from float: right to float: left. However, in the case
when the notification contains two sentences, but still fits beside the
editing controls, this is not enough; the first sentence could still float
beside editing controls. We must therefore also set float: none on them.
To do that, we replace the 'alone' class on the notification bar with an
'overflow' class on top controls, so that we can influence the behavior of
both notification bar and editing controls.

BUG=595332

Committed: https://crrev.com/5872007bbf986d490d45212060d31b101f2cc281
Cr-Commit-Position: refs/heads/master@{#384903}
==========

[commit-bot: I haz the power, 2016-04-04 14:36:24]

Patchset 8 (id:??) landed as
https://crrev.com/5872007bbf986d490d45212060d31b101f2cc281
Cr-Commit-Position: refs/heads/master@{#384903}