exo: Don't assume that application and startup IDs are non-null.

Use base::Optional for startup and application ID in order to
handle null values properly. Setting one of these to null will
now clear the window property.

Bug: 834971
Test: exo_unittests --gtest_filter=ShellSurfaceTest.Set*Id
Change-Id: I395fb02200f310b027060eb28e0132d8976532ab
Reviewed-on: https://chromium-review.googlesource.com/1020042Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552178}

components/exo/shell_surface_base.cc

@@ -516,10 +516,14 @@ void ShellSurfaceBase::UpdateSystemModal() {
 
 // static
 void ShellSurfaceBase::SetApplicationId(aura::Window* window,
-                                        const std::string& id) {
+                                        const base::Optional<std::string>& id) {
   TRACE_EVENT1("exo", "ShellSurfaceBase::SetApplicationId", "application_id",
-               id);
-  window->SetProperty(kApplicationIdKey, new std::string(id));
+               id ? *id : "null");
+
+  if (id)
+    window->SetProperty(kApplicationIdKey, new std::string(*id));
+  else
+    window->ClearProperty(kApplicationIdKey);
 }
 
 // static
@@ -528,18 +532,27 @@ const std::string* ShellSurfaceBase::GetApplicationId(
   return window->GetProperty(kApplicationIdKey);
 }
 
-void ShellSurfaceBase::SetApplicationId(const std::string& application_id) {
+void ShellSurfaceBase::SetApplicationId(const char* application_id) {
   // Store the value in |application_id_| in case the window does not exist yet.
-  application_id_ = application_id;
+  if (application_id)
+    application_id_ = std::string(application_id);
+  else
+    application_id_.reset();
+
   if (widget_ && widget_->GetNativeWindow())
-    SetApplicationId(widget_->GetNativeWindow(), application_id);
+    SetApplicationId(widget_->GetNativeWindow(), application_id_);
 }
 
 // static
 void ShellSurfaceBase::SetStartupId(aura::Window* window,
-                                    const std::string& id) {
-  TRACE_EVENT1("exo", "ShellSurfaceBase::SetStartupId", "startup_id", id);
-  window->SetProperty(kStartupIdKey, new std::string(id));
+                                    const base::Optional<std::string>& id) {
+  TRACE_EVENT1("exo", "ShellSurfaceBase::SetStartupId", "startup_id",
+               id ? *id : "null");
+
+  if (id)
+    window->SetProperty(kStartupIdKey, new std::string(*id));
+  else
+    window->ClearProperty(kStartupIdKey);
 }
 
 // static
@@ -549,9 +562,13 @@ const std::string* ShellSurfaceBase::GetStartupId(aura::Window* window) {
 
 void ShellSurfaceBase::SetStartupId(const char* startup_id) {
   // Store the value in |startup_id_| in case the window does not exist yet.
-  startup_id_ = std::string(startup_id);
+  if (startup_id)
+    startup_id_ = std::string(startup_id);
+  else
+    startup_id_.reset();
+
   if (widget_ && widget_->GetNativeWindow())
-    SetStartupId(widget_->GetNativeWindow(), startup_id);
+    SetStartupId(widget_->GetNativeWindow(), startup_id_);
 }
 
 void ShellSurfaceBase::Close() {

components/exo/shell_surface_base.h

@@ -117,15 +117,17 @@ class ShellSurfaceBase : public SurfaceTreeHost,
 
   // Sets the application ID for the window. The application ID identifies the
   // general class of applications to which the window belongs.
-  static void SetApplicationId(aura::Window* window, const std::string& id);
+  static void SetApplicationId(aura::Window* window,
+                               const base::Optional<std::string>& id);
   static const std::string* GetApplicationId(const aura::Window* window);
 
   // Set the application ID for the surface.
-  void SetApplicationId(const std::string& application_id);
+  void SetApplicationId(const char* application_id);
 
   // Sets the startup ID for the window. The startup ID identifies the
   // application using startup notification protocol.
-  static void SetStartupId(aura::Window* window, const std::string& id);
+  static void SetStartupId(aura::Window* window,
+                           const base::Optional<std::string>& id);
   static const std::string* GetStartupId(aura::Window* window);
 
   // Set the startup ID for the surface.
@@ -358,8 +360,8 @@ class ShellSurfaceBase : public SurfaceTreeHost,
   SkColor active_frame_color_ = SK_ColorBLACK;
   SkColor inactive_frame_color_ = SK_ColorBLACK;
   bool pending_show_widget_ = false;
-  std::string application_id_;
-  std::string startup_id_;
+  base::Optional<std::string> application_id_;
+  base::Optional<std::string> startup_id_;
   base::RepeatingClosure close_callback_;
   base::OnceClosure surface_destroyed_callback_;
   ScopedConfigure* scoped_configure_ = nullptr;

components/exo/shell_surface_unittest.cc

@@ -265,6 +265,9 @@ TEST_F(ShellSurfaceTest, SetApplicationId) {
   EXPECT_EQ("pre-widget-id", *ShellSurface::GetApplicationId(window));
   shell_surface->SetApplicationId("test");
   EXPECT_EQ("test", *ShellSurface::GetApplicationId(window));
+
+  shell_surface->SetApplicationId(nullptr);
+  EXPECT_EQ(nullptr, ShellSurface::GetApplicationId(window));
 }
 
 TEST_F(ShellSurfaceTest, SetStartupId) {
@@ -283,6 +286,9 @@ TEST_F(ShellSurfaceTest, SetStartupId) {
   EXPECT_EQ("pre-widget-id", *ShellSurface::GetStartupId(window));
   shell_surface->SetStartupId("test");
   EXPECT_EQ("test", *ShellSurface::GetStartupId(window));
+
+  shell_surface->SetStartupId(nullptr);
+  EXPECT_EQ(nullptr, ShellSurface::GetStartupId(window));
 }
 
 TEST_F(ShellSurfaceTest, Move) {