Remove dead code related to base::JsonPrefStore without a path.

Nobody uses base::JsonPrefStore::ReadResult without a path
and if they had tried it, it would have crashed.

This removes the code that tried to support non-path
JsonPrefStore and adds a DCHECK in the constructor in case
it happens in the future.

If the code is ever needed in the future, the
scoped_ptr<ReadResult> need to be initialized
with new ReadResult.

BUG=
R=bauerb@chromium.org

Committed: https://crrev.com/970f39d3eeac9baa033a48dfe3310df8e388c68f
Cr-Commit-Position: refs/heads/master@{#310300}

[Daniel Bratell, 2015-01-07 10:14:12]

baurb, please take a look at this 2 line patch.

The code has been like this for a while without crashing (to my knowledge) so I
suspect it might be dead code. Maybe it should be removed rather than fixed?

[Daniel Bratell, 2015-01-07 10:15:22]

bratell@opera.com changed reviewers:
+ bauerb@chromium.org
- baurb@chromium.org

[Daniel Bratell, 2015-01-07 10:16:02]

bauerb, please take a look at this 2 line patch (sorry about the misspelling.
:-( )

[Bernhard Bauer, 2015-01-07 10:28:58]

Yeah, constructing a JsonPrefStore with an empty path seems like more of a
programmer error (i.e. DCHECK) than an error condition that should be signaled
to the user. Luckily, the absence of crashes seems to suggest that this doesn't
actually happen :D If you want to remove this code (or replace it with a DCHECK
at construction time), I'd be okay with it.

[Daniel Bratell, 2015-01-07 10:53:01]

Less code is better so just a DCHECK for now.

Does this look fine? The code didn't seem to trigger when I ran content_shell so
I can't say I've done much testing. :-)

[Bernhard Bauer, 2015-01-07 11:01:03]

On 2015/01/07 10:53:01, Daniel Bratell wrote:
> Less code is better so just a DCHECK for now.
> 
> Does this look fine? The code didn't seem to trigger when I ran content_shell
so
> I can't say I've done much testing. :-)

Does content_shell even use preferences? :) But given that the old code didn't
seem to cause any crashes, I'm fairly certain that removing it will be fine.

https://codereview.chromium.org/836093005/diff/20001/base/prefs/json_pref_sto...
File base/prefs/json_pref_store.cc (right):

https://codereview.chromium.org/836093005/diff/20001/base/prefs/json_pref_sto...
base/prefs/json_pref_store.cc:260: DCHECK(!path_.empty());
Can we do this check at construction time?

[Daniel Bratell, 2015-01-07 13:44:34]

Please take a new look.

Moved the DCHECK to the constructors, and got the code to run successfully
inside Chrome.

https://codereview.chromium.org/836093005/diff/20001/base/prefs/json_pref_sto...
File base/prefs/json_pref_store.cc (right):

https://codereview.chromium.org/836093005/diff/20001/base/prefs/json_pref_sto...
base/prefs/json_pref_store.cc:260: DCHECK(!path_.empty());
On 2015/01/07 11:01:03, Bernhard Bauer wrote:
> Can we do this check at construction time?

Done.

[Bernhard Bauer, 2015-01-07 15:40:04]

LGTM, but can you update the description to reflect the new code? Thanks!

[Daniel Bratell, 2015-01-07 15:41:54]

On 2015/01/07 15:40:04, Bernhard Bauer wrote:
> LGTM, but can you update the description to reflect the new code? Thanks!

Thanks, and will do! Good reminder. I've forgotten that several times in the
past.

[Daniel Bratell, 2015-01-07 15:45:42]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2015-01-07 15:46:22]

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

[commit-bot: I haz the power, 2015-01-07 16:41:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-07 16:42:36]

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