Remove promotion-type-specific JSON handling from NotificationPromo

chrome/browser/web_resource/notification_promo_mobile_ntp.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 "chrome/browser/web_resource/notification_promo_mobile_ntp.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+#include "chrome/browser/web_resource/notification_promo.h"
+
+NotificationPromoMobileNtp::NotificationPromoMobileNtp(
+    const NotificationPromo& promo)
+    : valid_(false),
+      requires_mobile_only_sync_(false),
+      requires_sync_(false),
+      show_on_most_visited_(false),
+      show_on_open_tabs_(false),
+      show_as_virtual_computer_(false),
+      action_args_(NULL),
+      payload_(NULL) {
+  payload_ = promo.promo_payload();
+  if (!payload_)
+    return;
+  if (!payload_->GetString("promo_message_short", &text_))
+    return;

[achuithb, 2012/08/17 22:55:12]

Is this right? Are all the fields required? What happens if we bail early like
this with a partially constructed object?

[aruslan, 2012/08/20 15:36:53]

Currently all fields are required: if we bail out early, the valid_ field
wouldn't be set to true.
I can move valid_ = true to just after action_args_ (everything before is
clearly necessary) and change the defaults to match some common use case.
Would you mind having it in a separate CL -- I"m not sure what the defaults
should be :)
On 2012/08/17 22:55:12, achuith.bhandarkar wrote:

[achuithb, 2012/08/20 19:17:48]

Please file a bug, add a TODO with comments and a reference to the bug.

[aruslan, 2012/08/20 22:38:17]

I moved valid_ = true earlier and changed the defaults to match.
On 2012/08/20 19:17:48, achuith.bhandarkar wrote:

+  if (!payload_->GetString("promo_message_long", &text_long_))
+    return;
+  if (!payload_->GetString("promo_action_type", &action_type_))
+    return;
+  if (!payload_->GetList("promo_action_args", &action_args_))
+    return;
+  if (!action_args_)
+    return;
+  payload_->GetBoolean("promo_requires_mobile_only_sync",
+      &requires_mobile_only_sync_);
+  payload_->GetBoolean("promo_requires_sync", &requires_sync_);
+  payload_->GetBoolean("promo_show_on_most_visited", &show_on_most_visited_);
+  payload_->GetBoolean("promo_show_on_open_tabs", &show_on_open_tabs_);
+  payload_->GetBoolean("promo_show_as_virtual_computer",
+      &show_as_virtual_computer_);
+  payload_->GetString("promo_virtual_computer_title", &virtual_computer_title_);
+  payload_->GetString("promo_virtual_computer_lastsync",
+      &virtual_computer_lastsync_);
+  valid_ = true;
+}
+
+NotificationPromoMobileNtp::~NotificationPromoMobileNtp() {}
+