Updates preference.onPrefChaged dispatch to split incognito profile

Fixes two issues with how incognito extension prefs changes are
dispatched when the extension is in split incognito mode:
 1. If an extension was incognito split, but not incognito enabled, the
    incognito prefs changes were dispatched to the the extension's
    original profile instance (even though extension in the original
    profile generally cannot access incognito prefs). This CL does not
    dispatch incognito pref changes to extensions that are not
    incognito enabled.
 2. If an incognito pref change was dispatched to an incognito split
    extension when OTR profile was not around, in order to restrict
    the preference to the OTR profile, DispatchEventToExtensions()
    function would call Profile::GetOffTheRecordProfile which would
    create an OTR profile instance. This cl avoids this issue by
    dropping  the events that are expected to be restricted to a
    non-existent OTR profiles
    (Note: if the pref value changed in the original profile, and there
     is no incognito specific pref value, the value change is reported
     for the incognito pref, too),

BUG=796814,805480

Change-Id: I093949c9f9123e3baaab56932903f4f2be55903c
Reviewed-on: https://chromium-review.googlesource.com/882508
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532128}

chrome/browser/extensions/api/preference/preference_apitest.cc

@@ -346,6 +346,54 @@ IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest, OnChangeSplit) {
   EXPECT_TRUE(catcher_incognito.GetNextResult()) << catcher.message();
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest,
+                       OnChangeSplitWithNoOTRProfile) {
+  PrefService* prefs = profile_->GetPrefs();
+  prefs->SetBoolean(prefs::kBlockThirdPartyCookies, true);
+
+  extensions::ResultCatcher catcher;
+  ExtensionTestMessageListener loaded_incognito_test_listener(
+      "incognito loaded", false);
+
+  ExtensionTestMessageListener change_pref_listener("change pref value", false);
+
+  ASSERT_TRUE(
+      LoadExtensionIncognito(test_data_dir_.AppendASCII("preference")
+                                 .AppendASCII("onchange_split_regular_only")));
+
+  ASSERT_TRUE(change_pref_listener.WaitUntilSatisfied());
+  prefs->SetBoolean(prefs::kBlockThirdPartyCookies, false);
+
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+  EXPECT_FALSE(loaded_incognito_test_listener.was_satisfied());
+  EXPECT_FALSE(profile_->HasOffTheRecordProfile());
+}
+
+IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest,
+                       OnChangeSplitWithoutIncognitoAccess) {
+  PrefService* prefs = profile_->GetPrefs();
+  prefs->SetBoolean(prefs::kBlockThirdPartyCookies, true);
+
+  // Open an incognito window.
+  OpenURLOffTheRecord(profile_, GURL("chrome://newtab/"));
+  EXPECT_TRUE(profile_->HasOffTheRecordProfile());
+
+  extensions::ResultCatcher catcher;
+  ExtensionTestMessageListener loaded_incognito_test_listener(
+      "incognito loaded", false);
+
+  ExtensionTestMessageListener change_pref_listener("change pref value", false);
+
+  ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("preference")
+                                .AppendASCII("onchange_split_regular_only")));
+
+  ASSERT_TRUE(change_pref_listener.WaitUntilSatisfied());
+  prefs->SetBoolean(prefs::kBlockThirdPartyCookies, false);
+
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+  EXPECT_FALSE(loaded_incognito_test_listener.was_satisfied());
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionPreferenceApiTest, DataReductionProxy) {
   EXPECT_TRUE(RunExtensionTest("preference/data_reduction_proxy")) <<
       message_;

chrome/browser/extensions/api/preference/preference_helpers.cc

@@ -99,8 +99,7 @@ void DispatchEventToExtensions(Profile* profile,
     // TODO(bauerb): Only iterate over registered event listeners.
     if (router->ExtensionHasEventListener(extension->id(), event_name) &&
         extension->permissions_data()->HasAPIPermission(permission) &&
-        (!incognito || IncognitoInfo::IsSplitMode(extension.get()) ||
-         util::CanCrossIncognito(extension.get(), profile))) {
+        (!incognito || util::IsIncognitoEnabled(extension->id(), profile))) {
       // Inject level of control key-value.
       base::DictionaryValue* dict;
       bool rv = args->GetDictionary(0, &dict);
@@ -116,10 +115,19 @@ void DispatchEventToExtensions(Profile* profile,
       Profile* restrict_to_profile = nullptr;
       bool from_incognito = false;
       if (IncognitoInfo::IsSplitMode(extension.get())) {
-        if (incognito && util::IsIncognitoEnabled(extension->id(), profile)) {
+        if (incognito) {
+          // If off the record profile does not exist, there should be no
+          // extensions running in incognito at this time, and consequentially
+          // no need to dispatch an event restricted to an incognito extension.
+          // Furthermore, avoid calling GetOffTheRecordProfile() in this case -
+          // this method creates off the record profile if one does not exist.
+          // Unnecessarily creating off the record profile is undesirable, and
+          // can lead to a crash if incognito is disallowed for the current
+          // profile (see https://crbug.com/796814).
+          if (!profile->HasOffTheRecordProfile())
+            continue;
           restrict_to_profile = profile->GetOffTheRecordProfile();
-        } else if (!incognito &&
-                   PreferenceAPI::Get(profile)->DoesExtensionControlPref(
+        } else if (!PreferenceAPI::Get(profile)->DoesExtensionControlPref(
                        extension->id(), browser_pref, &from_incognito) &&
                    from_incognito) {
           restrict_to_profile = profile;

chrome/test/data/extensions/api_test/preference/onchange_split_regular_only/manifest.json

+{
+  "name" : "Preferences API Test Extension (Events)",
+  "version" : "0.1",
+  "manifest_version": 2,
+  "description" : "Preferences API Test Extension (Events)",
+  "permissions": [ "privacy", "tabs" ],
+  "background": {
+    "scripts": [ "test.js" ],
+    "persitent": false
+  },
+  "incognito": "split"
+}

chrome/test/data/extensions/api_test/preference/onchange_split_regular_only/test.js

+// Copyright 2018 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.
+
+// Tests preference.onChange API for an incognito split extension in case the
+// extension's incognito instance is not expected to be brought up.
+
+var allowCookies = chrome.privacy.websites.thirdPartyCookiesAllowed;
+
+function PreferenceChangeListener() {
+  this.encounteredEvents = [];
+  this.doneCallback_ = null;
+  this.valueCallbacks_ = {};
+}
+
+PreferenceChangeListener.prototype.start = function(event) {
+  var listener = this.onPrefChanged_.bind(this);
+
+  event.addListener(listener);
+  this.doneCallback_ = function() {
+    event.removeListener(listener);
+  }
+};
+
+PreferenceChangeListener.prototype.stop = function(callback) {
+  if (this.doneCallback_) {
+    this.doneCallback_();
+    this.doneCallback_ = null;
+  }
+  chrome.test.assertEq([], this.encounteredEvents);
+  chrome.test.assertEq({}, this.valueCallbacks_);
+  callback();
+};
+
+PreferenceChangeListener.prototype.listenForValue = function(value, callback) {
+  this.valueCallbacks_[value] = this.valueCallbacks_[value] || [];
+  this.valueCallbacks_[value].push(callback);
+};
+
+PreferenceChangeListener.prototype.getAndClearEncounteredEvents = function() {
+  var events = this.encounteredEvents;
+  this.encounteredEvents = [];
+  return events;
+};
+
+PreferenceChangeListener.prototype.onPrefChanged_ = function(pref) {
+  this.encounteredEvents.push(pref);
+  var callbacks = this.valueCallbacks_[pref.value];
+  delete this.valueCallbacks_[pref.value];
+  if (callbacks)
+    callbacks.forEach(callback => callback());
+};
+
+var allowCookiesChangeListener = null;
+
+// The incognito background is not expected to be run - send a message to the
+// test runner, and bail out.
+if (chrome.extension.inIncognitoContext) {
+  chrome.test.sendMessage('incognito loaded');
+} else {
+  chrome.test.runTests([
+    function setupPreferenceListener() {
+      chrome.test.assertFalse(!!allowCookiesChangeListener);
+      allowCookiesChangeListener = new PreferenceChangeListener();
+      allowCookiesChangeListener.start(allowCookies.onChange);
+      chrome.test.succeed();
+    },
+
+    function getInitialValue() {
+      allowCookies.get({}, chrome.test.callbackPass(pref => {
+        chrome.test.assertEq(
+            {levelOfControl: 'controllable_by_this_extension', value: false},
+            pref);
+      }));
+    },
+
+    function listenForUserChange() {
+      allowCookiesChangeListener.listenForValue(
+          true, chrome.test.callbackPass(function() {
+            var events =
+                allowCookiesChangeListener.getAndClearEncounteredEvents();
+            chrome.test.assertEq(events, [
+              {levelOfControl: 'controllable_by_this_extension', value: true}
+            ]);
+          }));
+
+      chrome.test.sendMessage('change pref value', chrome.test.callbackPass());
+    },
+
+    function changeDefault() {
+      allowCookiesChangeListener.listenForValue(
+          false, chrome.test.callbackPass(function() {
+            var events =
+                allowCookiesChangeListener.getAndClearEncounteredEvents();
+            chrome.test.assertEq(events, [
+              {value: false, levelOfControl: 'controlled_by_this_extension'}
+            ]);
+          }));
+
+      allowCookies.set({value: false}, chrome.test.callbackPass());
+    },
+
+    function changeIncognitoOnly() {
+      allowCookies.set(
+          {value: true, scope: 'incognito_session_only'},
+          chrome.test.callbackFail(
+              'You do not have permission to access incognito preferences.'));
+    },
+
+    function clearControl() {
+      allowCookiesChangeListener.listenForValue(
+          true, chrome.test.callbackPass(function() {
+            var events =
+                allowCookiesChangeListener.getAndClearEncounteredEvents();
+            chrome.test.assertEq(events, [
+              {levelOfControl: 'controllable_by_this_extension', value: true}
+            ]);
+          }));
+
+      allowCookies.clear({}, chrome.test.callbackPass());
+    },
+
+    function stopPreferenceListener() {
+      var listener = allowCookiesChangeListener;
+      allowCookiesChangeListener = null;
+      listener.stop(chrome.test.callbackPass());
+    }
+  ]);
+}