SdchObserver: add OnDictionary{Added,Removed}

SdchObserver: add OnDictionary{Added,Removed}

These methods are called when the observed SdchManager adds or removess
dictionaries.

BUG=

Committed: https://crrev.com/887d31b0a7727bb1c0b00c6d70a7bdef4fd29841
Cr-Commit-Position: refs/heads/master@{#329920}

[Elly Fong-Jones, 2015-05-11 21:08:12]

ellyjones@chromium.org changed reviewers:
+ mmenke@chromium.org

[Elly Fong-Jones, 2015-05-11 21:08:29]

PTAL? adding some events for Helen's SDCH integration CL's tests.

[mmenke, 2015-05-11 21:10:51]

On 2015/05/11 21:08:29, Elly Jones wrote:
> PTAL? adding some events for Helen's SDCH integration CL's tests.

Why are these needed for the Cronet tests and not for any of the pre-existing
tests?  Could we just copy whatever the existing tests do?

[Elly Fong-Jones, 2015-05-11 21:32:57]

On 2015/05/11 21:10:51, mmenke wrote:
> On 2015/05/11 21:08:29, Elly Jones wrote:
> > PTAL? adding some events for Helen's SDCH integration CL's tests.
> 
> Why are these needed for the Cronet tests and not for any of the pre-existing
> tests?  Could we just copy whatever the existing tests do?

The pre-existing tests run an EmbeddedTestServer and assume that "dictionary
request complete" implies "dictionary loaded". The Cronet tests are using a Java
test server that doesn't appear to provide that visibility, so they do a load
(from the client) of the dictionary to "warm the cache", then sleep for 1s while
the C++-side SdchManager code fetches the dictionary.

[mmenke, 2015-05-12 15:46:42]

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_manager_unitt...
File net/base/sdch_manager_unittest.cc (right):

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_manager_unitt...
net/base/sdch_manager_unittest.cc:57: const std::string& /* server_hash */)
override {}
Should have at least a minimal test for these methods.

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_observer.h
File net/base/sdch_observer.h (right):

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_observer.h#ne...
net/base/sdch_observer.h:30: // A dictionary has been removed from the observed
manager.
Should mention this isn't called on clear.

[mmenke, 2015-05-14 19:20:24]

LGTM

https://codereview.chromium.org/1133763003/diff/40001/chrome/browser/net/sdch...
File chrome/browser/net/sdch_browsertest.cc (right):

https://codereview.chromium.org/1133763003/diff/40001/chrome/browser/net/sdch...
chrome/browser/net/sdch_browsertest.cc:294: int fetch_count_;
nit:  DISALLOW_COPY_AND_ASSIGN

[Elly Fong-Jones, 2015-05-14 19:29:29]

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_observer.h
File net/base/sdch_observer.h (right):

https://codereview.chromium.org/1133763003/diff/1/net/base/sdch_observer.h#ne...
net/base/sdch_observer.h:30: // A dictionary has been removed from the observed
manager.
On 2015/05/12 15:46:42, mmenke wrote:
> Should mention this isn't called on clear.

Done.

https://codereview.chromium.org/1133763003/diff/40001/chrome/browser/net/sdch...
File chrome/browser/net/sdch_browsertest.cc (right):

https://codereview.chromium.org/1133763003/diff/40001/chrome/browser/net/sdch...
chrome/browser/net/sdch_browsertest.cc:294: int fetch_count_;
On 2015/05/14 19:20:24, mmenke wrote:
> nit:  DISALLOW_COPY_AND_ASSIGN

This won't work, since this class is used as a value in an std::map below, which
requires elements to be assignable. Oh well :(

[Elly Fong-Jones, 2015-05-14 19:29:32]

The CQ bit was checked by ellyjones@chromium.org

[commit-bot: I haz the power, 2015-05-14 19:30:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1133763003/40001

[commit-bot: I haz the power, 2015-05-14 20:29:05]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-14 20:29:53]

Patchset 3 (id:??) landed as
https://crrev.com/887d31b0a7727bb1c0b00c6d70a7bdef4fd29841
Cr-Commit-Position: refs/heads/master@{#329920}