Used oneof in blimp_message.proto

blimp/engine/session/blimp_engine_session.cc

 // Copyright 2015 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 "blimp/engine/session/blimp_engine_session.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
@@ skipping 275 matching lines @@
 void BlimpEngineSession::RegisterFeatures() {
   thread_pipe_manager_.reset(new ThreadPipeManager(
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::IO),
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::UI),
       net_components_->GetBrowserConnectionHandler()));
 
   // Register features' message senders and receivers.
   tab_control_message_sender_ =
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::TAB_CONTROL, this);
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kTabControl, this);
   navigation_message_sender_ =
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::NAVIGATION, this);
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kNavigation, this);
   render_widget_feature_.set_render_widget_message_sender(
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::RENDER_WIDGET,
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kRenderWidget,
                                             &render_widget_feature_));
   render_widget_feature_.set_input_message_sender(
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::INPUT,
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kInput,
                                             &render_widget_feature_));
   render_widget_feature_.set_compositor_message_sender(
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::COMPOSITOR,
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kCompositor,
                                             &render_widget_feature_));
   render_widget_feature_.set_ime_message_sender(
-      thread_pipe_manager_->RegisterFeature(BlimpMessage::IME,
+      thread_pipe_manager_->RegisterFeature(BlimpMessage::kIme,
                                             &render_widget_feature_));
 
   // The Settings feature does not need an outgoing message processor, since we
   // don't send any messages to the client right now.
-  thread_pipe_manager_->RegisterFeature(BlimpMessage::SETTINGS,
+  thread_pipe_manager_->RegisterFeature(BlimpMessage::kSettings,
                                         &settings_feature_);
 }
 
 bool BlimpEngineSession::CreateWebContents(const int target_tab_id) {
   DVLOG(1) << "Create tab " << target_tab_id;
   // TODO(haibinlu): Support more than one active WebContents (crbug/547231).
   if (web_contents_) {
     DLOG(WARNING) << "Tab " << target_tab_id << " already existed";
     return false;
   }
@@ skipping 122 matching lines @@
       web_contents_->GetRenderWidgetHostView()->GetRenderWidgetHost(),
       window_tree_host_->GetInputMethod()->GetTextInputClient());
 }
 
 void BlimpEngineSession::ProcessMessage(
     std::unique_ptr<BlimpMessage> message,
     const net::CompletionCallback& callback) {
   TRACE_EVENT1("blimp", "BlimpEngineSession::ProcessMessage", "TabId",
                message->target_tab_id());
   DCHECK(!callback.is_null());
-  DCHECK(message->type() == BlimpMessage::TAB_CONTROL ||
-         message->type() == BlimpMessage::NAVIGATION);
+  DCHECK(BlimpMessage::kTabControl == message->feature_case() ||
+         BlimpMessage::kNavigation == message->feature_case());
 
   net::Error result = net::OK;
-  if (message->type() == BlimpMessage::TAB_CONTROL) {
+  if (message->has_tab_control()) {

[Kevin M, 2016/05/20 18:28:15]

Check the feature case

[shaktisahu, 2016/05/20 22:29:41]

Why? We are already doing that in the DCHECK above. And it is much cleaner.

[Kevin M, 2016/05/21 00:32:05]

Consistency in how we are checking the type. In some places, we check
feature_case(), and in others, we check has_x(). I don't feel strongly about
this point, however.

     switch (message->tab_control().type()) {
       case TabControlMessage::CREATE_TAB:
         if (!CreateWebContents(message->target_tab_id()))
           result = net::ERR_FAILED;
         break;
       case TabControlMessage::CLOSE_TAB:
         CloseWebContents(message->target_tab_id());
       case TabControlMessage::SIZE:
         HandleResize(message->tab_control().size().device_pixel_ratio(),
                      gfx::Size(message->tab_control().size().width(),
                                message->tab_control().size().height()));
         break;
       default:
         NOTIMPLEMENTED();
         result = net::ERR_NOT_IMPLEMENTED;
     }
-  } else if (message->type() == BlimpMessage::NAVIGATION && web_contents_) {
+  } else if (message->has_navigation() && web_contents_) {

[Kevin M, 2016/05/20 18:28:15]

Check the feature case

[shaktisahu, 2016/05/20 22:29:41]

Ditto

     switch (message->navigation().type()) {
       case NavigationMessage::LOAD_URL:
         LoadUrl(message->target_tab_id(),
                 GURL(message->navigation().load_url().url()));
         break;
       case NavigationMessage::GO_BACK:
         GoBack(message->target_tab_id());
         break;
       case NavigationMessage::GO_FORWARD:
         GoForward(message->target_tab_id());
@@ skipping 159 matching lines @@
 
   aura::Window* parent = window_tree_host_->window();
   aura::Window* content = web_contents_->GetNativeView();
   if (!parent->Contains(content))
     parent->AddChild(content);
   content->Show();
 }
 
 }  // namespace engine
 }  // namespace blimp