Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay load google-earth-dbroot-parser #7140

Merged
merged 3 commits into from
Oct 16, 2018
Merged

Delay load google-earth-dbroot-parser #7140

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dde06c6
Delay load google-earth-dbroot-parser
mramato Oct 11, 2018
eaef93b
Update CHANGES
mramato Oct 15, 2018
b7cae12
Merge remote-tracking branch 'origin/master' into delay-load-gihugic-…
mramato Oct 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions Source/Core/GoogleEarthEnterpriseMetadata.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
define([
'../ThirdParty/google-earth-dbroot-parser',
'../ThirdParty/protobuf-minimal',
'../ThirdParty/when',
'./buildModuleUrl',
'./Check',
'./Credit',
'./defaultValue',
'./defined',
'./defineProperties',
'./GoogleEarthEnterpriseTileInformation',
'./isBitSet',
'./loadAndExecuteScript',
'./Math',
'./Request',
'./Resource',
'./RuntimeError',
'./TaskProcessor'
], function(
dbrootParser,
protobufMinimal,
when,
buildModuleUrl,
Check,
Credit,
defaultValue,
defined,
defineProperties,
GoogleEarthEnterpriseTileInformation,
isBitSet,
loadAndExecuteScript,
CesiumMath,
Request,
Resource,
Expand Down Expand Up @@ -488,6 +492,8 @@ define([
});
}

var dbrootParser;
var dbrootParserPromise;
function requestDbRoot(that) {
var resource = that._resource.getDerivedResource({
url: 'dbRoot.v5',
Expand All @@ -496,8 +502,23 @@ define([
}
});

return resource.fetchArrayBuffer()
.then(function(buf) {
if (!defined(dbrootParserPromise)) {
var url = buildModuleUrl('ThirdParty/google-earth-dbroot-parser.js');
var oldValue = window.cesiumGoogleEarthDbRootParser;
dbrootParserPromise = loadAndExecuteScript(url)
.then(function() {
dbrootParser = window.cesiumGoogleEarthDbRootParser(protobufMinimal);
if (defined(oldValue)) {
window.cesiumGoogleEarthDbRootParser = oldValue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we're using window.cesiumGoogleEarthDbRootParser for both the loaded script, and the function evaluated with protobufMinimal. Which means you have to set back the oldValue after evaluating. Would this be clearer if we just loaded the module to window.cesiumGoogleEarthDbRootModule or something similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. Here's what's currently happening

  1. We get the old value of cesiumGoogleEarthDbRootParser on the off-chance it exists
  2. We load the script, which defines cesiumGoogleEarthDbRootParser.
  3. We call the cesiumGoogleEarthDbRootParser function and store the result into dbrootParser (module scoped)
  4. We set back the old value of cesiumGoogleEarthDbRootParser or delete it completely if it didn't exist to begin with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, sorry for the confusion. Makes sense.

} else {
delete window.cesiumGoogleEarthDbRootParser;
}
});
}

return dbrootParserPromise.then(function() {
return resource.fetchArrayBuffer();
}).then(function(buf) {
var encryptedDbRootProto = dbrootParser.EncryptedDbRootProto.decode(new Uint8Array(buf));

var byteArray = encryptedDbRootProto.encryptionData;
Expand Down
19 changes: 3 additions & 16 deletions Source/Core/Resource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ define([
'./defineProperties',
'./deprecationWarning',
'./DeveloperError',
'./FeatureDetection',
'./freezeObject',
'./getAbsoluteUri',
'./getBaseUri',
'./getExtensionFromUri',
'./isBlobUri',
'./isCrossOriginUrl',
'./isDataUri',
'./loadAndExecuteScript',
'./objectToQuery',
'./queryToObject',
'./Request',
Expand All @@ -38,14 +38,14 @@ define([
defineProperties,
deprecationWarning,
DeveloperError,
FeatureDetection,
freezeObject,
getAbsoluteUri,
getBaseUri,
getExtensionFromUri,
isBlobUri,
isCrossOriginUrl,
isDataUri,
loadAndExecuteScript,
objectToQuery,
queryToObject,
Request,
Expand Down Expand Up @@ -1934,20 +1934,7 @@ define([
};

Resource._Implementations.loadAndExecuteScript = function(url, functionName, deferred) {
var script = document.createElement('script');
script.async = true;
script.src = url;

var head = document.getElementsByTagName('head')[0];
script.onload = function() {
script.onload = undefined;
head.removeChild(script);
};
script.onerror = function(e) {
deferred.reject(e);
};

head.appendChild(script);
return loadAndExecuteScript(url, functionName).otherwise(deferred.reject);
};

/**
Expand Down
32 changes: 32 additions & 0 deletions Source/Core/loadAndExecuteScript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
define([
'../ThirdParty/when'
], function(
when) {
'use strict';

/**
* @private
*/
function loadAndExecuteScript(url) {
var deferred = when.defer();
var script = document.createElement('script');
script.async = true;
script.src = url;

var head = document.getElementsByTagName('head')[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a (small) chance the page may not have a head element? Can we just append it to the document?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The head element will always exist.

script.onload = function() {
script.onload = undefined;
head.removeChild(script);
deferred.resolve();
};
script.onerror = function(e) {
deferred.reject(e);
};

head.appendChild(script);

return deferred.promise;
}

return loadAndExecuteScript;
});
6 changes: 2 additions & 4 deletions Source/ThirdParty/google-earth-dbroot-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
define([
'./protobuf-minimal'
], function(
window.cesiumGoogleEarthDbRootParser = function(
$protobuf) {
/* jshint curly: false, sub: true, newcap: false, shadow: true, unused: false*/
'use strict';
Expand Down Expand Up @@ -8147,4 +8145,4 @@ define([
// End generated code

return $root.keyhole.dbroot;
});
};
14 changes: 12 additions & 2 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var sourceFiles = ['Source/**/*.js',
'!Source/*.js',
'!Source/Workers/**',
'!Source/ThirdParty/Workers/**',
'!Source/ThirdParty/google-earth-dbroot-parser.js',
'!Source/ThirdParty/pako_inflate.js',
'!Source/ThirdParty/crunch.js',
'Source/Workers/createTaskProcessorWorker.js'];
Expand Down Expand Up @@ -891,6 +892,14 @@ function minifyCSS(outputDirectory) {
});
}

var gulpUglify = require('gulp-uglify');

function minifyModules(outputDirectory) {
return streamToPromise(gulp.src('Source/ThirdParty/google-earth-dbroot-parser.js')
.pipe(gulpUglify())
.pipe(gulp.dest(outputDirectory + '/ThirdParty/')));
}

function combineJavaScript(options) {
var optimizer = options.optimizer;
var outputDirectory = options.outputDirectory;
Expand All @@ -901,7 +910,8 @@ function combineJavaScript(options) {

var promise = Promise.join(
combineCesium(!removePragmas, optimizer, combineOutput),
combineWorkers(!removePragmas, optimizer, combineOutput)
combineWorkers(!removePragmas, optimizer, combineOutput),
minifyModules(outputDirectory)
);

return promise.then(function() {
Expand Down Expand Up @@ -1264,7 +1274,7 @@ function buildCesiumViewer() {

gulp.src(['Build/Cesium/Assets/**',
'Build/Cesium/Workers/**',
'Build/Cesium/ThirdParty/Workers/**',
'Build/Cesium/ThirdParty/**',
'Build/Cesium/Widgets/**',
'!Build/Cesium/Widgets/**/*.css'],
{
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"gulp-rename": "^1.2.2",
"gulp-replace": "^0.6.1",
"gulp-tap": "^1.0.1",
"gulp-uglify": "^3.0.0",
"gulp-zip": "^4.0.0",
"jasmine-core": "^3.1.0",
"jsdoc": "^3.4.3",
Expand Down