Commit b817eec5 authored by dzhioev's avatar dzhioev Committed by Commit bot

Added 'contextReady' message for HostPairingScreen.

Sometimes the screen starts to send updates to JS part to early, when screen
is not registered yet. As a result several messages with context diffs could
be ignored, and JS context became inconsistent with C++ context. I added
'contextReady' message, that is send from JS when context is ready to recieve
updates. Before that the screen handler caches changes in the local context.

BUG=375191

Review URL: https://codereview.chromium.org/581593003

Cr-Commit-Position: refs/heads/master@{#295944}
parent 0599b145
...@@ -84,6 +84,9 @@ class ScreenContext : public base::NonThreadSafe { ...@@ -84,6 +84,9 @@ class ScreenContext : public base::NonThreadSafe {
void ApplyChanges(const base::DictionaryValue& diff, void ApplyChanges(const base::DictionaryValue& diff,
std::vector<std::string>* keys); std::vector<std::string>* keys);
// Returns underlying dictionary containing all the stored data.
const base::DictionaryValue& storage() const { return storage_; }
private: private:
bool Set(const KeyType& key, base::Value* value); bool Set(const KeyType& key, base::Value* value);
......
...@@ -25,6 +25,9 @@ login.createScreen('HostPairingScreen', 'host-pairing', function() { ...@@ -25,6 +25,9 @@ login.createScreen('HostPairingScreen', 'host-pairing', function() {
/** @const */ var PAGE_ENROLLMENT_ERROR = 'enrollment-error'; /** @const */ var PAGE_ENROLLMENT_ERROR = 'enrollment-error';
/** @const */ var PAGE_PAIRING_DONE = 'pairing-done'; /** @const */ var PAGE_PAIRING_DONE = 'pairing-done';
/** @const */ var CALLBACK_CONTEXT_READY = 'contextReady';
/** @const */ var PAGE_NAMES = [ /** @const */ var PAGE_NAMES = [
PAGE_WELCOME, PAGE_WELCOME,
PAGE_CODE_CONFIRMATION, PAGE_CODE_CONFIRMATION,
...@@ -51,6 +54,7 @@ login.createScreen('HostPairingScreen', 'host-pairing', function() { ...@@ -51,6 +54,7 @@ login.createScreen('HostPairingScreen', 'host-pairing', function() {
}, this); }, this);
this.addContextObserver(CONTEXT_KEY_PAGE, this.pageChanged_); this.addContextObserver(CONTEXT_KEY_PAGE, this.pageChanged_);
this.send(CALLBACK_CONTEXT_READY);
}, },
pageChanged_: function(newPage, oldPage) { pageChanged_: function(newPage, oldPage) {
......
...@@ -14,10 +14,18 @@ const char kJsScreenPath[] = "login.HostPairingScreen"; ...@@ -14,10 +14,18 @@ const char kJsScreenPath[] = "login.HostPairingScreen";
const char kMethodContextChanged[] = "contextChanged"; const char kMethodContextChanged[] = "contextChanged";
// Sent from JS when screen is ready to receive context updates.
// TODO(dzhioev): Move 'contextReady' logic to the base screen handler when
// all screens migrate to context-based communications.
const char kCallbackContextReady[] = "contextReady";
} // namespace } // namespace
HostPairingScreenHandler::HostPairingScreenHandler() HostPairingScreenHandler::HostPairingScreenHandler()
: BaseScreenHandler(kJsScreenPath), delegate_(NULL), show_on_init_(false) { : BaseScreenHandler(kJsScreenPath),
delegate_(NULL),
show_on_init_(false),
js_context_ready_(false) {
} }
HostPairingScreenHandler::~HostPairingScreenHandler() { HostPairingScreenHandler::~HostPairingScreenHandler() {
...@@ -25,6 +33,11 @@ HostPairingScreenHandler::~HostPairingScreenHandler() { ...@@ -25,6 +33,11 @@ HostPairingScreenHandler::~HostPairingScreenHandler() {
delegate_->OnActorDestroyed(this); delegate_->OnActorDestroyed(this);
} }
void HostPairingScreenHandler::HandleContextReady() {
js_context_ready_ = true;
OnContextChanged(context_cache_.storage());
}
void HostPairingScreenHandler::Initialize() { void HostPairingScreenHandler::Initialize() {
if (!page_is_ready() || !delegate_) if (!page_is_ready() || !delegate_)
return; return;
...@@ -40,6 +53,8 @@ void HostPairingScreenHandler::DeclareLocalizedValues( ...@@ -40,6 +53,8 @@ void HostPairingScreenHandler::DeclareLocalizedValues(
} }
void HostPairingScreenHandler::RegisterMessages() { void HostPairingScreenHandler::RegisterMessages() {
AddPrefixedCallback(kCallbackContextReady,
&HostPairingScreenHandler::HandleContextReady);
} }
void HostPairingScreenHandler::Show() { void HostPairingScreenHandler::Show() {
...@@ -61,6 +76,10 @@ void HostPairingScreenHandler::SetDelegate(Delegate* delegate) { ...@@ -61,6 +76,10 @@ void HostPairingScreenHandler::SetDelegate(Delegate* delegate) {
void HostPairingScreenHandler::OnContextChanged( void HostPairingScreenHandler::OnContextChanged(
const base::DictionaryValue& diff) { const base::DictionaryValue& diff) {
if (!js_context_ready_) {
context_cache_.ApplyChanges(diff, NULL);
return;
}
CallJS(kMethodContextChanged, diff); CallJS(kMethodContextChanged, diff);
} }
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/chromeos/login/screens/host_pairing_screen_actor.h" #include "chrome/browser/chromeos/login/screens/host_pairing_screen_actor.h"
#include "chrome/browser/chromeos/login/screens/screen_context.h"
#include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h" #include "chrome/browser/ui/webui/chromeos/login/base_screen_handler.h"
namespace chromeos { namespace chromeos {
...@@ -18,6 +19,8 @@ class HostPairingScreenHandler : public HostPairingScreenActor, ...@@ -18,6 +19,8 @@ class HostPairingScreenHandler : public HostPairingScreenActor,
virtual ~HostPairingScreenHandler(); virtual ~HostPairingScreenHandler();
private: private:
void HandleContextReady();
// Overridden from BaseScreenHandler: // Overridden from BaseScreenHandler:
virtual void Initialize() OVERRIDE; virtual void Initialize() OVERRIDE;
virtual void DeclareLocalizedValues(LocalizedValuesBuilder* builder) OVERRIDE; virtual void DeclareLocalizedValues(LocalizedValuesBuilder* builder) OVERRIDE;
...@@ -33,6 +36,10 @@ class HostPairingScreenHandler : public HostPairingScreenActor, ...@@ -33,6 +36,10 @@ class HostPairingScreenHandler : public HostPairingScreenActor,
HostPairingScreenActor::Delegate* delegate_; HostPairingScreenActor::Delegate* delegate_;
bool show_on_init_; bool show_on_init_;
bool js_context_ready_;
// Caches context changes while JS part is not ready to receive messages.
ScreenContext context_cache_;
DISALLOW_COPY_AND_ASSIGN(HostPairingScreenHandler); DISALLOW_COPY_AND_ASSIGN(HostPairingScreenHandler);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment