Simplify the user agent overriding code, in preparation for moving it out of src/webkit.

Simplify the user agent overriding code, in preparation for moving it out of src/webkit.

These workarounds were added a long time ago before Chrome launched. The Microsoft one isn't needed as the download page has a link to the Mac version without the spoof. The Yahoo Japan download page says it's going away in June and just redirects to Microsoft. The gyao.yahoo.co.jp host doesn't use Silverlight now. promotion.shopping.yahoo.co.jp is dead.

BUG=338338
R=jamesr@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255534

[tfarina, 2014-03-06 18:44:17]

Thanks John!

I'll rebase my CL on top of this after it lands.

[jam, 2014-03-06 18:56:34]

On 2014/03/06 18:44:17, tfarina wrote:
> Thanks John!
> 
> I'll rebase my CL on top of this after it lands.

sure, I was going to do a few changes to simplify the code. this is the start. i
can ping you when i'm done, then you can rebase and do the file move of the
remaining code. sg?

[tfarina, 2014-03-06 19:48:41]

On 2014/03/06 18:56:34, jam wrote:
> On 2014/03/06 18:44:17, tfarina wrote:
> > Thanks John!
> > 
> > I'll rebase my CL on top of this after it lands.
> 
> sure, I was going to do a few changes to simplify the code. this is the start.
i
> can ping you when i'm done, then you can rebase and do the file move of the
> remaining code. sg?

sgtm.

Do you think I can judge the correctness of those CLs and you copy Darin to
accelerate the process? (I'm trying to improve my review side). If not (yet),
that is OK though.

[tfarina, 2014-03-06 19:55:45]

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
File webkit/common/user_agent/user_agent.cc (right):

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:9: #include "base/strings/string_util.h"
i think you can remove this and stringprintf.h now.

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:12: #include
"webkit/common/user_agent/user_agent_util.h"
is this used?

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:29: mutable std::string
user_agent_for_spoofing_hack_;
remove?

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:59: const std::string&
UserAgentState::Get(const GURL& url) const {
this parameter is not used anymore. Remove?

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
File webkit/common/user_agent/user_agent.h (right):

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.h:10: #include "base/basictypes.h"
remove

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.h:11: #include "url/gurl.h"
forward declare

[jam, 2014-03-06 21:02:58]

On 2014/03/06 19:48:41, tfarina wrote:
> On 2014/03/06 18:56:34, jam wrote:
> > On 2014/03/06 18:44:17, tfarina wrote:
> > > Thanks John!
> > > 
> > > I'll rebase my CL on top of this after it lands.
> > 
> > sure, I was going to do a few changes to simplify the code. this is the
start.
> i
> > can ping you when i'm done, then you can rebase and do the file move of the
> > remaining code. sg?
> 
> sgtm.
> 
> Do you think I can judge the correctness of those CLs and you copy Darin to
> accelerate the process? (I'm trying to improve my review side). If not (yet),
> that is OK though.
sure you can review the changes in content.

i need darin to review webkit as i'm not an owner there. particularly for this
cl, i just wanted to run it by someone who remembers why it was originally
added, to make sure that others agree that it's ok to remove these workarounds

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
File webkit/common/user_agent/user_agent.cc (right):

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:9: #include "base/strings/string_util.h"
On 2014/03/06 19:55:45, tfarina wrote:
> i think you can remove this and stringprintf.h now.

Done.

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:12: #include
"webkit/common/user_agent/user_agent_util.h"
On 2014/03/06 19:55:45, tfarina wrote:
> is this used?

Done.

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:29: mutable std::string
user_agent_for_spoofing_hack_;
On 2014/03/06 19:55:45, tfarina wrote:
> remove?

Done.

https://codereview.chromium.org/178263007/diff/40001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.cc:59: const std::string&
UserAgentState::Get(const GURL& url) const {
On 2014/03/06 19:55:45, tfarina wrote:
> this parameter is not used anymore. Remove?

Done.

[jam, 2014-03-06 21:36:00]

jamesr: darin is gone, care to take a look? in particular i want sanity checking
of removing the workarounds.

[jamesr, 2014-03-06 21:57:57]

Ah, I was just wondering the other day if we could remove these workarounds.

lgtm - should be fine.

[jamesr, 2014-03-06 22:08:46]

Slightly OT but is the locking in this class really necessary? In the render
process set and get are always called from the same thread.  In the browser, I
suspect we might query the value from the I/O thread for the network stack in
some cases but if we establish a happens-before relationship between the Set and
the Get call and never try to Set() after initialization we shouldn't need a
mutex.

[jam, 2014-03-06 22:13:47]

On 2014/03/06 22:08:46, jamesr wrote:
> Slightly OT but is the locking in this class really necessary? In the render
> process set and get are always called from the same thread.  In the browser, I
> suspect we might query the value from the I/O thread for the network stack in
> some cases but if we establish a happens-before relationship between the Set
and
> the Get call and never try to Set() after initialization we shouldn't need a
> mutex.

it probably wasn't needed; in the next cl i'm removing all that code anyways but
just wanted to do it in small steps.

[tfarina, 2014-03-07 00:15:06]

forgot to upload a new patch set?

I don't see some changes.

https://codereview.chromium.org/178263007/diff/60001/webkit/common/user_agent...
File webkit/common/user_agent/user_agent.h (right):

https://codereview.chromium.org/178263007/diff/60001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.h:10: #include "base/basictypes.h"
remove?

https://codereview.chromium.org/178263007/diff/60001/webkit/common/user_agent...
webkit/common/user_agent/user_agent.h:11: #include "url/gurl.h"
forward declare?