WebUI: DisallowJavascript only on Refresh and non-same-page navigations

content/browser/webui/web_ui_impl.cc

 // 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 "content/browser/webui/web_ui_impl.h"
 
 #include <stddef.h>
 
 #include "base/debug/dump_without_crashing.h"
 #include "base/json/json_writer.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/renderer_host/dip_util.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/browser/web_contents/web_contents_view.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_ui_controller.h"

[Charlie Reis, 2016/06/24 23:15:00]

IWYU nit: web_contents_observer.h

[tommycli, 2016/06/24 23:55:36]

Done.

 #include "content/public/browser/web_ui_message_handler.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_client.h"
 
 namespace content {
 
+class WebUIImpl::MainFrameNavigationObserver : public WebContentsObserver {
+ public:
+  MainFrameNavigationObserver(WebUIImpl* parent, WebContents* contents)
+      : WebContentsObserver(contents), parent_(parent) {}
+  ~MainFrameNavigationObserver() override {}
+
+ private:
+  void DidFinishNavigation(NavigationHandle* navigation_handle) override {
+    if (!navigation_handle->IsInMainFrame() ||

[Charlie Reis, 2016/06/24 23:15:00]

// Only disallow Javascript on cross-document navigations in the main frame.

[tommycli, 2016/06/24 23:55:36]

Done.

+        !navigation_handle->HasCommitted() || navigation_handle->IsSamePage()) {
+      return;
+    }
+
+    parent_->DisallowJavascriptOnAllHandlers();
+  }
+
+  WebUIImpl* parent_;

[Charlie Reis, 2016/06/24 23:15:00]

web_ui_, perhaps?  (Parent is a confusing term, since it might refer to frames
or other hierarchical WebContents things like <webview>.)

[tommycli, 2016/06/24 23:55:36]

Done.

+};
+
 const WebUI::TypeID WebUI::kNoWebUI = NULL;
 
 // static
 base::string16 WebUI::GetJavascriptCall(
     const std::string& function_name,
     const std::vector<const base::Value*>& arg_list) {
   base::string16 parameters;
   std::string json;
   for (size_t i = 0; i < arg_list.size(); ++i) {
     if (i > 0)
       parameters += base::char16(',');
 
     base::JSONWriter::Write(*arg_list[i], &json);
     parameters += base::UTF8ToUTF16(json);
   }
   return base::ASCIIToUTF16(function_name) +
       base::char16('(') + parameters + base::char16(')') + base::char16(';');
 }
 
 WebUIImpl::WebUIImpl(WebContents* contents, const std::string& frame_name)
     : link_transition_type_(ui::PAGE_TRANSITION_LINK),
       bindings_(BINDINGS_POLICY_WEB_UI),
       web_contents_(contents),
+      web_contents_observer_(new MainFrameNavigationObserver(this, contents)),
       frame_name_(frame_name) {
   DCHECK(contents);
 }
 
 WebUIImpl::~WebUIImpl() {
   // Delete the controller first, since it may also be keeping a pointer to some
   // of the handlers and can call them at destruction.
   controller_.reset();
 }
 
@@ skipping 23 matching lines @@
 }
 
 void WebUIImpl::RenderViewCreated(RenderViewHost* render_view_host) {
   controller_->RenderViewCreated(render_view_host);
 }
 
 void WebUIImpl::RenderViewReused(RenderViewHost* render_view_host,
                                  bool was_main_frame) {
   if (was_main_frame) {
     GURL site_url = render_view_host->GetSiteInstance()->GetSiteURL();
     GetContentClient()->browser()->LogWebUIUrl(site_url);

[Charlie Reis, 2016/06/24 23:15:01]

Ooh, I wonder if we can get rid of RenderViewReused entirely by doing this
logging in DidFinishNavigation...  That would be really nice.

We probably shouldn't do it in this CL, though, to avoid the risk of affecting
logs and possibly needing to revert.  Does it sound worth trying in a followup
CL?

[tommycli, 2016/06/24 23:55:36]

Yeah I think that would be eminently doable. But yes, like you said, in a
separate CL.

   }
-
-  for (WebUIMessageHandler* handler : handlers_)
-    handler->RenderViewReused();
 }
 
 void WebUIImpl::RenderFrameHostSwappingOut() {
-  for (WebUIMessageHandler* handler : handlers_)
-    handler->DisallowJavascript();
+  DisallowJavascriptOnAllHandlers();
 }
 
 WebContents* WebUIImpl::GetWebContents() const {
   return web_contents_;
 }
 
 float WebUIImpl::GetDeviceScaleFactor() const {
   return GetScaleFactorForView(web_contents_->GetRenderWidgetHostView());
 }
 
@@ skipping 161 matching lines @@
   return *frame_set.begin();
 }
 
 void WebUIImpl::AddToSetIfFrameNameMatches(
     std::set<RenderFrameHost*>* frame_set,
     RenderFrameHost* host) {
   if (host->GetFrameName() == frame_name_)
     frame_set->insert(host);
 }
 
+void WebUIImpl::DisallowJavascriptOnAllHandlers() {
+  for (WebUIMessageHandler* handler : handlers_)
+    handler->DisallowJavascript();
+}
+
 }  // namespace content