Added Client-Hints.

chrome/browser/net/client_hints.cc

Index: chrome/browser/net/client_hints.cc
diff --git a/chrome/browser/net/client_hints.cc b/chrome/browser/net/client_hints.cc
new file mode 100644
index 0000000000000000000000000000000000000000..09d97edc8e7fe58f5c97348d7ad38481e51d11b3
--- /dev/null
+++ b/chrome/browser/net/client_hints.cc
@@ -0,0 +1,82 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/net/client_hints.h"

[Ryan Sleevi, 2013/01/16 23:19:56]

style: This should be first, followed by a new line -
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Stream...

[bengr (incorrect), 2013/01/17 01:50:44]

Done.

+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_view.h"
+#include "third_party/WebKit/Source/Platform/chromium/public/WebScreenInfo.h"

[Ryan Sleevi, 2013/01/16 23:19:56]

I didn't think this was legal? I thought only the webkit glue was allowed to
pull in these headers.

[bengr (incorrect), 2013/01/17 01:50:44]

Done.

+
+using content::BrowserThread;
+using content::RenderViewHost;
+using content::RenderWidgetHostViewPort;
+
+scoped_refptr<ClientHints> ClientHints::create() {
+    return make_scoped_refptr(new ClientHints());
+}
+
+ClientHints::ClientHints() {
+}
+
+ClientHints::~ClientHints() {
+}
+
+bool ClientHints::IsScreenInfoAvailable(int process_id, int render_view_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // TODO(bengr): Decide if we should do this thread hopping on every request.
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+      base::Bind(&ClientHints::FetchScreenInfoOnUIThread, this,
+                 process_id, render_view_id));
+  ScreenMap::iterator it =
+      screen_hints_.find(std::pair<int, int>(process_id, render_view_id));
+  if (it != screen_hints_.end()) {

[Ryan Sleevi, 2013/01/16 23:19:56]

style: Put it as error handling first

if (it == screen_hints_.end())
  return false;

return true;

[bengr (incorrect), 2013/01/17 01:50:44]

Done.

+    return true;
+  }
+  return false;
+}
+
+std::string ClientHints::GetScreenInfoHints(int process_id,
+                                            int render_view_id) {
+  DCHECK(screen_hints_.find(std::pair<int, int>(process_id, render_view_id)) !=
+      screen_hints_.end());
+  return screen_hints_[
+      std::pair<int, int>(process_id, render_view_id)].hints_str;

[Ryan Sleevi, 2013/01/16 23:19:56]

You're forcing a double-find for the DCHECK. Further, this seems like there can
be a synchronization issue (hence your DCHECK), and I'd much rather see true
safety here. If a renderer/process gets a spurious message sent and then kills
itself, this could corrupt browser process, right?

[bengr (incorrect), 2013/01/17 01:50:44]

Removed double-find. I don't understand your second point, though.

On 2013/01/16 23:19:56, Ryan Sleevi wrote:

+}
+
+void ClientHints::FetchScreenInfoOnUIThread(int process_id,
+                                            int render_view_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  RenderViewHost* render_view_host =
+      RenderViewHost::FromID(process_id, render_view_id);
+  if (render_view_host != NULL && render_view_host->GetView() != NULL) {

[Ryan Sleevi, 2013/01/16 23:19:56]

style: Put error handling first, with early return

if (render_view_host == NULL || render_view_host->GetView() == NULL)
  return;

WebKit::WebScreenInfo ...

[bengr (incorrect), 2013/01/17 01:50:44]

Done.

+    WebKit::WebScreenInfo screen_info;
+#if defined(OS_POSIX) || defined(USE_AURA)
+    render_view_host->GetView()->GetScreenInfo(&screen_info);
+#else
+    screen_info.deviceScaleFactor = 0.0;
+#endif
+    gfx::Rect rect = render_view_host->GetView()->GetViewBounds();
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+      base::Bind(&ClientHints::UpdateScreenInfo, this,
+                 process_id, render_view_id, rect.width(), rect.height(),
+                 screen_info.deviceScaleFactor));
+  }
+}
+
+void ClientHints::UpdateScreenInfo(int process_id, int render_view_id,
+                                   int width, int height,
+                                   float dpr) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  std::ostringstream value;

[Ryan Sleevi, 2013/01/16 23:19:56]

NACK: This is forbidden in the style guide. 

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Stream...

use base::StringPrintF() and call it a day.

[bengr (incorrect), 2013/01/17 01:50:44]

Done.

+  value << "vh=" << height << ", vw=" << width;
+#if defined(OS_POSIX) || defined(USE_AURA)
+  value << ", dpr=" << dpr;
+#endif
+  screen_hints_[std::pair<int, int>(process_id, render_view_id)] =
+      ScreenHints(width, height, dpr, value.str());
+}
+
+