Coverting std::String() to ResourceIdentifier() in content_settings.

Coverting std::String() to ResourceIdentifier() in content_settings.

This is follow up of issue 399592 for replacing NO_RESOURCE_IDENTIFER with
ResourceIdentifier, With these changes their will be no std::String() in content_settings.
so usage is more consistent.

BUG=475862
Committed: https://crrev.com/bc8118fa3784612fcd8c2ec49ecce355b203f25d
Cr-Commit-Position: refs/heads/master@{#324619}

[Deepak, 2015-04-10 10:31:24]

deepak.m1@samsung.com changed reviewers:
+ bauerb@chromium.org

[Deepak, 2015-04-10 10:31:25]

PTAL at small change for content_setting clean up for consistency for usage of
ResourceIdentifier() instead of std::String() in content_settings code.
It is follow up for r399592.
Thanks

[Bernhard Bauer, 2015-04-10 10:47:31]

bauerb@chromium.org changed reviewers:
+ vabr@chromium.org

[Bernhard Bauer, 2015-04-10 10:47:32]

TBH, I'm not sure if these changes are really worth it, because nothing stops
new code from using std::string. I'll defer to Vaclav though.

[Deepak, 2015-04-10 10:52:10]

On 2015/04/10 10:47:32, Bernhard Bauer wrote:
> TBH, I'm not sure if these changes are really worth it, because nothing stops
> new code from using std::string. I'll defer to Vaclav though.

yes, You are right, For consistency I have replace existing std::String() to
ResourceIdentifier().

[vabr (Chromium), 2015-04-10 10:57:17]

On 2015/04/10 10:47:32, Bernhard Bauer wrote:
> TBH, I'm not sure if these changes are really worth it, because nothing stops
> new code from using std::string. I'll defer to Vaclav though.

I am also not sure. My original motivation for the whole refactor disappeared
when it stopped blocking the componentisation (cf. http://crbug.com/399592#c8).
As far as I'm concerned, the current status is OK, although I have nothing
against this CL, it seems to improve things.

To stop new code from using std::string(), one would need to encapsulate
std::string in ResourceIdentifier, instead of making it a typedef.

Whether that is worth it should be decided by content_setting OWNERS (I'm not
one).

Cheers,
Vaclav

[Bernhard Bauer, 2015-04-10 11:12:58]

On 2015/04/10 10:57:17, vabr (Chromium) wrote:
> On 2015/04/10 10:47:32, Bernhard Bauer wrote:
> > TBH, I'm not sure if these changes are really worth it, because nothing
stops
> > new code from using std::string. I'll defer to Vaclav though.
> 
> I am also not sure. My original motivation for the whole refactor disappeared
> when it stopped blocking the componentisation (cf.
http://crbug.com/399592#c8).
> As far as I'm concerned, the current status is OK, although I have nothing
> against this CL, it seems to improve things.
> 
> To stop new code from using std::string(), one would need to encapsulate
> std::string in ResourceIdentifier, instead of making it a typedef.

Exactly, that seems like even more effort.

Y'know, I'm coming to think that using ResourceIdentifier() instead of
std::string() is nothing more than a readability improvement, in the same way
that you're supposed to use variables when passing flags to a method (i.e.

  bool awesome = true;
  DoStuff(awesome);

or 

  DoStuff(/* awesome */ true);

instead of passing the flag undocumented. That's really just a style issue
though, so it needs to be enforced by reviews (just like many other things).

Given that, this CL LGTM.

> Whether that is worth it should be decided by content_setting OWNERS (I'm not
> one).

Feel free to add yourself, in particular if you are planning on componentizing
content settings :)

> Cheers,
> Vaclav

[Deepak, 2015-04-10 11:39:45]

On 2015/04/10 11:12:58, Bernhard Bauer wrote:
> On 2015/04/10 10:57:17, vabr (Chromium) wrote:
> > On 2015/04/10 10:47:32, Bernhard Bauer wrote:
> > > TBH, I'm not sure if these changes are really worth it, because nothing
> stops
> > > new code from using std::string. I'll defer to Vaclav though.
> > 
> > I am also not sure. My original motivation for the whole refactor
disappeared
> > when it stopped blocking the componentisation (cf.
> http://crbug.com/399592#c8).
> > As far as I'm concerned, the current status is OK, although I have nothing
> > against this CL, it seems to improve things.
> > 
> > To stop new code from using std::string(), one would need to encapsulate
> > std::string in ResourceIdentifier, instead of making it a typedef.
> 
> Exactly, that seems like even more effort.
> 
> Y'know, I'm coming to think that using ResourceIdentifier() instead of
> std::string() is nothing more than a readability improvement, in the same way
> that you're supposed to use variables when passing flags to a method (i.e.
> 
>   bool awesome = true;
>   DoStuff(awesome);
> 
> or 
> 
>   DoStuff(/* awesome */ true);
> 
> instead of passing the flag undocumented. That's really just a style issue
> though, so it needs to be enforced by reviews (just like many other things).
> 
> Given that, this CL LGTM.
> 
> > Whether that is worth it should be decided by content_setting OWNERS (I'm
not
> > one).
> 
> Feel free to add yourself, in particular if you are planning on componentizing
> content settings :)
> 
> > Cheers,
> > Vaclav

I am happy to work with @vabr, It will be good learning for me.
@vabr, please add me in this .
Thanks

[Deepak, 2015-04-10 11:39:50]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2015-04-10 11:40:22]

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

[commit-bot: I haz the power, 2015-04-10 12:15:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-10 12:15:57]

Patchset 2 (id:??) landed as
https://crrev.com/bc8118fa3784612fcd8c2ec49ecce355b203f25d
Cr-Commit-Position: refs/heads/master@{#324619}

[vabr (Chromium), 2015-04-10 15:46:12]

On 2015/04/10 11:39:45, Deepak wrote:
> On 2015/04/10 11:12:58, Bernhard Bauer wrote:
> > On 2015/04/10 10:57:17, vabr (Chromium) wrote:
> > > On 2015/04/10 10:47:32, Bernhard Bauer wrote:
> > > > TBH, I'm not sure if these changes are really worth it, because nothing
> > stops
> > > > new code from using std::string. I'll defer to Vaclav though.
> > > 
> > > I am also not sure. My original motivation for the whole refactor
> disappeared
> > > when it stopped blocking the componentisation (cf.
> > http://crbug.com/399592#c8).
> > > As far as I'm concerned, the current status is OK, although I have nothing
> > > against this CL, it seems to improve things.
> > > 
> > > To stop new code from using std::string(), one would need to encapsulate
> > > std::string in ResourceIdentifier, instead of making it a typedef.
> > 
> > Exactly, that seems like even more effort.
> > 
> > Y'know, I'm coming to think that using ResourceIdentifier() instead of
> > std::string() is nothing more than a readability improvement, in the same
way
> > that you're supposed to use variables when passing flags to a method (i.e.
> > 
> >   bool awesome = true;
> >   DoStuff(awesome);
> > 
> > or 
> > 
> >   DoStuff(/* awesome */ true);
> > 
> > instead of passing the flag undocumented. That's really just a style issue
> > though, so it needs to be enforced by reviews (just like many other things).
> > 
> > Given that, this CL LGTM.
> > 
> > > Whether that is worth it should be decided by content_setting OWNERS (I'm
> not
> > > one).
> > 
> > Feel free to add yourself, in particular if you are planning on
componentizing
> > content settings :)
Thanks, Bernhard, I'll consider it. :)

> > 
> > > Cheers,
> > > Vaclav
> 
> I am happy to work with @vabr, It will be good learning for me.
> @vabr, please add me in this .
> Thanks
Thanks, Deepak, for your enthusiasm. :)
I'm afraid there might be a little misunderstanding about who Bernhard suggested
to be added where, but nevertheless you are free to join componentisation.
The content settings componentisation (bugs labelled
Hotlist-ContentSettings-Component) is currently in a state where couple of bugs
are taken which block the rest. Once we get through with them, there will be
some more parts free for taking, in particular componentising tests and some
final tasks.

There are more componentisation efforts going on, you can see the root bugs for
those under the label Hotlist-C14n.

If you don't know what componentisation is, I invite you to read
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/7QIDpvmDu_M/KaSJa....

Thanks!
Vaclav