From cb3a9c69ab395fdc8af98890819104ff612264d4 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 5 Dec 2017 15:59:15 +0900 Subject: [PATCH] Add a SessionPreferences to manage session related data By design the BrowserClient should not be aware of the api:: classes. --- atom/browser/api/atom_api_session.cc | 23 ++++---- atom/browser/api/atom_api_session.h | 4 +- atom/browser/atom_browser_client.cc | 6 +- atom/browser/session_preferences.cc | 61 ++++++++++++++++++++ atom/browser/session_preferences.h | 46 +++++++++++++++ atom/browser/web_contents_preferences.cc | 16 ----- atom/browser/web_contents_preferences.h | 6 +- atom/common/options_switches.cc | 24 ++++---- atom/common/options_switches.h | 2 +- docs/api/session.md | 15 ++--- filenames.gypi | 2 + lib/renderer/init.js | 24 ++++---- spec/api-browser-window-spec.js | 44 +++++++------- spec/fixtures/api/preloads.html | 7 +++ spec/fixtures/module/set-global-2.js | 1 - spec/fixtures/module/set-global-preload-1.js | 1 + spec/fixtures/module/set-global-preload-2.js | 1 + spec/fixtures/module/set-global-preload-3.js | 1 + 18 files changed, 190 insertions(+), 94 deletions(-) create mode 100644 atom/browser/session_preferences.cc create mode 100644 atom/browser/session_preferences.h create mode 100644 spec/fixtures/api/preloads.html delete mode 100644 spec/fixtures/module/set-global-2.js create mode 100644 spec/fixtures/module/set-global-preload-1.js create mode 100644 spec/fixtures/module/set-global-preload-2.js create mode 100644 spec/fixtures/module/set-global-preload-3.js diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 5bae8a179e19..39da83789102 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -17,6 +17,7 @@ #include "atom/browser/atom_permission_manager.h" #include "atom/browser/browser.h" #include "atom/browser/net/atom_cert_verifier.h" +#include "atom/browser/session_preferences.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/content_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" @@ -447,6 +448,8 @@ Session::Session(v8::Isolate* isolate, AtomBrowserContext* browser_context) content::BrowserContext::GetDownloadManager(browser_context)-> AddObserver(this); + new SessionPreferences(browser_context); + Init(isolate); AttachAsUserData(browser_context); } @@ -680,18 +683,17 @@ void Session::CreateInterruptedDownload(const mate::Dictionary& options) { length, last_modified, etag, base::Time::FromDoubleT(start_time))); } -void Session::AddPreload(const base::FilePath::StringType& preloadPath) { - preloads_.push_back(preloadPath); -} - -void Session::RemovePreload(const base::FilePath::StringType& preloadPath) { - preloads_.erase( - std::remove(preloads_.begin(), preloads_.end(), preloadPath), - preloads_.end()); +void Session::SetPreloads( + const std::vector& preloads) { + auto* prefs = SessionPreferences::FromBrowserContext(browser_context()); + DCHECK(prefs); + prefs->set_preloads(preloads); } std::vector Session::GetPreloads() const { - return preloads_; + auto* prefs = SessionPreferences::FromBrowserContext(browser_context()); + DCHECK(prefs); + return prefs->preloads(); } v8::Local Session::Cookies(v8::Isolate* isolate) { @@ -780,8 +782,7 @@ void Session::BuildPrototype(v8::Isolate* isolate, .SetMethod("getBlobData", &Session::GetBlobData) .SetMethod("createInterruptedDownload", &Session::CreateInterruptedDownload) - .SetMethod("addPreload", &Session::AddPreload) - .SetMethod("removePreload", &Session::RemovePreload) + .SetMethod("setPreloads", &Session::SetPreloads) .SetMethod("getPreloads", &Session::GetPreloads) .SetProperty("cookies", &Session::Cookies) .SetProperty("protocol", &Session::Protocol) diff --git a/atom/browser/api/atom_api_session.h b/atom/browser/api/atom_api_session.h index bd52a05cf875..d56c2400d4df 100644 --- a/atom/browser/api/atom_api_session.h +++ b/atom/browser/api/atom_api_session.h @@ -82,8 +82,7 @@ class Session: public mate::TrackableObject, void GetBlobData(const std::string& uuid, const AtomBlobReader::CompletionCallback& callback); void CreateInterruptedDownload(const mate::Dictionary& options); - void AddPreload(const base::FilePath::StringType& preloadPath); - void RemovePreload(const base::FilePath::StringType& preloadPath); + void SetPreloads(const std::vector& preloads); std::vector GetPreloads() const; v8::Local Cookies(v8::Isolate* isolate); v8::Local Protocol(v8::Isolate* isolate); @@ -107,7 +106,6 @@ class Session: public mate::TrackableObject, std::string devtools_network_emulation_client_id_; scoped_refptr browser_context_; - std::vector preloads_; DISALLOW_COPY_AND_ASSIGN(Session); }; diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 49ca11bfa58e..24500c4c3075 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -17,6 +17,7 @@ #include "atom/browser/atom_speech_recognition_manager_delegate.h" #include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/native_window.h" +#include "atom/browser/session_preferences.h" #include "atom/browser/web_contents_permission_helper.h" #include "atom/browser/web_contents_preferences.h" #include "atom/browser/window_list.h" @@ -277,9 +278,12 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( } content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); - if (web_contents) + if (web_contents) { WebContentsPreferences::AppendExtraCommandLineSwitches( web_contents, command_line); + SessionPreferences::AppendExtraCommandLineSwitches( + web_contents->GetBrowserContext(), command_line); + } } void AtomBrowserClient::DidCreatePpapiPlugin( diff --git a/atom/browser/session_preferences.cc b/atom/browser/session_preferences.cc new file mode 100644 index 000000000000..0731036d66f2 --- /dev/null +++ b/atom/browser/session_preferences.cc @@ -0,0 +1,61 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/session_preferences.h" + +#include "atom/common/options_switches.h" +#include "base/command_line.h" +#include "base/memory/ptr_util.h" + +namespace atom { + +namespace { + +#if defined(OS_WIN) +const base::FilePath::CharType kPathDelimiter = FILE_PATH_LITERAL(';'); +#else +const base::FilePath::CharType kPathDelimiter = FILE_PATH_LITERAL(':'); +#endif + +} // namespace + +// static +int SessionPreferences::kLocatorKey = 0; + +SessionPreferences::SessionPreferences(content::BrowserContext* context) { + context->SetUserData(&kLocatorKey, base::WrapUnique(this)); +} + +SessionPreferences::~SessionPreferences() { +} + +// static +SessionPreferences* SessionPreferences::FromBrowserContext( + content::BrowserContext* context) { + return static_cast(context->GetUserData(&kLocatorKey)); +} + +// static +void SessionPreferences::AppendExtraCommandLineSwitches( + content::BrowserContext* context, base::CommandLine* command_line) { + SessionPreferences* self = FromBrowserContext(context); + if (!self) + return; + + base::FilePath::StringType preloads; + for (const auto& preload : self->preloads()) { + if (!base::FilePath(preload).IsAbsolute()) { + LOG(ERROR) << "preload script must have absolute path: " << preload; + continue; + } + if (preloads.empty()) + preloads = preload; + else + preloads += kPathDelimiter + preload; + } + if (!preloads.empty()) + command_line->AppendSwitchNative(switches::kPreloadScripts, preloads); +} + +} // namespace atom diff --git a/atom/browser/session_preferences.h b/atom/browser/session_preferences.h new file mode 100644 index 000000000000..ab0ba470bbf0 --- /dev/null +++ b/atom/browser/session_preferences.h @@ -0,0 +1,46 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_SESSION_PREFERENCES_H_ +#define ATOM_BROWSER_SESSION_PREFERENCES_H_ + +#include + +#include "base/files/file_path.h" +#include "base/supports_user_data.h" +#include "content/public/browser/browser_context.h" + +namespace base { +class CommandLine; +} + +namespace atom { + +class SessionPreferences : public base::SupportsUserData::Data { + public: + static SessionPreferences* FromBrowserContext( + content::BrowserContext* context); + static void AppendExtraCommandLineSwitches( + content::BrowserContext* context, base::CommandLine* command_line); + + explicit SessionPreferences(content::BrowserContext* context); + ~SessionPreferences() override; + + void set_preloads(const std::vector& preloads) { + preloads_ = preloads; + } + const std::vector& preloads() const { + return preloads_; + } + + private: + // The user data key. + static int kLocatorKey; + + std::vector preloads_; +}; + +} // namespace atom + +#endif // ATOM_BROWSER_SESSION_PREFERENCES_H_ diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 1a369dd0e9a0..81ce0c1e0646 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -8,9 +8,6 @@ #include #include -#include "atom/browser/api/atom_api_session.h" -#include "atom/browser/api/atom_api_web_contents.h" -#include "atom/browser/api/atom_api_window.h" #include "atom/browser/native_window.h" #include "atom/browser/web_view_manager.h" #include "atom/common/native_mate_converters/value_converter.h" @@ -139,19 +136,6 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( LOG(ERROR) << "preload url must be file:// protocol."; } - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - mate::Handle api_web_contents = - atom::api::WebContents::CreateFrom(isolate, web_contents); - auto session = atom::api::Session::CreateFrom( - isolate, api_web_contents.get()->GetBrowserContext()); - for (const auto& preloadPath : session->GetPreloads()) { - if (base::FilePath(preloadPath).IsAbsolute()) - command_line->AppendSwitchNative(switches::kSessionPreloadScript, - preloadPath); - else - LOG(ERROR) << "preload script must have absolute path."; - } - // Run Electron APIs and preload script in isolated world bool isolated; if (web_preferences.GetBoolean(options::kContextIsolation, &isolated) && diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index 366aa1d95208..00f582b00617 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -61,14 +61,14 @@ class WebContentsPreferences private: friend class content::WebContentsUserData; + // Get preferences value as integer possibly coercing it from a string + bool GetInteger(const std::string& attributeName, int* intValue); + static std::vector instances_; content::WebContents* web_contents_; base::DictionaryValue web_preferences_; - // Get preferences value as integer possibly coercing it from a string - bool GetInteger(const std::string& attributeName, int* intValue); - DISALLOW_COPY_AND_ASSIGN(WebContentsPreferences); }; diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index 3daf7419b09e..0bfbae8c5c4b 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -177,18 +177,18 @@ const char kAppUserModelId[] = "app-user-model-id"; const char kAppPath[] = "app-path"; // The command line switch versions of the options. -const char kBackgroundColor[] = "background-color"; -const char kPreloadScript[] = "preload"; -const char kPreloadURL[] = "preload-url"; -const char kSessionPreloadScript[] = "session-preload"; -const char kNodeIntegration[] = "node-integration"; -const char kContextIsolation[] = "context-isolation"; -const char kGuestInstanceID[] = "guest-instance-id"; -const char kOpenerID[] = "opener-id"; -const char kScrollBounce[] = "scroll-bounce"; -const char kHiddenPage[] = "hidden-page"; -const char kNativeWindowOpen[] = "native-window-open"; -const char kWebviewTag[] = "webview-tag"; +const char kBackgroundColor[] = "background-color"; +const char kPreloadScript[] = "preload"; +const char kPreloadURL[] = "preload-url"; +const char kPreloadScripts[] = "preload-scripts"; +const char kNodeIntegration[] = "node-integration"; +const char kContextIsolation[] = "context-isolation"; +const char kGuestInstanceID[] = "guest-instance-id"; +const char kOpenerID[] = "opener-id"; +const char kScrollBounce[] = "scroll-bounce"; +const char kHiddenPage[] = "hidden-page"; +const char kNativeWindowOpen[] = "native-window-open"; +const char kWebviewTag[] = "webview-tag"; // Command switch passed to renderer process to control nodeIntegration. const char kNodeIntegrationInWorker[] = "node-integration-in-worker"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index ee5c1fcf58e5..7576fc9f61dc 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -91,7 +91,7 @@ extern const char kAppPath[]; extern const char kBackgroundColor[]; extern const char kPreloadScript[]; extern const char kPreloadURL[]; -extern const char kSessionPreloadScript[]; +extern const char kPreloadScripts[]; extern const char kNodeIntegration[]; extern const char kContextIsolation[]; extern const char kGuestInstanceID[]; diff --git a/docs/api/session.md b/docs/api/session.md index 54ca97fe662e..aa975fddd54f 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -384,22 +384,17 @@ the initial state will be `interrupted`. The download will start only when the Clears the session’s HTTP authentication cache. -#### `ses.addPreload(preloadPath)` +#### `ses.setPreloads(preloads)` -* `preloadPath` String - An absolute path to the preload script +* `preloads` String[] - An array of absolute path to preload scripts -Adds a script that will be executed on ALL web contents that are associated with +Adds scripts that will be executed on ALL web contents that are associated with this session just before normal `preload` scripts run. -#### `ses.removePreload(preloadPath)` - -* `preloadPath` String - An absolute path to the preload script - -Removes the given script from the list of preload scripts - #### `ses.getPreloads()` -Returns `String[]` an array of paths to preload scripts that have been registered +Returns `String[]` an array of paths to preload scripts that have been +registered. ### Instance Properties diff --git a/filenames.gypi b/filenames.gypi index 2559aeb61255..b2ab8a36103f 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -283,6 +283,8 @@ 'atom/browser/relauncher.h', 'atom/browser/render_process_preferences.cc', 'atom/browser/render_process_preferences.h', + 'atom/browser/session_preferences.cc', + 'atom/browser/session_preferences.h', 'atom/browser/ui/accelerator_util.cc', 'atom/browser/ui/accelerator_util.h', 'atom/browser/ui/accelerator_util_mac.mm', diff --git a/lib/renderer/init.js b/lib/renderer/init.js index 5ddf1d38d06e..1b0766d3007d 100644 --- a/lib/renderer/init.js +++ b/lib/renderer/init.js @@ -67,7 +67,7 @@ electron.ipcRenderer.on('ELECTRON_INTERNAL_RENDERER_ASYNC_WEB_FRAME_METHOD', (ev let nodeIntegration = 'false' let webviewTag = 'false' let preloadScript = null -const sessionPreloadScripts = [] +let preloadScripts = [] let isBackgroundPage = false let appPath = null for (let arg of process.argv) { @@ -87,11 +87,16 @@ for (let arg of process.argv) { appPath = arg.substr(arg.indexOf('=') + 1) } else if (arg.indexOf('--webview-tag=') === 0) { webviewTag = arg.substr(arg.indexOf('=') + 1) - } else if (arg.indexOf('--session-preload') === 0) { - sessionPreloadScripts.push(arg.substr(arg.indexOf('=') + 1)) + } else if (arg.indexOf('--preload-scripts') === 0) { + preloadScripts = arg.substr(arg.indexOf('=') + 1).split(path.delimiter) } } +// The webContents preload script is loaded after the session preload scripts. +if (preloadScript) { + preloadScripts.push(preloadScript) +} + if (window.location.protocol === 'chrome-devtools:') { // Override some inspector APIs. require('./inspector') @@ -174,17 +179,8 @@ if (nodeIntegration === 'true') { }) } -for (const sessionPreloadScript of sessionPreloadScripts) { - try { - require(sessionPreloadScript) - } catch (error) { - console.error('Unable to load session preload script: ' + sessionPreloadScript) - console.error(error.stack || error.message) - } -} - -// Load the script specfied by the "preload" attribute. -if (preloadScript) { +// Load the preload scripts. +for (const preloadScript of preloadScripts) { try { require(preloadScript) } catch (error) { diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ab085c205b1a..195e94f39b78 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1022,39 +1022,39 @@ describe('BrowserWindow module', () => { }) describe('session preload scripts', function () { - it('can add and remove multiple session preload script', function () { - var preload = path.join(fixtures, 'module', 'set-global.js') - var preload2 = path.join(fixtures, 'module', 'set-global-2.js') - const mSession = session.defaultSession - assert.deepEqual(mSession.getPreloads(), []) - mSession.addPreload(preload) - assert.deepEqual(mSession.getPreloads(), [preload]) - mSession.addPreload(preload2) - assert.deepEqual(mSession.getPreloads(), [preload, preload2]) - mSession.removePreload(preload) - assert.deepEqual(mSession.getPreloads(), [preload2]) - mSession.removePreload(preload2) - assert.deepEqual(mSession.getPreloads(), []) + const preloads = [ + path.join(fixtures, 'module', 'set-global-preload-1.js'), + path.join(fixtures, 'module', 'set-global-preload-2.js') + ] + const defaultSession = session.defaultSession + + beforeEach(() => { + assert.deepEqual(defaultSession.getPreloads(), []) + defaultSession.setPreloads(preloads) + }) + afterEach(() => { + defaultSession.setPreloads([]) + }) + + it('can set multiple session preload script', function () { + assert.deepEqual(defaultSession.getPreloads(), preloads) }) it('loads the script before other scripts in window including normal preloads', function (done) { - var preload = path.join(fixtures, 'module', 'set-global.js') - var preload2 = path.join(fixtures, 'module', 'set-global-2.js') - const mSession = session.defaultSession - ipcMain.once('answer', function (event, test) { - mSession.removePreload(preload2) - assert.equal(test, 'preload2') + ipcMain.once('vars', function (event, preload1, preload2, preload3) { + assert.equal(preload1, 'preload-1') + assert.equal(preload2, 'preload-1-2') + assert.equal(preload3, 'preload-1-2-3') done() }) w.destroy() - mSession.addPreload(preload2) w = new BrowserWindow({ show: false, webPreferences: { - preload: preload + preload: path.join(fixtures, 'module', 'set-global-preload-3.js') } }) - w.loadURL('file://' + path.join(fixtures, 'api', 'preload.html')) + w.loadURL('file://' + path.join(fixtures, 'api', 'preloads.html')) }) }) diff --git a/spec/fixtures/api/preloads.html b/spec/fixtures/api/preloads.html new file mode 100644 index 000000000000..f13510b1b212 --- /dev/null +++ b/spec/fixtures/api/preloads.html @@ -0,0 +1,7 @@ + + + + + diff --git a/spec/fixtures/module/set-global-2.js b/spec/fixtures/module/set-global-2.js deleted file mode 100644 index 9e2b61da2b9f..000000000000 --- a/spec/fixtures/module/set-global-2.js +++ /dev/null @@ -1 +0,0 @@ -if (!window.test) window.test = 'preload2' diff --git a/spec/fixtures/module/set-global-preload-1.js b/spec/fixtures/module/set-global-preload-1.js new file mode 100644 index 000000000000..22dfdf918506 --- /dev/null +++ b/spec/fixtures/module/set-global-preload-1.js @@ -0,0 +1 @@ +window.preload1 = 'preload-1' diff --git a/spec/fixtures/module/set-global-preload-2.js b/spec/fixtures/module/set-global-preload-2.js new file mode 100644 index 000000000000..7542009f7b09 --- /dev/null +++ b/spec/fixtures/module/set-global-preload-2.js @@ -0,0 +1 @@ +window.preload2 = window.preload1 + '-2' diff --git a/spec/fixtures/module/set-global-preload-3.js b/spec/fixtures/module/set-global-preload-3.js new file mode 100644 index 000000000000..9cfef949277e --- /dev/null +++ b/spec/fixtures/module/set-global-preload-3.js @@ -0,0 +1 @@ +window.preload3 = window.preload2 + '-3'